Repository navigation
perf(fusillade): archive large batches in resumable chunks - #1885
Open
fergusfinn wants to merge 1 commit into
Open
fergusfinn wants to merge 1 commit into
fergusfinn wants to merge 1 commit into
Conversation
Deploying control-layer with
|
| Latest commit: |
be350bf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5a012a67.control-layer.pages.dev |
| Branch Preview URL: | https://perf-archive-moves-in-chunks.control-layer.pages.dev |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
All reported issues were addressed across 17 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
A batch moved into the archive in one transaction that copied every row, deleted the originals and stamped the location. On a large batch that transaction outlives the daemon's maintenance timeout, rolls back, and the next pass starts over, so the copy is repeated indefinitely and each attempt leaves its copied rows (and their TOAST) behind as dead tuples. Move a batch in bounded chunks instead, one transaction per chunk. Each chunk copies and deletes the same rows, so every committed state keeps a row in exactly one table; the batch is `split` between chunks, a state every reader, retry and freeze path already handles, and the chunk that moves the last row stamps `archive`. Every statement of a chunk ends by the call's maintenance deadline, new chunks start only in the first half of that budget, and an abandoned chunk discards only its own rows; later passes resume from what is left. A missing or fenced partition is still reported as SkippedNoPartition after partial progress. Download pages now read the batch's bucket stamp in the same statement instead of resolving it once up front, so a download sees rows that move mid-stream into whichever week the mover chose. Claude-Session: https://claude.ai/code/session_01X51YHjxkWDVMWSdBkLj3ti
fergusfinn
force-pushed
the
perf/archive-moves-in-chunks
branch
from
September 30, 2026 15:59
cad1732 to
be350bf
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #1878. It uses #1878's maintenance deadline (
maintenance_deadline(),begin_maintenance_write_until(),bound_to_deadline()), so merge #1878 first; this PR's base then moves tomain.Problem
A batch moved into the archive in one transaction. It copied every row with
INSERT ... SELECT ... ON CONFLICT DO NOTHING, deleted the originals, then stampedlocation = 'archive'.On a large batch that transaction outlives the daemon's maintenance timeout. It rolls back, and the next pass starts again from the first row. So the copy is repeated on every pass and never finishes. Each attempt leaves all of its copied rows, and their out-of-line (TOAST) storage, behind as dead tuples in the archive partition. On a large deployment the partition's heap and TOAST storage fill with discarded copies and grow far larger than the live data. Autovacuum then spends hours on it, while the batch itself stays live.
With #1878 a failing move is cancelled on the server at the call's deadline instead of running on. It is still retried, and still discarded, on every pass.
Change
A batch now moves in bounded chunks, one transaction per chunk (
fusillade-arsenal/src/postgres/batch_archive_move.rs):SKIP LOCKED, the same as before. It re-checks the same preconditions (not deleted, not already archived, counts frozen), share-locks the week's bucket row and checks the partition exists.requests. It verifies that every row it selected was copied and deleted, or aborts.locationtosplitwhile rows remain and toarchivewhen the last row moves. The stamp uses the sameretry_versioncompare-and-swap as before.archive_batchcall takes a single maintenance deadline, which fix(fusillade): bound maintenance transactions on the server #1878 derives from the daemon's configured query timeout.begin_maintenance_write_until(deadline), and every later statement in the chunk is bound withbound_to_deadline.ArchiveOutcome::Progressed { rows }. The batch stayssplitand remains a candidate, so the next pass resumes from what is left. The daemon countsProgressedas a move (outcome="progressed") and adds its rows tofusillade_archive_moved_rows_total.SkippedNoPartitioneven after progress, so the daemon still raisesarchive_partition_missing. In both cases the batch stayssplit.(SELECT archive_bucket FROM batches WHERE id = $1), the same wayget_batch_requestsalready does. Previously they resolved it once when the stream started, falling back to the week the batch was created in. A move stamps the bucket in the same transaction as the first rows it moves, so every page sees the partition of every row its snapshot sees archived. That holds whichever week the mover chose: its own week, or the current week when its own week has been retired.Every chunk deletes what it copies, so no row is ever copied twice. The
ON CONFLICTarm stays only as a guard and does not fire on the normal path. An abandoned chunk discards only its own rows; earlier chunks stay committed.Safety argument
requestsandbatch_requests_archive. The copy and delete of a chunk commit together, and the delete removes only rows verified present in the archive.splitalready works: it is the state a retry of an archived batch leaves, with some rows archived and the rest live. Every path that reads or changes a batch's rows already handles it:get_batch_requests,get_requestsand the download streams read the union of live and bucket-pruned archive rows, whatever the location.retry_failed_requests_for_batchand the per-request retry move failed or cancelled archived rows back when location is notlive, and re-pend live ones in place.splitbatches. Candidate listing already includessplit.retry_versionunder the batch-row lock. Each chunk re-checks both under that lock, so a retry between chunks stops the move. The batch then stayssplitand is resumed after it re-freezes.location = 'archive', so a part-moved batch keeps its week alive. A fenced week stops further chunks and is reported, as above.Testing
New tests:
test_archive_moves_large_batch_in_resumable_chunks: 7 rows moved in chunks of 2, one chunk per call. After every call it asserts:get_batch_requestsandget_requestsreturn every row exactly once;splitand still listed as a candidate until the last chunk stampsarchive.It takes 4 transactions, and the archive ends up holding exactly the batch's ids, each once.
test_archive_finishes_large_batch_within_one_call_budget: one call with the normal deadline moves all 7 rows chunk by chunk and returnsArchived { rows: 7 }.test_retry_of_part_moved_batch_resumes_and_rearchives: a batch is retried after two chunks have moved. Failed rows on both sides become pending live rows, only completions stay archived, and the counts still include the archived completions. After re-freezing it finishes into the same single bucket.test_part_moved_batch_reports_a_fenced_partition: after one chunk the batch's week is fenced. The next call returnsSkippedNoPartition, and the batch stayssplitwith every row in exactly one table and readable.test_download_across_a_move_into_the_current_week: the batch's creation week is retired, so the mover routes it into the current week. An output download and a results download each start while the batch is live. With a one-item download buffer, each stream has read its first 1,000-row page but not its second when the batch moves, so the move lands exactly between pages. Both return all 1,005 rows exactly once. As a negative control, restoring the old creation-week bucket in the output stream makes this test fail with 1,000 of 1,005 rows.Unit tests for the outcome of a call that stops after progress.
Existing coverage and checks:
cargo test -p fusillade-arsenal --testsandcargo test -p fusillade.cargo fmt --checkandcargo clippy --all-features --no-deps -D warningspass for fusillade-arsenal, fusillade and fusillade-core. An offline build (SQLX_OFFLINE=true) of those crates and dwctl also passes.scripts/check_query_projections.pypasses. The existing reviewed exemption for the archiveINSERT ... SELECT r.*moves to the new file with the query.The
.sqlxoffline data is updated for the new and changed statements only.https://claude.ai/code/session_01X51YHjxkWDVMWSdBkLj3ti