fix(core): build the key BTree before merging into an unindexed base table - #282
Merged
Merged
Conversation
…table The delete-only merge_insert (lance-format#280) probes the key index. Without one it is a hash join over the whole base table: on a 174 GB / 67k-row store that no master had ever indexed, every worker that tried to merge it was OOMKilled within 15 s. Building the BTree first took 19 s and the merge then peaked at 9 GiB. The master builds the index before fan-out (lance-format#277), but a worker's own timer or a manual merge-wal call must not depend on the master having visited the store. `merge_prepared_batches` now builds the BTree when the base has fragments and no exact-answer key index; an empty base has nothing to join and is left alone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With the merge now building the id BTree on an unindexed base, the first fragment is indexed and the rest are not; Lance compacts indexed and unindexed fragments in separate groups, so reaching one fragment takes two passes. Assert the real invariant: every pass that rewrote fragments enqueued exactly one IndexId, and the final no-op pass none. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
beinan
added a commit
that referenced
this pull request
Sep 30, 2026
## Problem Today `mai3_bigclimb_run6p5_77b_t0r1` sat at 17,254 pending WAL generations, every read answered 503 for ~5 hours, and no metric or log said so — a customer noticed first. The stats scanner was reusing a stale row (#281), no master had ever indexed the store (#282), and the read cap fired silently. 40+ more stores were in the same state. ## Fix One metric per blind spot: | metric | kind | meaning / alert | |---|---|---| | `master_stores_pending_over_read_cap` | gauge | stores whose reads the cap is refusing right now; **alert when > 0** | | `master_stores_pending_over_1k`, `master_wal_pending_generations_total`, `master_wal_pending_generations_max` | gauge | fleet backlog shape, refreshed every stats scan | | `master_stats_pending_recounted_total` | counter | version-unchanged shortcut found a pending count different from the row it would have reused (the #281 condition); `warn!` when growth > 256 | | `rollout_reads_refused_pending_total{store}` | counter | now labeled by store so the unreadable store is named | | `rollout_merge_index_built_on_demand_total` | counter | a merge built the key BTree because no master had (the #282 condition) | Gauges come from a small `Backlog::summarize` over the scan snapshot. ## Verification `backlog_summary_counts_stores_over_the_read_cap` (17,254 / 1,200 / 3 → total 18,457, max 17,254, 1 over cap, 2 over 1k). Existing scanner recount tests and core read-cap/merge tests pass; clippy clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
beinan
added a commit
that referenced
this pull request
Oct 1, 2026
…#286) ## 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](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
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.
Problem
The delete-only
merge_insertfrom #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-walcall — 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_batchesbuilds the key BTree (create_key_btree_index, idempotent) when the base table has fragments andhas_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.🤖 Generated with Claude Code