dedupe: make the extent loader order-independent (Stage 2.1) - #112
Merged
Conversation
GET_DUPLICATE_EXTENTS now excludes extents whose file is a whole-file dup-group member statically, via a `filedup` CTE mirroring GET_DUPLICATE_FILES' membership test, everywhere `extents` is referenced. Previously that exclusion happened *temporally*: the whole-file pass deleted those extent rows (dbfile_remove_extent_hashes) before the extent loader ran in the same pass. Making it static means the extent load no longer depends on whole-file completion - the prerequisite for pipelining the two passes in the streaming rewrite. End state is identical (every whole-file member's extents are deleted for real anyway), pinned by the new regression test. test_extent_order_independent.py covers both a group that survives removal of its whole-file members (>=2 extent members remain) and the degenerate case where removing them drops it below 2. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The filedup membership CTE was copy-pasted between GET_DUPLICATE_EXTENTS and dbfile_count_dupe_bytes; if the flags bit or the digest+size grouping ever changed, the extent loader and the progress byte-total could silently drift (exactly the double-count the exclusion prevents). Factor the CTE body into a single FILEDUP_CTE macro composed as "with " FILEDUP_CTE ... in both places. No query-plan change; make check clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The filedup CTE (a full-table group-by over files) ran on every per-batch GET_DUPLICATE_EXTENTS load. On a 3.5M-file hashfile that materialization costs ~6 s per batch - 54 batches of producer-serialized stall while the dedupe pool sits idle, reported as the phase being stuck at 'loading duplicate extents'. Replace the set with a correlated exists probe (FILEDUP_MEMBER, backed by idx_files_digest_size) that only touches the rows the enclosing query already examines. Measured on that hashfile: one batch extent load drops 21.7 s -> 0.2 s; the once-per-phase byte count drops 13.6 s -> 9.5 s. Row-for-row identical loader output verified across four generation windows (including a full-phase window, 29639 rows). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
Field report on a 3.5M-file hashfile: the Pushed
🤖 Generated with Claude Code |
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
1 participant
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.
What & why
Prerequisite for the streaming dedupe rewrite (Stage 2).
GET_DUPLICATE_EXTENTSnow excludes extents whose file is a whole-file dup-group member statically — via afiledupCTE mirroringGET_DUPLICATE_FILES' membership test, applied everywhereextentsis referenced.Previously that exclusion was temporal: the whole-file pass deleted those extent rows (
dbfile_remove_extent_hashes) before the extent loader ran in the same pass. Making it static means the extent load no longer depends on the whole-file pass having finished — the prerequisite for pipelining the two passes (Stage 2.2/2.3). The end state is identical, since every whole-file member's extents get deleted for real anyway.Tests
New
tests/integration/test_extent_order_independent.py(btrfs; fsync-forced extent boundary liketest_extent_dedupe), written against the pre-change build first and confirmed still passing after:{A,B,C,D}must still dedupe{C,D}once A,B are excluded.make checkclean (107 tests). No schema change (query-only).🤖 Generated with Claude Code