Skip to content

fix(config): coalesce preview config reads per source snapshot - #4522

Merged
kwakayama merged 7 commits into
mainfrom
fix/preview-config-read-coalescing
Sep 19, 2026
Merged

kwakayama merged 7 commits into
mainfrom
fix/preview-config-read-coalescing

Conversation

@kwakayama

@kwakayama kwakayama commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Fixes veryfront/veryfront-issue-inbox#1510
Fixes veryfront/veryfront-issue-inbox#1511

Cause

On 2026-09-19 03:40:30 UTC, 10 concurrent GET / requests for one hosted project reached a production pod about 100s after it started. None completed. One request's trace made 1,044 calls to /projects/:id/cache/*: 998 individual /cache/get calls (p50 1.0s, 312 hit the 10s timeout), 985 of them under mdx.fetch_module < utils.parallelMap < mdx.process_vf_modules. The api-cache-http circuit breaker opened at 03:41:02, about 2,000 fast failures followed, the MDX transform tree timed out at 47-66s (#1510), and the SSR pipeline hit its 60s deadline (#1511). The same pattern appeared on 2026-09-15 01:16 on another pod (2,076 request timeouts). Operator memory recycling (since 2026-09-12) replaces 11-26 production pods per day, so cold pods are now common.

Two multipliers on a cold pod:

  1. fetchAndCacheModule deduplicates in-flight modules only within one processVfModuleImports call (context.inFlightModules). N concurrent cold renders of the same page each walk the whole _vf_modules graph and repeat every distributed cache read.
  2. Bundle recovery dominates the reads. Log analysis of a single cold production render (2,080 /cache/get in one request) shows each _vf_modules section recovering a graph of about 247 HTTP bundles. ensureHttpBundlesExist fetches the code in batches (get-batch), but then looks up each bundle's recovery identity with its own get calls (identity + import map, about 2 per bundle, which matches the ~517 gets per section in the trace). Sibling sections recovered the same bundle graph at the same time.

Fix

  • Process-wide single flight for entry module fetches (module-fetcher/shared-module-fetches.ts). Entry fetches (no parent module) with the same identity share one in-flight resolution. The key covers project ID, content source, ESM cache directory, project directory, local-project flag, compile mode, React version, dependency snapshot key, module server origin, server external packages, missing-module mode, and entry path. Results never cross projects or content versions.
    • Only entry fetches are shared. Nested fetches stay scoped to the resolution that owns them, and a call made from inside a shared resolution never joins another one, so two resolutions cannot wait on each other.
    • Entries are removed when they settle. Resolved paths are then served from the module path cache, and a rejection is shared with current waiters only. The next request retries.
    • invalidateModulePaths and clearModulePathCache reset the map, so a request that starts after a content change never joins a resolution that read the old source.
    • Joined renders still record the resolved modules into their own render session (route module manifest).
    • A never-settling resolution stops accepting new joiners after 60s (the existing Singleflight stale guard).
    • Per-request limits still apply to joined renders. Modules the shared resolution recorded count toward the joined render's 500-module graph limit. If the shared resolution fails with the leading render's TransformTreeTimeoutError, a joined render retries alone within its own deadline.
  • Missing HTTP bundles are fetched once per cache directory across callers (bundle-recovery.ts). ensureHttpBundlesExist claims missing hashes synchronously before its first await. Other callers wait for the claim, then read the result from disk, and recover any bundle the claimant could not write. A claim is held until the bundle is written or single-bundle recovery settles. A caller releases all of its claims before it waits on other claims. Recovery that runs under a held claim is marked with AsyncLocalStorage. Nested ensureHttpBundlesExist calls inside it fetch in-flight bundles themselves instead of waiting, so claim holders never wait on each other.
  • Batched recovery identity lookups (HttpBundleCache.getBatchRecoveryIdentities). Identity records, shared import maps (read once per fingerprint), and original URLs are read with getBatch instead of one get per bundle. The semantics match getIdentityMetadata + getOriginalUrl.

The 30s transform-tree deadline semantics are unchanged.

