Skip to content

fix: a merge extends a stale key BTree, not just builds a missing one - #286

Merged
beinan merged 2 commits into
lance-format:mainfrom
beinan:fix/merge-extends-stale-index
Oct 1, 2026
Merged

beinan merged 2 commits into
lance-format:mainfrom
beinan:fix/merge-extends-stale-index

Conversation

@beinan

@beinan beinan commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Problem

#282 builds the key BTree before the delete-only merge_insert when the base has no index. But an index that exists and covers only some fragments is just as bad: merge_insert hash-joins every fragment the index does not cover.

Production, 00:55 UTC: bp-prod2rubric-…-ghcp-100 (2.2 GB base, 65 fragments, BTree built when it had 5) — a merge of 64 generations totalling 37 MB took a worker from 5 to 32 GiB and OOMKilled 19/20 workers that merged it concurrently. Reproduced deterministically on one worker (5 → 20+ GiB → killed). After a single optimize_indices the identical merge on the identical worker peaked at 3 GiB and finished in 39 s.

Fix

In merge_prepared_batches: if the base has fragments and no BTree, build it (unchanged); otherwise call extend_key_btree_index so the index covers everything appended since — the same thing the master does before fan-out (#278), now also on the worker's own merge path (timer, manual route). New counter rollout_merge_index_extended_on_demand_total.

Verification

  • New merge_extends_a_stale_id_btree_over_new_fragments: after four merges at most one fragment (the one just appended) is uncovered.
  • extend_id_btree_index_covers_fragments_added_since_build updated to the new invariant (1 uncovered after two merges, not 2).
  • Full core suite passes; clippy clean.

🤖 Generated with Claude Code

beinan and others added 2 commits October 1, 2026 01:35
lance-format#282 builds the key BTree before the delete-only merge_insert when the
base has none. Present is not enough: every merge and compaction appends
fragments the index does not cover, and merge_insert hash-joins exactly
those. A 2.2 GB store whose BTree covered 1 of 65 fragments took a worker
from 5 to 32 GiB on a 37 MB merge and OOMKilled 19 of 20 at once; after
one optimize_indices the same merge on the same worker peaked at 3 GiB.

The worker now does what the master already does before fan-out (lance-format#278):
build the index if missing, otherwise extend it over the unindexed
fragments. Counted as rollout_merge_index_extended_on_demand_total.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
append() adds one index delta per merge. Compaction refuses to bin
fragments covered by different index-delta sets together, so with a
delta per merge every fragment sat in its own bin and nothing ever
coalesced (three compaction tests caught it). merge(1) folds the new
fragments into the existing delta, so the index stays a single delta.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@beinan

beinan commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up commit: extend_key_btree_index now uses OptimizeOptions::merge(1) instead of append(). With one index delta per merge, Lance's compaction planner refused to bin fragments covered by different delta sets together, so nothing coalesced — compact_reduces_fragments_and_preserves_reads, compaction_folds_merged_fragments_and_preserves_reads and repair_drops_fragments_whose_files_are_missing caught it. Full core (250) and master (78, incl. etcd) suites pass locally.

@beinan
beinan merged commit 7c86651 into lance-format:main Oct 1, 2026
10 checks 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.

1 participant