Repository navigation
perf(vcs): run VCS sync batches in the placed fetch handler - #1553
Conversation
Maple review🟢 Confidence 7/10 · likely safe to merge The consumer hands each VCS sync batch to the Worker's own placed fetch handler, shortening its Postgres and ingest hops. Safe to merge, but a hop that fails re-retries messages that already exhausted their attempts. The open
Before merge
Findings🟠 Warning · F1 · Retry-all after a failed placed hop drops the exhausted message's terminal statuscorrectness · When the hop fails after the batch may have started, every message goes through 🤖 Prompt to fix this finding with an AI agentProduction impactOpen errors in the changed files
After this merges, Maple checks whether they stop. Production traffic of the changed files (last 7 days)
Telemetry this change adds and removes (3)
Ingest cost of new log lines
What was checked
Observability coverage: 3 of 3 changes observable
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
🍁 Maple PR previewNote Preview resources were removed when the Final commit |
Maple review🟢 Confidence 8/10 · likely safe to merge Runs VCS sync batches through a placed fetch handler over a self binding, fixing the failed-hop terminal status and falling back to inline.
Before merge
Fixed since the last review
Production impactOpen errors in the changed files
After this merges, Maple checks whether they stop. Production traffic of the changed files (last 7 days)
Telemetry this change adds and removes (4)
Ingest cost of new log lines
What was checked
Observability coverage: 2 of 2 changes observable
|
Worker placement applies to fetch invocations only, so the VCS sync queue consumer ran in whatever colo the queue picked, far from Postgres and the ingest gateway. The consumer now posts its batch to the api Worker's own fetch handler over a self service binding (API_SELF), the way alerting hands off its cron ticks. The placed handler runs the batch and answers one outcome per message (ack, or retry with an optional delay), and the consumer applies them. Without the binding or token, or when the handler refuses the request (400/401/404), the batch runs inline exactly as before. A transport failure or a 5xx retries every message, and a message missing from the answer is retried, so an unknown outcome is never acked. The self-binding helpers move from alerting into @maple/backend/platform/self-binding so both Workers share them. The route's server span is suppressed so the batch time stays on maple-vcs-sync, not maple-api.
…final deliveries of a failed hop The api Worker has public routes, so the batch route refuses any request that carries cf-connecting-ip. Cloudflare stamps that header on every request from the internet and a client cannot remove it; a service binding call carries only the headers the consumer set. The refusal is a 404, which the consumer treats as "did not run" and so falls back inline. A named entrypoint was not an option: placement does not apply to RPC methods or named entrypoints. The route decodes messages against the real VcsSyncJob union, caps a request at one queue batch (10 messages) and at 10 x 128 KB plus an envelope, read with a bounded stream so an oversize body is refused before it is buffered. When the hop fails with an unknown outcome, messages on their final delivery are no longer retried into nothing. Their terminal status is recorded through VcsSyncService.recordExhaustedFailure and they are acked, as a failed final attempt inline would be. Every other message is still retried, never acked.
…r holds The batch route accepted the internal service token, which other Workers also hold, so any of them could post jobs to it. The hop now carries a random secret alchemy mints once and binds to the api Worker alone. The edge check on cf-connecting-ip stays as a second guard. A consumer without the secret runs its batch inline, as before.
c6d447b to
15a8812
Compare
Maple review🟢 Confidence 7/10 · likely safe to merge VCS sync batches now hop to the api Worker's own placed fetch handler over a self binding, authenticated with a per-Worker secret, with inline fallback. The four changed-since files hold up. The 10 open
Before merge
Production impactOpen errors in the changed files
After this merges, Maple checks whether they stop. Production traffic of the changed files (last 7 days)
Telemetry this change adds and removes (4)
Ingest cost of new log lines
What was checked
Observability coverage: 3 of 3 changes observable
|
|
Merged with 1 step from "Before merge" still open. If it is done, tick it on the review comment; if not, now is the time.
|
Maple: after this shippedDeployed to Errors in the changed files:
Maple looks once, an hour after the first deploy that carries the merge. |
Problem
The VCS sync queue consumer (
maple-vcs-sync,VcsSyncQueue.processBatch) runs in whatever colo the queue delivers to, not beside the database and the ingest gateway. Every Postgres query and every CI telemetry POST pays a cross-continent round trip.Production, 6 h ending 2026-10-19 09:40 UTC,
deployment.environment = production:processBatchtotalFor comparison, placed invocations in the same window:
maple-apiEU (fetch, FRA) Postgres p50 8 to 40 ms; alerting (placed through its self binding) Postgres p50 6 ms EU / 8 ms US, ingest POST p50 16 ms EU / 10 ms US.Cause
Worker placement (
resolveWorkerPlacement,packages/infra/src/cloudflare/stage.ts:57) applies only to fetch invocations.apps/api/src/worker/consumers.ts:45ranprocessBatchdirectly in the queue invocation, so it was never placed.Fix
The same pattern alerting uses for its cron ticks (
apps/alerting/src/tick-dispatch.ts):API_SELF: Cloudflare.Workers.Self(apps/api/src/worker.ts).id,body,attemptsper message) toPOST /internal/vcs-sync/batchwith a hop secret as the bearer (worker/vcs-sync-dispatch.ts). The secret is an alchemyRandom, minted once, kept in state and bound to the api Worker alone, so no other Worker holding the internal service token can post jobs. The route is answered inmakeFetchahead of the route graph, like/health.ack, orretrywith an optional delay. The consumer applies them to the messages it holds.processBatchis split so the per-message decision (settleMessage) is shared. Inline, each message is still acked or retried as soon as it settles. Retry, rate-limit delay and exhausted-attempt handling are unchanged.cf-connecting-ipgets a 404 before the bearer is checked: Cloudflare stamps that header on requests from the internet, while a binding call carries only the headers the consumer set. A named entrypoint is not used because placement does not apply to RPC methods or named entrypoints.VcsSyncJobunion, takes at most one queue batch (10 messages) and at most 10 x 128 KB plus a 16 KB envelope, read through a bounded stream. Any violation is a 400.recordExhaustedFailureand an ack instead (there is no dead-letter queue), the same terminal handling as a failed final attempt inline; it is not re-run, since the placed run may have finished it. A message missing from the answer is retried. An unknown outcome is otherwise never acked.ci-traces/logs.tsare unchanged.maple-apiserver span is suppressed (TracerDisabledWhen), so batch time stays onmaple-vcs-syncand does not skewmaple-apilatency. The batch span now carriesmaple.vcs.dispatch(placed/inline) andmaple.placement(cf-placementheader). The consumer records aVcsSyncQueue.dispatchspan.selfFetcher,workerSecretFor,internalTokenFor) move from alerting into@maple/backend/platform/self-bindingand both Workers use them.Tests (
apps/api/src/worker/vcs-sync.test.ts) drive the real consumer hand-off through a binding that answers with the real route handler and the realprocessPlacedBatch: per-message ack / delayed retry / exhausted ack / retry; inline fallback for a missing binding and a 401; retry on a failed placed run, and terminal record plus ack for a final delivery in it; retry for a message with no outcome; and the inlineprocessBatchpath. The route itself is tested for a full batch, a 401 for the internal service token, a 404 to a request withcf-connecting-ipeven with the bearer, the inline fallback when a binding call is refused that way, and 400s for 11 messages, a body that is not a job, and an oversize body with and withoutcontent-length.Before / After
processBatchtotalprocessBatchtotalMethod: before numbers are production telemetry via
maple query --remote --since 6h, window ending 2026-10-19 09:40 UTC. After numbers are expectations from the placed alerting Worker's latencies in the same regions, not measurements. A PR preview is US only and has no GitHub installations, so the VCS queue gets no traffic there and this path cannot be exercised end to end. The preview only confirms that the route is deployed and refuses an unauthenticated call.Preview result (PR 1553, 2026-10-19 ~09:58 UTC): the api Worker deployed at this commit (
x-maple-revision: d91f47b8a),GET /health200,POST /internal/vcs-sync/batchwith no bearer 401 and with a wrong bearer 401. The preview job as a whole failed later on an AWSVpcLimitExceededfor the ingest and electric stacks, which this PR does not touch, so the seed and smoke-test step did not run.Merge order
Merge this before #1551. #1551 then rebases to give both VCS paths (queue consumer and the batch route) the isolate token and ingest-key caches; its body describes the resolution.
How to verify after deploy
Outbound colo by region (expect FRA for EU, IAD for US):
Postgres and ingest latency (expect
SELECT vcs_installationsp50 under 15 ms, ingest POST p50 under 30 ms):Batch time and how batches ran (compare with 1,875 s EU / 518 s US per 6 h):
Any
inlinerows after deploy mean the hop was refused;VcsSyncQueue.dispatchspans carrymaple.vcs.dispatch.reason. A steadyrefused-404reason would mean binding calls arrive withcf-connecting-ip, which the docs do not suggest; the batches still run, inline.Summary by CodeRabbit