Tests (red before the fix)

  • module-fetcher/index.test.ts, "process-wide module fetch single-flight": 10 concurrent cold resolutions of one entry graph (page -> a, b -> c) against a distributed cache stub that counts get and adds 200ms latency. Before: 80 cache gets and 40 source reads (10x). After: 8 cache gets (same as one solo cold graph) and 4 source reads. Also covers a joined render retrying alone after the leading render's deadline, graph-limit admission for joined renders, cross-project isolation of the same path, a shared rejection that is retried by the next request (before: 3 source reads for 3 concurrent callers, and they did not all reject), no joining across an invalidation, and session attribution for every joined render.
  • shared-module-fetches.test.ts (hermetic): key identity per input, single run per key, rejection not retained, synchronous failure, nested-call bypass, reset, and session replay.
  • bundle-recovery.test.ts: 30 bundles recovered with zero single-key reads and one import-map read (before: 60 single reads). 5 concurrent ensureHttpBundlesExist calls fetch each of 20 bundles once (before: 100 code reads, after: 20). A bundle a concurrent claimant failed to fetch is retried by the waiter. Single-bundle recovery for 3 concurrent callers runs once (before: 3 direct code reads).

Local: deno task lint:ci, deno task typecheck, deno fmt --check, and deno task test:file for src/transforms/, src/modules/react-loader/, src/modules/manifest/, src/rendering/, src/cache/, and src/server/context/ all pass.

Staging verification

Deployed to staging as 20260919165659-6fa06e2ac11c (release v0.1.37-rc.534); all 4 replicas rolled out. Pod logs show OpenTelemetry initialized followed by Sentry initialized successfully, and OTel trace_id still appears in request logs.

