fix: merge the WAL as delete-only merge_insert + append, not an upsert - #280
Merged
Merged
Conversation
`WhenMatched::UpdateAll` takes every matched target row with all of its columns to join and rewrite the fragment. Rollout rows carry multi-megabyte inline blobs, so a merge that matched 17k rows read 7.6 GB (459 KB/row), one that matched 1.4k rows read 4.0 GB (3 MB/row), and two in flight held a 32 GiB worker at its limit: anon 27-30 GiB, falling back to 8 GiB within two minutes of the merges finishing. Eight workers were OOMKilled this way in two hours on c5d7618 even with merges bounded to two per worker. The merge is now two commits: a delete-only `merge_insert` on the key (probes the id BTree, touches only the key column and deletion vectors) and a plain append of the new rows to fresh fragments. No target blob is ever read. Last-write-wins is unchanged and a retry after a crash between the commits converges to the same state. Debug builds of the two commits overflow the 2 MiB test-thread stack in DataFusion's optimizer walk (release fits); tests get 8 MiB via `.cargo/config.toml` and the CI env. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
beinan
force-pushed
the
fix/merge-delete-then-append
branch
from
September 30, 2026 12:59
37b1bb3 to
e5a9763
Compare
beinan
added a commit
that referenced
this pull request
Sep 30, 2026
…table (#282) ## Problem The delete-only `merge_insert` from #280 probes the key BTree. When the base table has none, Lance falls back to a hash join over the entire base table. `mai3_bigclimb_run6p5_77b_t0r1` (174 GB, 67k rows, 2.6 MB/row) had never been indexed because the master's stats row for it was stale (#281), so every worker that ran a merge on it — the master's fan-out or a manual `/internal/merge-wal` call — was OOMKilled within ~15 s. Building the BTree first took 19 s and the same merge then peaked at 9 GiB. ## Fix `StorageBase::merge_prepared_batches` builds the key BTree (`create_key_btree_index`, idempotent) when the base table has fragments and `has_key_btree_index()` is false, before the delete-only merge. An empty base table has nothing to join and is left unindexed. The master's pre-fan-out build (#277) stays; this is the worker-side guarantee that does not depend on the master having visited the store. ## Verification - `merge_builds_the_id_btree_when_the_base_has_rows_but_no_index`: first merge into an empty base builds nothing; the second merge, against a populated base, creates the BTree and rows still read back exactly once. - Full core suite (249 tests) passes; clippy clean across the workspace. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
Problem
The WAL merge uses
WhenMatched::UpdateAll. Lance's upsert takes every matched target row with all of its columns to join and rewrite the fragment. Rollout rows carry multi-megabyte inline blobs, so in production:anon27–30 GiB, dropping to 8 GiB within two minutes of the merges finishingFix
merge_prepared_batchesis now two commits:merge_inserton the key column (WhenMatched::Delete,WhenNotMatched::DoNothing) — probes the id BTree, touches only the key column and deletion vectors;Dataset::appendof the new rows into fresh fragments.No target blob is ever read. Last-write-wins is unchanged (
read_flushed_generationsalready reduces to one newest row per key). A crash between the commits leaves the old rows deleted and the new rows still in the WAL; the retry deletes nothing and appends once — same end state.Debug builds of the two commits overflow the 2 MiB test-thread stack inside DataFusion's optimizer walk (release builds fit). Tests get 8 MiB via
.cargo/config.toml[env] RUST_MIN_STACKand the same variable in the CI workflow env.Verification
merge_deletes_old_rows_and_appends_instead_of_rewriting_fragments: after overwriting one of two keys, the original fragment is kept with a deletion vector and the new row lands in a second fragment; the read returns the newer content exactly once.--ignored) andlance-contextsuites pass; clippy clean.🤖 Generated with Claude Code