Skip to content

Compactions keep running while a commit waits to plan - #40

Merged
tefkah merged 4 commits into
mainfrom
compaction-race
Oct 15, 2026
Merged

tefkah merged 4 commits into
mainfrom
compaction-race

Conversation

@isTravis

@isTravis isTravis commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

Fixes a race in #39 that stalled a large commit on staging (session 568fc1c9, 2026-10-15), and shortens how long a lost compaction waits.

What happened. A commit request moves the session to committing straight away, but the phase that says it is compacting (commitPhase: 'compacting') is set later, by the push.commit job. Compaction jobs only treated a committing session as active once that phase was set. So a compaction that ran in the gap, which was every compaction queued just before the request, decided the session no longer wanted it and returned. Its runs stayed claimed.

The stalled-compaction sweep was the only way back, and it re-queued a claim only once it was 30 minutes old. On staging, 176 of 190 runs were claimed by compactions that had quit; the queue was empty and every job had acknowledged its message. The loader's log showed it exactly: compacting 190 runs (176 being merged) for 30 minutes, then 175, 115 and 85 runs in the three minutes after the sweep re-queued them.

The fix. A compaction keeps running while its session is open, or committing and not yet planned (commitPlan is null). Once the commit has planned, it reads the run list it planned from, so a compaction finishing after that changes nothing. A compaction that finishes while a serial commit reads the runs is harmless: that commit read the run list once, and every input stays in storage. The sweep's re-queue of lost compactions uses the same rule.

Faster recovery. A compaction claim is refreshed when its merge starts and after each piece, and a piece takes minutes. So the sweep now re-queues a claim after 15 quiet minutes (recoverConfig.compactionStaleMs), not the 30 used for commit stalls. The sweep runs every 10 minutes, so a lost compaction comes back within 15 to 25 minutes instead of 30 to 40. The cost: a compaction that waits more than 15 minutes in a backed-up queue before it starts may run twice. That's safe, because each piece is skipped if it's already written.

Tests. recover.test.ts adds three:

  • a commit requested before the compaction job runs: the compaction now does its work, and the commit completes;
  • a lost compaction of a session that's committing but not yet planned is re-queued by the sweep;
  • a claim quiet for 10 minutes is left alone, and one quiet for 16 minutes is re-queued.

The first two fail without the fix. Server tests 250/250, typecheck and oxlint clean.

Not merged or deployed: a commit is in flight on staging and recovers on its own through the sweep.

isTravis and others added 4 commits October 15, 2026 12:01
…it's marked compacting; the sweep re-queues lost ones on the same rule
…: claims refresh as merges start and pieces finish
…he sweep re-queues a session's compactions only once none of its claims has moved
@tefkah

tefkah commented Oct 15, 2026

Copy link
Copy Markdown
Member

Two commits on top, from the Crossref trial:

1. A smaller version of the same race, in compactForCommit. finishCompaction re-queues the commit only when commitPhase === 'compacting'. compactForCommit read the runs first and set the phase after. A compaction finishing between those two steps saw no phase, so it didn't re-queue the commit, while the commit had already counted it as in flight. If it was the last compaction the commit was waiting for, the commit sat until the stall sweep (30–40 min). Now, when there are too many runs, the commit sets the phase first and then reads the runs again. A compaction finishing before that second read shows up in it; one finishing after re-queues the commit itself. This costs one extra read, only for commits that need compacting. I didn't find a deterministic way to test the gap.

2. The 15-minute sweep in a backed-up queue. A claim is refreshed only when its merge starts and as pieces finish. On the full dump, thousands of pieces sit in the bulk queue at a time, so plenty of claims go quiet for 15 minutes just waiting their turn. Re-queueing those adds duplicate pieces to the same backlog, which delays everything further. Changes:

  • The sweep re-queues a session's compactions only once none of that session's claims has been refreshed within the window. If one is moving, the queue is moving.
  • A piece refreshes its claim when it starts, as well as when it finishes.
  • What it still catches: everything lost at once, like this morning's race, comes back within 15–25 minutes as before. A single compaction lost while others are still running comes back once the rest go quiet. That's a bounded delay, and nothing is lost.
  • The new test fails without the change: one claim 40 minutes quiet and another 2 minutes, then both quiet.

3. Concurrency.

  • Bulk queue max_concurrency is 20 → 100 on staging and prod (the Cloudflare cap is 250).
  • unitLimits is 32 per session → 64, and 64 per account → 96. Otherwise units would still be capped at 32 whatever the queue allows.
  • Why: in the trial, the units for 5.4M records ran at about 8 a minute with 20 slots. The full dump is about 33× that.
  • Watch D1 and R2 error rates on the first big commit after deploying the bulk Worker (wrangler deploy -c wrangler.bulk.jsonc --env staging).

Server tests 251/251, typecheck, oxlint and oxfmt clean.

@tefkah
tefkah merged commit 4afc8ff into main Oct 15, 2026
1 check passed
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.

2 participants