feat(master): rebuild the id ZoneMap index after every compaction that rewrote fragments - #273
Merged
Merged
Conversation
…t rewrote fragments IndexId has been a task kind since lance-format#194 but nothing ever enqueued it except a manual POST /tasks, so in practice no store had an id index (prod: 0 IndexId tasks in 15 h across 10k stores). The ZoneMap is per-fragment min/max, and compaction replaces exactly those fragments, so the natural trigger is a compaction that added fragments: enqueue an IndexId that depends on the compaction task. The dependency reuses the per-target lock ordering and lets the index wait behind the queue like any other task. Off when nothing was rewritten. INDEX_AFTER_COMPACTION (default true) turns it off. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
beinan
added a commit
that referenced
this pull request
Sep 29, 2026
… workers merge (#277) ## Problem At 2026-09-29 22:12 UTC, 18 of 20 production workers were OOMKilled within 66 seconds. The merge sweep had queued 184 targets (normal: 20–50) and every worker was running ~24 concurrent `merge-wal` requests. The memory was not the WAL rows (the #265 budget bounds those). It was `merge_insert`. Lance's merge_insert joins the source rows to the base table on the key, and it probes a scalar index only when that index's plugin `provides_exact_answer()`; otherwise `create_full_table_joined_stream` reads the **whole base table** into a DataFusion hash join. Our `id` index was a **ZoneMap**, whose plugin returns `false`, so every merge of every store did the full join. The largest affected store has a 1.95 TB, 5.8M-row base table: each 64-generation merge (a few hundred rows) read 2 TB. That also explains the trend line: pending generations went from 17k to **426k** in 24 h, 40 stores over 4k pending. Merge cost scaled with base-table size rather than with the rows merged, so the large stores could not be drained, so more merges queued, so more full joins ran concurrently. #273 (index after compaction) did not help because it built a ZoneMap. ## Fix - `create_key_zonemap_index` → `create_key_btree_index` (`IndexType::BTree`). New `has_key_btree_index()` says whether the exact-answer index is present; a ZoneMap under the same name does not count. - A `merge-wal` task on a rollout store without the BTree builds it before fanning out to the workers (`INDEX_BEFORE_MERGE`, default on; one manifest read when already present). This is what rescues the stores already in trouble: the very next merge probes. #273 keeps the index current after each compaction. - Master detail string and metrics renamed accordingly; `master_merge_wal_index_built_total` counts pre-merge builds. A BTree on a string `id` is larger than a ZoneMap (it stores every key), tens of MB for 5.8M rows. That is the cost of a merge that reads MBs instead of TBs. ## Verification - **`merge_insert_probes_the_id_btree_instead_of_scanning`** (core): builds a store, then asks Lance's `MergeInsertJob::explain_plan` which path a merge would take. With no index it renders a plan containing `HashJoin`; **with a ZoneMap it still renders `HashJoin`**; with the BTree it refuses with the scalar-index `NotSupported` (explain only renders the full-scan plan), which is the observable signal for the indexed path. A real merge through the index then succeeds. - `create_id_btree_index_builds_and_is_idempotent`: asserts `has_id_btree_index()` false before / true after, idempotent rebuild. - `merge_wal_builds_the_id_btree_first` (master, etcd): a merge on an unindexed store leaves a BTree behind; a second merge does not bump the version. - Master suite `--include-ignored` 75/75; `storage_reliability` 10/10; clippy `-D warnings`; fmt. ## Rollout note Prod is holding on `MERGE_WAL_CONCURRENCY=1` (env) since the incident. Once this ships, large stores get a BTree on their first merge, after which merge memory is independent of base-table size and the concurrency can go back up. 🤖 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
TaskKind::IndexId(a ZoneMap scalar index on the base table'sidcolumn) has existed since #194, but nothing ever enqueues it except a manualPOST /api/v1/tasks. In production that means zeroIndexIdtasks in 15 h across ~10k stores, so point lookups onidscan every fragment.Fix
A compaction that added fragments enqueues an
IndexIdfor the same target withdepends_on = [compaction task id]:compact_innernow returns theCompactionMetricsinstead of a formatted string so the caller can make that decision. New flagINDEX_AFTER_COMPACTION(defaulttrue) turns it off. Enqueue failure is logged and does not fail the compaction, which is already committed.Verification
compaction_that_rewrites_fragments_enqueues_id_index: a 4-fragment store is compacted; anIndexIdwith the compaction as its dependency appears and runs toDonewith the expected detail; a second compaction that rewrites nothing enqueues no second index.--include-ignored): 72 passed. Clippy-D warnings, fmt clean.🤖 Generated with Claude Code