Skip to content

fix: merge the WAL as delete-only merge_insert + append, not an upsert - #280

Merged
beinan merged 1 commit into
lance-format:mainfrom
beinan:fix/merge-delete-then-append
Sep 30, 2026
Merged

beinan merged 1 commit into
lance-format:mainfrom
beinan:fix/merge-delete-then-append

Conversation

@beinan

@beinan beinan commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator

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:

  • a merge matching 17k rows read 7.6 GB (459 KB/row); one matching 1.4k rows read 4.0 GB (3 MB/row)
  • two such merges in flight held a 32 GiB worker at its limit — cgroup anon 27–30 GiB, dropping to 8 GiB within two minutes of the merges finishing
  • 8 workers OOMKilled in two hours on the fix: stop reads and merges from opening the whole shard at once #278 image even with merges bounded to two per worker

Fix

merge_prepared_batches is now two commits:

  1. a delete-only merge_insert on the key column (WhenMatched::Delete, WhenNotMatched::DoNothing) — probes the id BTree, touches only the key column and deletion vectors;
  2. a plain Dataset::append of the new rows into fresh fragments.

No target blob is ever read. Last-write-wins is unchanged (read_flushed_generations already 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_STACK and the same variable in the CI workflow env.

Verification

  • New 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.
  • Full core suite (247 tests), server, master (incl. etcd --ignored) and lance-context suites pass; clippy clean.

🤖 Generated with Claude Code

`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
beinan force-pushed the fix/merge-delete-then-append branch from 37b1bb3 to e5a9763 Compare September 30, 2026 12:59
@beinan
beinan merged commit 86f0829 into lance-format:main Sep 30, 2026
10 checks passed
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>
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.

1 participant