Two staging Remote E2E Health runs (concurrent UI and ai-live traffic) triggered the recurring gateway-accounting events VERYFRONT-API-J and VERYFRONT-API-K. All of these events share the path /ai/gateway/anthropic/v1/messages.

  • Before (earlier releases, last 3 days): 43 of 61 events carried another request's transaction, such as GET /conversations/:conversation_id/runs/:run_id/snapshot, GET /credits/balance, POST /agent-runtimes/push-services/:service_id/heartbeat or GET /projects/:project_reference/files.
  • After (v0.1.37-rc.534): 4 of 4 events carry POST /ai/gateway/:provider/* (17:28:58Z and 17:41:58Z).

Preview hosted config reads used a fresh flight key per request, so a burst of preview requests for one project exhausted the shared source-read admission budget (2 active + 16 queued) and failed with evaluator-unavailable: worker-overloaded. Key preview reads on the adapter's source snapshot identity and generation instead. An edit advances the generation, so it still starts a new read; adapters that cannot name their snapshot keep the previous fresh key.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@coderabbitai

coderabbitai Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 7 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 67fc896a-9ff1-4b10-af83-c259b0f744bf

📥 Commits

Reviewing files that changed from the base of the PR and between 3c55ee9 and e1e362b.

📒 Files selected for processing (5)
  • src/config/loader.test.ts
  • src/config/loader.ts
  • src/platform/adapters/fs/veryfront/request-context.ts
  • src/platform/request-context-access.test.ts
  • src/platform/request-context-access.ts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review 🔄 Running since 2026-09-19T18:28:38.165766Z e1e362b Manual request
🔒 Security Review ✅ Completed 2026-09-19T18:35:17.213372Z e1e362b New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

📦 Client bundle boundary

Entrypoint Modules Source size Server leaks
src/index.client.ts 289 2319 KiB ✅ 0

A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in scripts/lint/client-bundle-baseline.json to burn down.

@gitar-bot

gitar-bot Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

Gitar is working

Gitar

Copy link
Copy Markdown
Contributor Author

Code Review — Score: 88/100 (good, minor suggestions)

Well-scoped incident-driven fix that closes the exact gap it names, with strong test evidence and no production-path risk.

Strengths

  • The root cause is precisely diagnosed (Sentry burst, admission limits of 2 active + 16 queued, 69–113 cache misses/min) and the fix maps directly onto it: preview reads now coalesce per retained source snapshot instead of getting a fresh key every request.
  • Reuses the existing getSourceSnapshotIdentity/getSourceSnapshotVersion adapter capability already established elsewhere in the codebase (page-rendering.ts, layout-collector.ts, multi-project-adapter.ts) rather than inventing a new contract — good consistency with prior art, including the error-registry hint (server.ts:169) that already documents this exact capability pair for "strict preview configuration."
  • Fails safe by design: an adapter that can't name its snapshot, returns an unstable observation across the two verification reads, or throws, keeps the prior always-fresh-key behavior (loader.ts:1046-1065). Production keys are provably unchanged, since previewSnapshot is forced undefined whenever productionMode is true.
  • Test coverage is a highlight: loader.test.ts adds a test that reproduces the exact failure mode (20 concurrent preview loads exceeding maxActive + maxQueued) and a companion test proving a snapshot-version bump starts a new read instead of joining the stale one — both follow the file's existing helper/style conventions.
  • PR description documents the local validation matrix (typecheck, lint, fmt, targeted + full suite) and lays out a concrete staging verification plan for after merge.

Minor suggestions (non-blocking)

  • captureHostedConfigSourceSnapshot (loader.ts:1046) does 4 sequential awaits per preview cache-miss (identity, version, then both again for stability). If any adapter implementation of getSourceSnapshotIdentity/getSourceSnapshotVersion does real I/O rather than an in-memory read, this adds latency to every preview request. Worth confirming that assumption, or at least pairing the two stability-check calls with Promise.all to shave a couple of microtask hops off the hot path.
  • The PR is currently mergeable_state: blocked with CI/automated review still pending (CodeRabbit rate-limited, Codex review running at review time) and no human reviewer feedback yet — nothing actionable from bots so far, but this should land green with the requested reviewer's sign-off before merge.

Nothing here blocks approval on the code itself — the two points above are polish, not correctness concerns.


Generated by Claude Code

@codecov

codecov Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.82540% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/config/loader.ts 96.19% 0 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!

@kwakayama

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 78b4295d10

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/config/loader.ts Outdated
@kwakayama

Copy link
Copy Markdown
Contributor Author

Addressed the abort finding: the loader rechecks the request signal after the snapshot probe and before creating a read flight. A new test aborts during an async getSourceSnapshotVersion and asserts no read starts (it failed before the recheck). @codex review

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@github-actions

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8f6530b834

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/config/loader.ts Outdated
@kwakayama

Copy link
Copy Markdown
Contributor Author

Addressed the P1: the cold snapshot probe now runs behind source-read admission. Concurrent requests for one project share one admitted warm-up probe, and each request then takes its own observation. The new test admits cold preview snapshot probes through the source-read budget failed on the previous head (4 distinct cold probes all ran at once) and passes now. @codex review

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@github-actions

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3205151bef

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/config/loader.ts Outdated
@kwakayama

Copy link
Copy Markdown
Contributor Author

Addressed: the admitted warm-up key now includes a SHA-256 digest of the request credential, so each concrete adapter's cold initialization goes through source-read admission. The new test admits cold preview snapshot probes separately for each credential failed on the previous head and passes now. @codex review

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@github-actions

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b6e06d7c05

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/config/loader.ts Outdated

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@kwakayama

Copy link
Copy Markdown
Contributor Author

Addressed: the warm-up key now covers every request-context field that MultiProjectFSAdapter passes to the proxy manager (project slug, project ID, credential digest, mode, release, environment, branch). Equal keys therefore select the same concrete adapter, and each follower's own observation is warm. The test now varies credential, project slug, and environment. It failed on the previous head and passes now. @codex review

@github-actions

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dc3e54e604

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/config/loader.ts Outdated
@kwakayama

Copy link
Copy Markdown
Contributor Author

Addressed: the unadmitted second capture is gone. The admitted probe flight settles with the snapshot, and waiters with an identical adapter selector share it. A failed or unstable probe falls back to an unshared admitted read instead of probing again. The new test does not retry a failed preview snapshot probe outside source-read admission failed on the previous head (a second probe ran for every context) and passes now. The selector-key finding was already addressed in dc3e54e. @codex review

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@github-actions

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e5987c363d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/config/loader.ts
@kwakayama

Copy link
Copy Markdown
Contributor Author

Addressed: a shared preview read now runs in the creator's request context but without its request-scoped file cache (runWithoutRequestScopedFileCache), so bytes a request pinned before an edit cannot reach requests that join at the new snapshot version. The new test does not share bytes a request pinned before the preview snapshot advanced failed on the previous head (the fresh request got before-edit) and passes now. @codex review

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@github-actions

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: e1e362b2d1

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@sonarqubecloud

Copy link
Copy Markdown

@kwakayama
kwakayama added this pull request to the merge queue Sep 19, 2026
Merged via the queue into main with commit b6f19aa Sep 19, 2026
95 of 99 checks passed
@kwakayama
kwakayama deleted the fix/preview-config-read-coalescing branch September 19, 2026 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant