feat(mem-wal): record index catch-up positions and withdraw them on index change - #8263
Conversation
|
Important This PR touches the Lance format specification. Substantive changes to the format specification — the If this is a meaningful format change:
|
1e422cc to
31ac8b2
Compare
31ac8b2 to
f37f530
Compare
e84e709 to
2833110
Compare
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The safe-retirement protocol is the right direction, but five independent state-transition gaps currently let activation or coverage outlive the manifest or index state that justified it. That can make already-retired SSTables invisible or allow retirement before every selected index actually contains the rows.
Keep the activated protocol, but make activation durable across every manifest derivation and restore boundary, reject half-active manifests, and bind and validate each advance against the exact target index metadata after rebase.
Please mark this PR with the breaking-change label.
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The safe-retirement protocol remains the right direction, but six independent contract gaps still let activation or coverage outlive the exact manifest or index state that justified retirement, while the durable format specification does not describe the new mode. These gaps can make retired SSTables invisible or allow retirement before every selected index contains the rows.
A viable revision must make activation irreversible across every manifest derivation and restore path, reject half-active states, bind and validate each advance against the exact post-rebase target metadata, and publish the feature bit plus its legacy/safe semantics in the stable format specification.
Please mark this PR with the breaking-change label.
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
Allowing a coverage-only repair to survive concurrent appends is consistent with the per-generation catch-up contract, but six independent gaps still let activation or coverage outlive the exact manifest or index state that justified retirement, while the durable format specification does not describe the new mode. These gaps can make retired SSTables invisible or allow retirement against an unusable target index.
A viable revision must make activation irreversible across every manifest derivation and restore path, reject half-active states, bind and validate each advance against the exact post-rebase target metadata, apply target validation per advance, and publish the feature bit plus its legacy/safe semantics in the stable format specification.
Please mark this PR with the breaking-change label.
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The naming is clearer, but six independent gaps still let required-catch-up state or catch-up positions outlive the exact manifest or index state that justified retirement, while the durable format specification does not describe the new mode. These gaps can make retired SSTables invisible or allow retirement against an unusable target index.
A viable revision must make the mode irreversible across every manifest derivation and restore path, reject half-active states, bind and validate each advance against the exact post-rebase target metadata, apply target validation per advance, and publish bit 128 plus its legacy/required semantics in the stable format specification.
Please mark this PR with the breaking-change label.
06ec148 to
7167f89
Compare
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The feature-bit persistence and exact index-fragment fences close the earlier gaps, but two state boundaries can still publish catch-up semantics that were never established: generations are validated against a different snapshot than the index, and restore can normalize an invalid historical flag pair.
Bind every claimed generation to the same inspection snapshot used to build the index (or retry across concurrent compaction), and validate the historical manifest before restore republishes it.
Please mark this PR with the breaking-change label.
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The implementation now closes the previously identified snapshot-binding and restore-validation gaps, but those two correctness fixes remain unprotected by committed regressions. Add the direct conflict-fence and persisted-restore tests so these durable-data boundaries cannot silently reopen.
Please mark this PR with the breaking-change label.
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The catch-up protocol itself is the right safety boundary, but leaving it behind an opt-in migration means newly initialized MemWAL tables still use the legacy “missing means caught up” behavior, and no production caller enables the new mode.
MemWAL is explicitly experimental, so the viable direction is to make the current initializer emit the feature bit and conservative missing-entry semantics by default, while treating pre-bit experimental tables as unsupported or recreatable instead of preserving a second legacy mode.
Please mark this PR with the breaking-change label.
|
|
||
| /// Require a recorded index catch-up before an SSTable stops being served. | ||
| /// | ||
| /// Until this is called, a missing index-coverage entry reads as "fully |
There was a problem hiding this comment.
Making this a separate activation step leaves the current write path on the unsafe contract: InitializeMemWalBuilder::execute still creates default MemWAL details without setting the feature bit, and only tests invoke this method. A newly initialized table can therefore compact rows, leave index_catchup absent, and have that absence interpreted as fully caught up—the root cause this change is meant to remove. Since the format is explicitly experimental and the repository contract forbids migrations or fallbacks for earlier unstable revisions, set both bits during current MemWAL initialization and make absence conservative there; reject or recreate old no-bit experimental tables instead of requiring callers to opt in.
There was a problem hiding this comment.
As this feature is still experimental, we may still make a few major changes therefore setting it as default is more risky, we will revisit this once we are more settled with the implementations
SSTables may only be retired once every index a query can rely on is proved to contain the compacted rows. This adds the saved protocol and the commit-time enforcement that keeps that proof honest. Feature bit. FLAG_MEM_WAL_SAFE_RETIREMENT (128, moving FLAG_UNKNOWN to 256) is set in both the reader and writer words. A reader without it treats a missing index_catchup entry as "fully caught up" and could answer an index-only query without the SSTables holding the newest rows; a writer without it would change an index without invalidating the coverage that index had proved. The bit is preserved across apply_feature_flags rather than derived there: a Manifest holds only a byte offset to its index section, so it cannot see the MemWAL details, and deriving it would let an unrelated commit silently downgrade the table to legacy semantics. Coverage advance. CreateIndex gains IndexCatchupAdvance, naming one logical index, the exact final segment UUID set the repair expects, and the per-shard generations it captured when it opened the table. Publishing it on the index operation means the index result and the coverage it proves land in one commit and can never disagree. The expected segment set is the fence against a concurrent reindex: if the index is not the one the repair built, the claim is refused rather than attached to someone else's index. Central invalidation. apply_mem_wal_index_coverage runs once the final index list is known, so it sees exactly what the commit publishes rather than what any one operation arm intended. An unchanged logical index keeps its entry, a dropped one loses it, and a changed one loses it unless this transaction proves what the new index covers. Ordinary create, reindex, append, replacement and remap therefore lose coverage conservatively -- they may well have covered the compacted rows, but they do not say so. Dropping coverage is always safe, since missing coverage schedules a repair and retains SSTables; wrongly keeping it is not. Putting the rule here rather than in each caller means an ordinary index job cannot forget it. Coverage is also refused when it exceeds the recorded compaction progress for that shard, or names a shard with no progress at all: either would let the WAL pod retire SSTables that no commit copied into the base table. Activation. UpdateMemWalState gains a one-way activate_safe_retirement flag, applied after apply_feature_flags so it survives that reset. It refuses a table that already carries beta-protocol compaction progress, which was never an active retirement proof and which Lance cannot validate against WAL shard manifests; such a table must be drained through an explicit migration instead. Absence is an ordinary progress update and the flag is only written when true, so ordinary updates stay byte-identical on the wire. The conflict resolver carries advances through a rebase untouched, because apply-time validation re-checks the expected segment set against the rebased final index list. Lance stores and validates progress here; it does not decide which SSTables may be retired. There is no trim-eligibility or retirement-frontier helper: that calculation belongs to the WAL pod.
… system index Three gaps left by the coverage protocol, none of which change behaviour on a table that has not activated safe retirement. is_index_caught_up read a missing index_catchup entry as "fully caught up". That is right only for a legacy table; with the safe-retirement bit set, a missing entry means the index has proved nothing, so its SSTables must be retained and a repair scheduled. The accessor cannot see the manifest and so cannot make that distinction itself, so it is renamed is_index_caught_up_legacy -- callers select the semantics from the feature bit, and the name now says which reading they are getting. drop_index refused nothing, so an ordinary index drop could remove __lance_mem_wal and with it the compaction progress and coverage the WAL pod retires SSTables against. The table would then claim nothing had ever been compacted while those SSTables were already gone. Dropping the system index is now refused; disabling MemWAL is a dedicated operation. The table.proto comments for IndexCatchupProgress and index_catchup still documented "absent means fully caught up" unconditionally. They now state that this holds only for legacy tables, that the manifest feature bit selects the reading, and that only the dedicated repair path may add entries -- ordinary index operations have theirs removed automatically because they do not prove what the new index covers.
The stale-generation commit test is introduced by the compaction-progress change, where UpdateMemWalState has no activation flag. This commit adds that field, so the test's construction needs it here.
Take the index list as a slice, collapse the nested early-return guard, and restore build_manifest's doc comment that the new helper displaced.
Adapts three CreateIndex sites that landed on main while this branch was open.
The advance was only routed in when the table already carried the system index, so a claim made against a table without it was dropped silently instead of refused -- the one outcome this protocol never allows.
Legacy tables no longer have coverage invalidated. A missing entry reads as "fully caught up" there, so dropping one widened coverage instead of narrowing it. Invalidation now runs only when both feature bits are set. Activation clears any coverage the beta protocol left behind, rejects a table with only one bit set, and is a real no-op when already active. It also rebases: the conflict resolver no longer refuses an activation that raced another commit, and an explicit `false` on the wire is refused rather than read as absent. Segments are compared by whole metadata rather than UUID alone, so an operation that prunes a fragment bitmap while keeping the UUID no longer keeps coverage it has invalidated. An advance now merges onto what the index already proved, keeping shards it does not name and taking the per-shard maximum, so repairing one shard cannot erase another or move coverage backward. An index that changed carries nothing forward. A coverage-only commit publishes no index work, so it must now show the named index spans every live fragment before its claim is recorded. Unchanged commits no longer rewrite the system index, which kept minting a UUID and dropping the decoded-details cache on unrelated commits. Both new operation fields are compared in `Operation` equality. Adds `activate_mem_wal_safe_retirement` so callers stop hand-building the activation transaction.
The fields already speak the standard replication vocabulary (index_catchup, caught_up_generations), while the comments called the same thing a proof. Proof is formal-methods wording and oversells what a writer does here: it reports how far an index has caught up. Renames one test to match.
The advances comment named a component that does not exist in this repo, and listed "append" among the operations that clear an entry -- readable as Operation::Append, which is a data append and clears nothing. Says which operations change an index, and that what they clear is the index_catchup entry.
The check required the index to span every live fragment, compared against the fragment list the commit publishes. Any append landing while the repair ran put a fragment there that the index could not cover, so the repair failed -- on a table under continuous writes, always. The claim an advance makes is only about generations already compacted. Fragments appended since are a later catch-up gap, the same way a newer concurrent compaction is. What still gets rejected is a target that can back no claim at all: a segment with no fragment bitmap, or an index covering nothing that still exists.
Every other flag in feature_flags.rs names the format element it controls -- deletion files, stable row ids, table config, base paths. This one named a consequence instead, and read as though the other mode were labelled unsafe. FLAG_MEM_WAL_SAFE_RETIREMENT -> FLAG_MEM_WAL_INDEX_CATCHUP, matching the index_catchup field and the caught_up_generations vocabulary already in the data model. The operation field becomes require_index_catchup: "require" has no natural inverse, which is what a one-way migration should read like. Field number 2 is unchanged, so nothing moves on the wire.
The feature bit did not survive an ordinary commit. `new_from_previous` zeroes both feature words, and the preservation logic read them back from that zeroed destination, so it preserved nothing -- only the activating commit itself carried the bit, and the next append or config change dropped the table to legacy semantics. Inheritance now reads the source manifest, at both points where a manifest is derived from another: build_manifest and restore. Restore previously republished a historical manifest verbatim, which walked an activated table backwards. A half-set state is refused rather than folded into both bits. One bit set means a legacy reader or a legacy writer is still permitted, which is neither mode. An advance now declares the fragments it saw the index cover. UUIDs alone were not a fence: pruning narrows a segment's bitmap while keeping its UUID, so a claim made before the prune still matched afterwards. Whether a commit publishes index work is decided per advance rather than once per transaction. Creating one index no longer excuses a different index named by another advance in the same commit. Adds logs where state changed silently: catch-up being invalidated, which is what to check first when SSTables stop becoming trimmable; the one-way migration; and restore keeping a bit the restored version never had.
Three cases that must keep working untouched: a table with no MemWAL index, a MemWAL table that has not migrated, and an index job on a migrated table, which loses its catch-up entry but still commits.
The spec listed feature bits only through 16 and said 32 and above are unknown, though 32 and 64 already exist, and said an index absent from index_catchup is always fully caught up -- which the new bit inverts. Another implementation had no way to decide whether to reject bit 128 or how to read an absent entry.
…apshot The bit still never persisted. `write_manifest_file` runs `apply_feature_flags` a second time after `build_manifest`, so removing preservation from that function cleared the bit immediately before the write: activation returned success and the stored table stayed legacy. It is carried across the reset again -- the same way the file already handles FLAG_STABLE_ROW_IDS -- and inheritance stays for the boundaries where a manifest is derived from another and starts with zeroed words. Shallow clone was a third such boundary and zeroed them too. An advance now declares the fragments live when the repair inspected the table, and must cover all of them. Nothing maps a generation to the fragments its rows landed in, so covering the inspected table is how a claim is tied to the generations it names; "covers something live" was not. Fragments appended since remain a later gap. This applies to every advance, which removes the per-transaction question of whether index work was published -- publishing one index was never evidence about another, and removing a segment was never evidence at all. Restoring an activated table to a version from before the migration is refused. That version's catch-up and compaction values were never validated by this protocol, and keeping the bit would republish them as if they had been. Also: optimize_indices can now record catch-up, building the advance from the segments it publishes since the caller cannot name them in advance; a half-set manifest is refused on read, not only on commit; activation writes back to the receiver instead of leaving it stale; and the bitmaps are counted in transaction memory.
An advance describes the table as the repair saw it, but the generation bound was checked against compaction progress at commit time. A rebase over a commit that advanced that progress moved the bound: rows from the newly compacted generations landed in fragments the repair never inspected, and a claim naming them passed both checks. Such a rebase is now refused, so the repair re-plans against the newer state rather than claiming generations its index does not cover. Restore reads the historical manifest below the reader validation boundary, so a half-set one reached the flag reset and was quietly republished as legacy. It is validated where it is read. Adds the round-trip this needed all along: activate, reopen from storage, commit, reopen again. Testing build_manifest alone is what let the bit be cleared by the second apply_feature_flags twice over. Reintroducing that bug fails this test. Invalidation logs at info: it is the trail for why SSTables stopped becoming trimmable, not a debug detail.
A repair whose index already covers every fragment rebuilds nothing, and optimize_indices returned before it could record anything. That is the ordinary case after a remap -- coverage was dropped because the segment changed, while the index still spans the table -- so catch-up stayed missing, the agent rescheduled the same repair forever, and the SSTables never retired. The advance is now built before that return, driven by the requested index names rather than by whatever was rebuilt. A claim may not exceed what the version this call read had already compacted. Anything compacted since landed in fragments this call never inspected, so its rows are not in the index being published. Checked where the advance is built, which is where the read version is still known. Reverts two things from the previous commits. Refusing to rebase an advance over a commit that advanced compaction progress would have rejected repairs on any table whose shards compact, which is all of them: an advance names every shard, so ordinary compaction anywhere would starve the repair. And refusing to drop the MemWAL system index broke lancedb's unset_lsm_write_spec, which drains the writers and then drops it on purpose; that guard was unrelated hardening for a hazard that predates this protocol, and pointed at a dedicated disable operation that does not exist.
7d1698b to
894c50b
Compare
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The catch-up protocol itself is the right safety boundary, but leaving it behind an opt-in migration means newly initialized MemWAL tables still use the legacy “missing means caught up” behavior, and no production caller enables the new mode.
MemWAL is explicitly experimental, so the viable direction is to make the current initializer emit the feature bit and conservative missing-entry semantics by default, while treating pre-bit experimental tables as unsupported or recreatable instead of preserving a second legacy mode.
Please mark this PR with the breaking-change label.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
jackye1995
left a comment
There was a problem hiding this comment.
Approved.
Non-blocking future hardening: if a production path is added that refreshes __lance_mem_wal by publishing a replacement CreateIndex, its rebase should reconcile the current index_catchup map (including withdrawals and concurrent advances), or retry when that state changed. Otherwise a stale system-index refresh could republish a catch-up position withdrawn by a concurrent user-index replacement. This is not blocking for the current experimental scope.
## What `Operation::UpdateMemWalState` builds its manifest from scratch and never populates `final_fragments`, so the commit publishes a manifest with **no fragments**. Every row in the table disappears. Nothing errors. The operation touches indexes rather than data, so it is declared compatible with concurrent `Append` / `CreateIndex` and no conflict is raised — the commit succeeds and the data is gone. Compare `UpdateBases`, the arm immediately below: also index/metadata-only, and it does carry the fragment list forward. ## Fix ```rust final_fragments.extend(maybe_existing_fragments?.clone()); ``` ## Reachability No caller in this repo commits a standalone `UpdateMemWalState` today. MemWAL compaction progress is recorded by `MergeInsertBuilder::mark_sstables_as_compacted`, which rides an `Operation::Update` — a data operation that populates fragments normally. So the bug is latent here, not active. It becomes reachable the moment anything commits the operation on its own, which the MemWAL index catch-up work (#8263) does. ## Tests `test_update_mem_wal_state_preserves_fragments`: commit `UpdateMemWalState` on a dataset with rows, assert the fragment list and row count are unchanged. Verified it fails without the fix (`left: []`, `right: [0]`). The existing tests in `index/mem_wal.rs` already commit this operation against a dataset holding 10 rows — they pass because none of them read the rows afterwards, asserting only on conflict behavior and `compacted_sstables`. `cargo test -p lance --lib -- mem_wal transaction` — 636 passed.
…8481) Replaces the `IndexCatchupAdvance` mechanism from #8263. A commit no longer carries a claim about index coverage; the coverage is derived from the version the transaction read. ## Why Under #8263, a worker that extended an index had to describe what it had done — the index name, the exact segment UUIDs it expected to publish, the fragments those segments covered when it looked, and every fragment live at that moment — and the commit re-validated all of it. Four fields and a validation pass to transmit a fact the commit can already see. It can see it because coverage has only one possible proof. Nothing maps a compaction generation to the fragments its rows landed in. The only way an index can show it holds those rows is to span the table as the transaction read it. So rather than accept a claim and check it, derive it: an index whose segments together cover every fragment live at `read_version` is caught up to that version's `compacted_sstables`. Three things follow that the advance model could not offer: - **A claim cannot go stale.** There is no window between inspecting and committing, because there is nothing to inspect. - **The answer survives rebase.** `read_version` is fixed for a transaction's life, so every commit attempt derives the same result. #8263 needed the advance carried through the rebase untouched and re-validated. - **Any operation that commits can earn coverage.** An ordinary index build that happens to cover the table records catch-up as a side effect. Under #8263 only a dedicated repair could, so a build that fully covered had to throw the fact away and wait for a repair to re-establish it. Because the position is only written by a commit, `optimize_indices` keeps committing on an activated table even when it has no new segment to publish. ## What it keeps from #8263 The parts that were not about transmission: - `FLAG_MEM_WAL_INDEX_CATCHUP`, both words, and the refusal of a half-set state - `index_catchup` on `MemWalIndexDetails`, and the reader rule that a missing entry means "not caught up" - Activation (`require_index_catchup`), including its refusal of a table that already carries beta-protocol compaction progress - Withdraw-on-change: an index this commit changes keeps no position it cannot re-earn Two rules bound what a commit may record. It never credits past its own `compacted_sstables`, so a read version since rolled back cannot retire SSTables no live commit copied in. And it never lowers a position an index already held, provided the index is unchanged. "Unchanged" compares whole segment metadata, not segment UUIDs. `Operation::Update` prunes a segment's fragment bitmap in place when it touches an indexed field, keeping the UUID — so a UUID-only comparison carries a position forward for an index that now covers less. That is not hypothetical; it is reachable from an ordinary merge-insert. `a_bitmap_pruned_in_place_does_not_keep_its_position` pins it. ## What it removes `IndexCatchupAdvance` and its proto message, the `mem_wal_index_catchup_advances` field on `CreateIndex`, `OptimizeOptions::mem_wal_index_catchup`, the advance-validation pass, and the rebase handling that carried an advance through. ## Tests 34 unit tests over the derivation, in `dataset/transaction.rs`, and 6 through a real commit, in `index/mem_wal.rs`. The derivation alone is not the feature — `commit_transaction` has to load the read version and hand it down, and only for tables carrying the bit — so the commit-path tests cover that an index earns coverage, a legacy table earns none, and a rebase past an append does not move what a commit earns. Twelve fences were regressed one at a time and the failing test confirmed. Four of the first attempts caught nothing, because the test asserted an outcome that both the correct and the broken path produce; each was replaced with one that discriminates. One guard is deliberately untested: skipping the read-version index load on legacy tables is a cost guard, not a correctness one, and regressing it changes no observable behaviour. `cargo test -p lance --lib`: 2914 passed. fmt and clippy clean. ## Follow-ups - A user index build cannot rebase past an `UpdateMemWalState` commit — only the system index may, anything else is rejected outright rather than retried (`conflict_resolver.rs`, unchanged since January). Now that an ordinary build can earn coverage, that race is worth revisiting: it costs a completed build. - `segments_before` still clones every index segment on each commit for tables on the protocol. The snapshot has to be owned because the operation rewrites the list, but a smaller snapshot would do.
#3780) `exclusion_watermarks` resolved a single index and capped SSTable exclusion at that index's catch-up watermark. It now takes every index the query relies on and retains to the **lowest** of them, and the resolver collects arms together rather than returning at the first match. This is groundwork, not a fix for a reachable bug: `reject_unsupported` refuses hybrid search, so the vector and full-text arms are mutually exclusive and the list never holds more than one entry today. The generalisation is what the remaining work below plugs into. Unchanged: a plain scan uses the compaction watermark alone, an index with no catch-up entry contributes no cap, and a caught-up index falls back to the compaction watermark. Taking a minimum over more indexes can only lower a watermark, so the failure direction is "read an SSTable unnecessarily", never "miss rows". ## Tests Three in `lsm`: the existing lagging-index test updated for the new signature; `exclusion_watermark_takes_the_minimum_across_every_index_used` (two indexes at 7 and 4 against compaction at 9 — each alone stops at its own watermark, together the lower governs, order-independent); and `an_untracked_index_does_not_widen_a_lagging_sibling`. `cargo test -p lancedb --lib` — 45 lsm tests, 484 in the crate. `cargo fmt --check` clean. ## Follow-ups This crate pins lance to a released tag, so anything needing unreleased Lance symbols waits for a bump. 1. **Select legacy versus strict semantics from the feature bit.** On a table with `FLAG_MEM_WAL_INDEX_CATCHUP` set, a *missing* entry must mean "not caught up" and retain the SSTables, instead of leaving the compaction watermark unchanged. Needs the bit from lance-format/lance#8263. **This must land before any table is activated** — otherwise the bit is set while queries still read permissively. 2. **Collect scalar and bitmap-family prefilter indexes.** The genuinely multi-index query is a vector search with a scalar prefilter, and it is gated on the vector index alone today. Identifying the others needs the planner's chosen indexes, not the columns the filter names, so it needs a Lance-side helper. 3. **Verify a retained SSTable can actually answer.** Both base and SSTable arms use `fast_search`; a source without a compatible index contributes nothing, so retention alone does not guarantee its rows are returned. Needs a flat-search fallback or an explicit error in Lance's `LsmScanner`. 4. **Planner-level integration tests.** Current tests exercise the watermark arithmetic directly. End-to-end coverage over real queries — prefilter forms, legacy versus activated, missing index and missing shard entries — depends on 1–3.
MemWAL SSTables may only stop being served once every index a query relies on is known to contain the compacted rows. This adds the recorded protocol and the commit-time enforcement that keeps those catch-up positions honest.
Nothing consumes it yet: the read side is lancedb#3780, and no table sets the bit until something explicitly asks for it.
Feature bit
FLAG_MEM_WAL_INDEX_CATCHUP(128, movingFLAG_UNKNOWNto 256), in both the reader and writer words. Without it a missingindex_catchupentry reads as "fully caught up"; with it, as "not caught up". A writer needs it too, since one that doesn't maintainindex_catchupcan change an index without withdrawing the position recorded for it. A half-set state is refused rather than completed — one bit set means a legacy reader or writer is still permitted, which is neither mode.The bit isn't derivable from a manifest, and it's dropped at two different kinds of boundary, so keeping it takes two things:
apply_feature_flagsresets both words and runs twice per commit (build_manifest, thenwrite_manifest_file), so it carries the bit across its own reset — the same way that file already handlesFLAG_STABLE_ROW_IDS.new_from_previous,shallow_cloneand restore each derive a manifest whose words start zeroed, so there's nothing there to carry.inherit_mem_wal_index_catchupmoves it from the source.Coverage advance
CreateIndexgainsIndexCatchupAdvance: one logical index, the exact segment set expected, and the per-shard generations captured when the repair opened the table. Publishing it on the index operation means the index result and its catch-up land in one commit and can't disagree.It also declares two things about the moment of inspection:
A claim also may not exceed what the version the repair read had already compacted, checked where the advance is built.
Central invalidation
Runs once the final index list is known, so it sees what the commit publishes rather than what an operation arm intended.
Only on a table that requires catch-up. On a legacy table a missing entry means "caught up", so removing one there would widen coverage.
"Unchanged" compares whole segment metadata, not UUIDs:
Rewriteprunes a bitmap while keeping the UUID. Ordinary create, reindex, append, replacement and remap therefore lose their position conservatively — they may well cover the rows, but they don't say so.An advance merges onto what the index already recorded: unnamed shards keep their generation, named ones take the higher of the two. So repairing one shard can't erase another, and a delayed retry can't move a position backward. An index that changed carries nothing forward.
Commits that change nothing leave the system index alone, so unrelated commits don't mint a UUID and drop the decoded-details cache.
Activation
UpdateMemWalStategains a one-wayrequire_index_catchup. It refuses a table carrying beta-protocol compaction progress, which this protocol never validated, and clears any betaindex_catchupfor the same reason. Restoring an activated table to a pre-migration version is refused on the same grounds. Repeating the call is a true no-op that keeps every recorded generation. An explicitfalseis refused, so no caller can express "deactivate".Dataset::require_mem_wal_index_catchupis the entry point.OptimizeOptions::mem_wal_index_catchupis how a repair records catch-up alongside the index work it publishes — the advance is built there, since a caller can't name segments that don't exist yet, and it's recorded even when the index needed no rebuilding. Initializing MemWAL deliberately does not activate: a table stays on legacy semantics until something can actually repair coverage.Scope
Lance stores and validates positions; it does not decide which SSTables may be retired. There is deliberately no trim-eligibility helper — that belongs to the component that owns the SSTables.
Tests
36 in the coverage module, 19 in
index::mem_wal, covering invalidation, advance validation, activation, and the protobuf round trip. Every test for a fix here was confirmed to fail against the unfixed code.Two matter more than the rest. A persisted round trip — activate, reopen from storage, commit, reopen — because testing
build_manifestalone is what let the feature bit be cleared by the secondapply_feature_flagstwice over. And three that pin that nothing else changes: a table with no MemWAL index, a MemWAL table that hasn't migrated, and an ordinary index job on one that has, which loses its position but still commits. With no advances the coverage code has no error path, so an ordinary index job can never be blocked.cargo test -p lance --lib— 2881 passed;-p lance-table— 180.cargo fmt --all --checkclean. Clippy as CI runs it reports no new findings.Format spec
versioning.mdlisted bits only through 16 and said 32 and above are unknown, though 32 and 64 already exist; 128 is now documented alongside them.mem_wal.mdsaid an absentindex_catchupentry always means caught up; it now states both readings, which bit selects them, and what an advance must declare.Known limitations
Positions are dropped more often than strictly necessary. A remap preserves what an index covers but changes its metadata, so the entry goes and a repair re-records it. Deliberate: the alternative is a list of known-narrowing paths that a later operation could silently fail to join.
An honest map is not on its own enough to retire an SSTable. Invalidation stops the next retirement from acting on a withdrawn position; it cannot undo one that already happened. Closing that needs a guard at commit time or a margin on the retirement side, and belongs with the component that owns the SSTables.