fix(mem_wal): enforce visible ⟹ durable, and split the index apply from the WAL append#7791
Draft
hamersaw wants to merge 14 commits into
Draft
fix(mem_wal): enforce visible ⟹ durable, and split the index apply from the WAL append#7791hamersaw wants to merge 14 commits into
hamersaw wants to merge 14 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…ndex `insert_batches_parallel` spawned one OS thread per index on every call via `std::thread::scope`. That is sized for flush-time batches; for a handful of rows the spawn costs more than the indexing itself. Merge it into `insert_batches`, which now picks the path by size: inline on the calling thread when there is a single index or the batch is at or below `PARALLEL_INDEX_MIN_ROWS` (64) rows, and threaded above it. Atomicity is unchanged — both paths run every task and keep only the first error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`check_poisoned()` guarded the three write paths but no read path, so a writer that had been fenced by a peer or had latched a persistence failure kept serving scans. Those scans can hand out rows that are not durable and that replay will not reproduce — a divergent snapshot from a shard that is already known to be dead. Recovery is evict and reopen; until then the shard should fail closed. Guard `scan()`, `active_memtable_ref()`, and `in_memory_memtable_refs()`, mirroring SlateDB's `check_closed()` at the top of every read. `memtable_stats()` is deliberately left unguarded: a poisoned writer is exactly when an operator — and the eviction path — most needs to read its state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t re-append After `replay_memtable_from_wal` rehydrated a memtable, the durability cursor stayed at "nothing flushed". The next WAL flush therefore re-covered `[0, end)`: it appended the already-durable rows to the WAL a second time *and* re-inserted every replayed row into the in-memory indexes. None of the three indexes is idempotent. HNSW mints fresh node ids for the same row, so KNN returns it twice and burns two of the `k` slots. FTS increments `doc_count`, `total_tokens`, and every term's `df` rather than recomputing them, corrupting BM25 for the whole memtable. BTree is a multiset whose second insert sets `pk_has_overrides` permanently, disabling HNSW plan selection and WAND pruning. A full scan kept looking healthy while every index-accelerated query silently returned duplicates — and it compounded, because the WAL now held those rows twice, so the next crash replayed both copies. Stamp the durability cursor at the end of replay: the batches came from the WAL, and `insert_batches` has just re-derived the indexes over them. The index cursor already advanced itself; only durability was missing. Regression test reproduces the original report: reopen + replay + one 2-row put grew the WAL from 8 rows to 18 instead of to 10. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hard open An in-memory index configured on a column that cannot support it fails deterministically on every insert — including inserts replayed from the WAL. So once a row is durable the shard can never reopen: replay re-reads the same rows, hits the same error, and `open()` propagates it. That is a permanently down shard, and it is the failure mode that makes poison-and-replay non-terminating. Validate once at open, before a single row is accepted: FTS columns must be Utf8/LargeUtf8/Utf8View, HNSW columns must be FixedSizeList<Float32> with a non-zero dimension, every index column must exist, and composite primary-key columns must have an order-preserving key encoding. BTree needs only existence — its backend falls through to per-row `ScalarValue` extraction and accepts any type the schema can hold. Also close two related gaps: - FTS silently appended an empty batch when its column was missing from the batch, so a misconfigured index stayed empty forever while the shard reported healthy. It now errors, matching BTree and HNSW. - HNSW's runtime capacity-exhaustion errors were labelled `invalid_input`, which blames the caller for a well-formed batch. They mean the index was sized below what the memtable holds, so they are `internal`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`mark_wal_flushed` had no production callers. The two collections it maintained — `wal_batch_mapping` and `flushed_batch_positions` — were allocated per memtable and read only from tests, as was `MemTable::last_flushed_wal_entry_position` (production tracks that on `WriterState`, a different struct). `BatchStore:: is_wal_flush_complete` had no callers at all. What the L0 flush actually gates on is `MemTable::all_flushed_to_wal()`, which derives from the batch store's durability watermark and stays. The tests were calling `mark_wal_flushed` only to satisfy that precondition, so they now set the watermark directly — the same thing production does, instead of a parallel bookkeeping path that only tests could reach. The two tests that covered nothing but the deleted mapping are replaced by one that covers the surviving behaviour: `all_flushed_to_wal()` flips only once the watermark covers every committed batch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The durable watermark was meaningless across a memtable rotation. `WalFlusher` is built once per writer and its watch channel is never reset, but the targets put against it were *memtable-local* batch positions — and positions restart at 0 in every new memtable. So after memtable A flushed N batches (watermark = N), the first put into memtable B targeted position 1, saw N >= 1, and acked immediately with no WAL append at all. The first N puts into every post-rotation memtable were falsely acked as durable: `durable_write: true` silently degraded to non-durable. When B's append finally landed it sent a *smaller* value, walking the watermark backwards — so a watcher from A targeting N could miss N entirely and block until some later memtable climbed back to it. A hang, not just a stale read. Fix the coordinate system rather than the symptom: - `BatchStore` carries an immutable `global_offset`, stamped at freeze from the outgoing store's `global_end()`. It is a coordinate, not a cursor: it cannot restart and cannot move backwards. - The durability cursor moves onto `WalFlusher` as a writer-global exclusive count, and is the only thing the watch channel carries. It advances monotonically (`max`), so an out-of-order completion cannot walk it back. - `BatchStore::local_end(global_cursor)` is the single place the global-to-local subtraction is written. It saturates in both directions, and both are reachable: a cursor below a store's offset means "nothing here yet" — the ordinary state of a freshly rotated memtable — and open-coding the subtraction underflows there, which in release wraps to a huge end and makes the entire new memtable visible at once. - The per-memtable `max_flushed_batch_position` (inclusive, `usize::MAX`-sentinel) is deleted. `pending_wal_flush_*` and `all_flushed_to_wal` now derive from the global cursor. Two things fell out while wiring it up: - The put path captured `batch_store` *after* the freeze check that may rotate the memtable, pairing the new store with the old store's end position. Capture it before, next to the insert that produced those positions. - `flush_memtable` sampled the cursor before awaiting the WAL-append completion it depends on, so the L0 precondition saw a stale value. Sample it after. Regression test: with a local target, a post-rotation put acks at durable=2 while its own batch spans [2, 3) — acked, never appended. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`max_visible_batch_position` was an inclusive position starting at 0, so a cursor of 0 meant *both* "nothing is visible" and "batch 0 is visible". There was no sentinel and no `Option` to tell them apart. The window is real, not theoretical. `BatchStore::append` publishes `committed_len` on the put path, under the state lock, before the WAL flush that indexes the batch is even triggered — and that flush is a ~100ms S3 PUT on another task. So batch 0 of every memtable sat committed and readable for a full round-trip before it was indexed or durable, and only through the arms backed by the batch store: the index-backed arms, reading the same cursor, saw nothing. The tiers actively disagreed. Express the cursor as an exclusive count. `0` now means nothing, the `+1` disappears, `i < count` replaces `i <= max`, and the off-by-one becomes inexpressible. `checked_sub(1)` conversions at the consumers fall away — every site got simpler, as did `bounded_in_memory_membership`, whose `batch_count == 0` case now falls out of the arithmetic instead of being special-cased. Rename it to `indexed_count` while we are here. It has only ever been an *indexed* cursor — it is advanced at the end of `insert_batches`, once every index insert for a batch completes, and never before. Five read sites treated it as a visibility cursor, which is a separate thing that belongs to the writer. Deriving visibility from it is the next change; this one just stops the name from lying. Two things the conversion turned up: - `point_lookup` did `indexed_count().min(len - 1)`, reading the cursor as an inclusive index. It had no "nothing visible" case at all. - `test_shard_writer_e2e_correctness` passed *only* because of this bug: it wrote with `durable_write: false` and scanned, and the rows appeared because the un-advanced cursor of 0 was misread as "batch 0 is visible". A non-durable put is not yet read-your-writes — the index apply is welded to the WAL flush — so the test now writes durably, which is what it meant to test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Readers keyed off `indexed_count`, which the index insert advances itself. But the WAL append and the index apply run under one `tokio::join!`, and `join!` runs both arms to completion — it does not cancel the index arm when the append fails. So on a failed append the index arm still advanced the cursor every reader treats as visibility, publishing rows that are not in the WAL and that replay will not reproduce. `MemTableScanner::new` asserts the opposite in its own doc comment. Split the cursor in two. `indexed_count` stays the *apply* cursor: it tracks what the index layer has ingested, it is what HNSW's contiguity check needs, and it advances on a failed flush — which is fine, because indexes are derived state and replay rebuilds them. `visible_count` is new, and is the only thing readers snapshot: the writer advances it downstream of *both* arms succeeding, so `visible => durable` holds unconditionally. Also make an index-apply failure terminal. `insert_batches` joins every index thread unconditionally, so a failure leaves the others fully applied, and none of them can be rolled back: HNSW has no delete, FTS has already incremented its collection statistics, BTree has linked into an append-only skiplist. Both ways out are corrupt — retry re-covers the range and re-inserts into the indexes that did succeed; skip it and the failed index is permanently short rows the others have. So discard instead: poison, and let reopen rebuild the indexes from the WAL. The rollback primitive already exists and it is `open()`. Replay publishes explicitly: its batches are durable by construction and it bypasses the flush, so without it the recovered rows stay invisible. Regression tests pin both halves. With the index arm publishing, a row whose WAL append failed becomes readable (visible_count=1 where it must be 0). An index insert that fails deterministically now poisons the writer instead of leaving it to limp on with a corrupt index. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…the WAL flush The WAL append and the index apply were welded together under one `tokio::join!` on one schedule. They have nothing in common: one is an in-memory write measured in microseconds, the other a ~100ms S3 PUT billed per call. Batching exists to bound API cost, which an in-memory index apply does not incur — so the index was being dragged onto the WAL's schedule for no reason, and that is what made read-your-writes impossible without durability. Split them into two tasks with two channels, each a single sequential consumer: - The index-apply task is triggered per put, in both modes. Being a single consumer is the safety property: `HnswGraph::insert_batch` hard-rejects any range whose start is not its `indexed_len`, and a single consumer guarantees contiguous in-order ranges however many putters race behind it. Ordering comes from the task, not from a flush interval — so triggering per-put is exactly as safe as triggering on a timer, and a put becomes visible in milliseconds rather than waiting on an S3 round-trip. - The WAL-append task keeps the expensive work and is now append-only. The `join!` is gone, and with it the dirty read it caused: a failed append can no longer publish rows through an index arm that ran anyway. They need separate channels, not two message types on one: `TaskDispatcher::run` awaits `handle()` inline, so sharing would queue every index apply behind an S3 PUT. Visibility becomes derived rather than published. `WriterCursors` holds the writer-global `durable` count; each memtable's `IndexStore` holds its own `indexed` count; and `visible` is computed on demand as `min(indexed, durable)` under `durable_write`, or just `indexed` without it. Nothing caches it, which is deliberate: a cached `min` recomputed by two independent tasks is the classic store-buffer race — under Release/Acquire both tasks can read the other's pre-store value, both compute a minimum below the true one, and a max-clamped publish leaves it permanently short, hanging any put blocked on it. With nothing cached there is nothing to leave stale. The notify channel is a bare wake-up and every waiter recomputes. What this buys: `durable_write: false` now costs the caller durability *only*, not visibility. A non-durable put is read-your-writes through every arm, indexed ones included — previously the row sat unindexed in the batch store until some later flush happened along, so a full scan could find it while every index-accelerated query could not. Also: `close()` drains both tasks, `freeze` triggers the index apply for the outgoing store, and every memtable's `IndexStore` is bound to the cursors — including the empty one built when no indexes are configured, which would otherwise fall back to `visible == indexed` and publish before durability. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`flush_interval_ms` was inert. It routed to a timer that was only ever evaluated *on the write path*, so it could add a redundant trigger but never delay or batch one — and since every durable put triggered its own append, tuning the knob did nothing at all. Give the WAL flusher a real ticker (`MessageHandler::tickers`, which the memtable flusher already used) and take the append off the put path. The append is the only thing on that schedule: it is an S3 PUT, billed per call, and bounding that cost is the entire reason a flush interval exists. The index apply is not on it — it is in-memory and free to batch, so it stays per-put on its own task. The tick names no store. `MessageFactory` is synchronous and cannot take the async state lock, and capturing an `Arc<BatchStore>` at handler construction would pin the first memtable forever. So `WalFlushSource::NextPending` is resolved when the message is *handled* — by walking the live stores oldest-first and taking the first that still owes an append. Oldest-first, not "the active memtable", and this is load-bearing. WAL entry positions are assigned in append-call order; replay walks them ascending; row positions follow; and primary-key recency is "newest visible row position wins". So append order *is* dedup order. A tick enqueued before a freeze is handled after it, and resolving to the active memtable would append the incoming memtable's batches ahead of the outgoing one's tail — silently handing the key to the stale row on the next replay. It survives the crash that caused it, and a full scan cannot see it. Selecting by cursor makes the target a function of what is durable rather than of when the timer fired. Two starvation fixes in the dispatcher, both of which this makes reachable: - The interval used the default `MissedTickBehavior::Burst`, which replays every tick missed while `handle()` ran. A WAL append easily outlasts its own interval, so missed ticks accumulate, the ticker arm is always ready, and — being `biased` — it starves the channel. Now `Delay`. - The `biased` select polled the ticker *before* `rx.recv()`. A tick only ever adds an append a real trigger would have made anyway, whereas a message may be a freeze's completion cell or `close()`'s final append, which nothing else delivers. Messages now win. `durable_write: true` with no ticker is rejected at open: the put path no longer triggers its own append, so such a writer would block forever on an append that never comes. Better a clear error than a deadlock. Accepted cost: single-client sequential *durable* throughput drops from ~10 writes/sec (one PUT round-trip) to roughly one per tick. That is the policy choice — the interval should mean what it says, and API cost should be bounded. Latency-sensitive callers want `durable_write: false`, which now costs them durability only, not visibility. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A single memtable holds at most `max_memtable_batches` batches, but a WAL is unbounded. Replay stuffed every WAL entry into one memtable, so a shard whose WAL had grown past one memtable's capacity failed `open()` outright with "MemTable batch store is full" — permanently unopenable. Replay now rotates exactly as the live write path does, on the same trigger. When the active memtable reaches the flush threshold it is sealed and — because the data is already durable in the WAL — flushed straight to a Lance generation with the same `MemTableFlusher::flush` the live path uses, rather than held in memory until open finishes. That bounds resident memory to ~two memtables and truncates the WAL as it goes, so a later reopen replays only the unflushed tail. Rotation is at WAL-entry boundaries, so each sealed generation covers a clean range of complete entries and stamps the last as its `replay_after_wal_entry_position`. The flush trigger is now one predicate, `memtable_reached_flush_threshold`, shared by the live path (post-insert, "room for one more batch?") and replay (pre-insert, "room for the next entry?"). It carries both criteria — `max_memtable_size` bytes and batch-store capacity. The byte trigger matters beyond avoiding overflow: it is what keeps a memtable under `max_memtable_rows`, and therefore keeps the in-memory HNSW index (sized to `max_memtable_rows`) from exhausting its capacity when the final active memtable is indexed. A single predicate is what stops the two paths from drifting — the exact class of bug this change set is about. `MemTable::is_batch_store_full` is deleted: its one production caller now goes through the shared predicate, and the two remaining test callers use the public `batch_store().is_full()` directly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… cursor Making the durability cursor writer-global renamed `MemTableStats:: max_flushed_batch_position` (per-memtable, inclusive, Option) to `durable_batch_count` + `global_offset` (writer-global, exclusive), but the Python and Java bindings still referenced the old field, so neither excluded crate compiled. Expose the new fields: Python's stats dict gains `durable_batch_count` and `global_offset` in place of `max_flushed_batch_position`, and Java's `MemTableStats` replaces `maxFlushedBatchPosition()` (Optional<Long>) with `durableBatchCount()` and `globalOffset()` (primitive longs), with the JNI constructor updated to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`sync_indexed_write`, `async_index_buffer_rows`, and `async_index_interval` had no behavioral consumers — they were only serialized into the config-defaults metadata map. They existed to configure async index-update buffering, which the index-apply-task split replaced with unconditional per-put indexing: a write is now read-your-writes through the index in every mode, which is exactly what `sync_indexed_write` promised. The knobs no longer control anything. Remove all three across every surface: the Rust core field, builders, defaults, and metadata serialization; the write-throughput bench's now-meaningless sync/async index axis; and the Python and Java binding parameters. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
hamersaw
force-pushed
the
feat/wal-poison-read-visibility
branch
from
2c51844 to
6a9ab9a
Compare
The public `insert_batches` doc linked to the private `PARALLEL_INDEX_MIN_ROWS` const, which `-D rustdoc::private-intra-doc-links` rejects. Demote the link to a plain code span; the surrounding prose already explains the threshold. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
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.
Summary
The MemWAL writer had a visibility model that didn't hold.
visible ⟹ durablewasasserted in
MemTableScanner's own doc comment but never enforced, and the two watermarksit was defined over were both broken. This series makes the invariant true, then rebuilds
durable_writeto mean what SlateDB'sawait_durablemeans: it controls whether thecaller blocks on durability, and nothing else.
Every bug below was verified by reintroducing it and watching a test fail, not by
inspection. Line references are to the pre-change tree.
The bugs
A poisoned writer served reads.
check_poisoned()guarded the three write paths and noread path, so a writer already known dead — fenced by a peer, or with a latched persistence
failure — kept handing out rows that were never durable and that replay would not
reproduce. A divergent snapshot from a corpse.
Replay re-appended and re-indexed everything it recovered. Nothing marked replayed
batches as already-durable, so the next WAL flush re-covered
[0, end): it appended thealready-durable rows a second time and re-inserted every replayed row into the indexes.
None of the three in-memory indexes is idempotent — HNSW mints fresh node ids for the same
row, FTS increments
doc_count/dfrather than recomputing them, BTree is a multiset whosesecond insert sets
pk_has_overridespermanently. A full scan looked healthy while everyindex-accelerated query silently returned duplicates, and it compounded: the WAL then held
those rows twice, so the next crash replayed both copies. (Reproduced: one 2-row put after a
reopen grew the WAL from 8 rows to 18.)
The durable watermark was meaningless across a memtable rotation.
WalFlusheris builtonce per writer and its watch channel is never reset, but the targets put against it were
memtable-local batch positions — and positions restart at 0 in every memtable. So after
memtable A flushed N batches, the first put into memtable B targeted position 1, saw N ≥ 1,
and acked immediately with no WAL append at all. The first N puts into every
post-rotation memtable were falsely acked as durable;
durable_write: truesilentlydegraded to non-durable. When B's append landed it sent a smaller value, walking the
watermark backwards — so a watcher from A could miss its target entirely and block forever.
The visibility cursor's zero was ambiguous. An inclusive position starting at 0 meant
both "nothing is visible" and "batch 0 is visible".
BatchStore::appendpublishescommitted_lenon the put path before the flush that indexes the batch is even triggered,and that flush is a ~100ms S3 PUT on another task — so batch 0 of every memtable sat
committed and readable for a full round-trip before it was indexed or durable, and only
through the arms backed by the batch store. The index-backed arms saw nothing. The tiers
actively disagreed.
A failed WAL append still published its rows. The append and the index apply ran
concurrently under one
tokio::join!, which runs both arms to completion and does notcancel the index arm when the append fails. So a failed append still advanced the cursor
every reader keyed off.
Index failures were never terminal. An index error carries no fence reason, so it was
always non-terminal — the writer limped on with a corrupt index. But
insert_batchesjoinsevery index thread unconditionally, so a failure leaves the others fully applied, and none
of HNSW/FTS/BTree has a delete. Both ways out are corrupt: retry re-covers the range and
re-inserts into the indexes that did succeed; skip it and the failed index is permanently
short rows the others have.
The flush interval was inert. It routed to a timer only ever evaluated on the write
path, so it could add a redundant trigger but never delay or batch one — and since every
durable put triggered its own append, tuning the knob did nothing.
The design
Two cursors, one derived view.
durableis a writer-global exclusive count on the writer;indexedis a per-memtable count on itsIndexStore; andvisibleis derived, neverstored —
min(indexed, durable)underdurable_write, or justindexedwithout it.Nothing caches
visible, deliberately. A cachedminrecomputed by two independent tasks isthe classic store-buffer race: under Release/Acquire both tasks can read the other's
pre-store value, both compute a minimum below the true one, and a max-clamped publish leaves
it permanently short — hanging any put blocked on it. With nothing cached there is nothing to
leave stale. The notify channel is a bare wake-up; every waiter recomputes.
The append and the index apply become two tasks on two channels. They have nothing in
common: one is an in-memory microsecond write, the other a ~100ms S3 PUT billed per call.
Batching exists to bound API cost, which an index apply does not incur. They need separate
channels because
TaskDispatcher::runawaitshandle()inline — sharing would queue everyindex apply behind an S3 PUT. Each is a single sequential consumer, which is the safety
property:
HnswGraph::insert_batchhard-rejects any range whose start is not itsindexed_len, and a single consumer guarantees contiguous in-order ranges however manyputters race behind it. Ordering comes from the task, not the flush interval — so
triggering per-put is exactly as safe as triggering on a timer.
The WAL append moves onto a real background ticker, and a tick names no store: it is resolved
when handled, by walking the live stores oldest-first and taking the first that still
owes an append. Not "the active memtable" — WAL entry positions are assigned in append-call
order, replay walks them ascending, row positions follow, and PK recency is "newest visible
row position wins". So append order is dedup order, and a tick enqueued before a freeze but
handled after it would append the incoming memtable ahead of the outgoing one's tail,
silently handing the key to the stale row on the next replay. It survives the crash that
caused it, and a full scan cannot see it.
What this buys
durable_write: falsenow costs durability only, not visibility. A non-durable put isread-your-writes through every arm, index-backed ones included. Previously the row sat
unindexed in the batch store until some later flush wandered by — a full scan could find it
while every index-accelerated query could not.
visible ⟹ durableis unconditional in durable mode, via themin.Accepted cost: single-client sequential durable throughput drops from ~10 writes/sec
(one PUT round-trip) to roughly one per tick. That is the policy choice — the interval should
mean what it says, and API cost should be bounded. Latency-sensitive callers want
durable_write: false.Breaking change
MemTableStats::max_flushed_batch_position: Option<usize>(per-memtable, inclusive) →durable_batch_count: usize+global_offset: usize(writer-global, exclusive). The field'smeaning changed, so it is renamed to force callers to notice rather than silently misread
it.
durable_write: truewith nomax_wal_flush_intervalis now rejected atopen(): the putpath no longer triggers its own append, so such a writer would block forever. Better a clear
error than a deadlock.
Follow-ups (not in this PR)
max_memtable_batches, replay overflows the batch store — a shard whose WAL outgrew onememtable's capacity cannot reopen. Pre-existing and orthogonal; found while building a
rotation test.
sync_indexed_writeis inert — zero consumers in the write path. Its doc promises"newly written data is immediately searchable via indexes", which is exactly what this PR
now delivers unconditionally. Should either mean something or be deleted.
Test plan
cargo test -p lance --lib— 2334 passed, 0 failedcargo clippy --all --tests --benches -- -D warnings— cleancargo fmt --allreintroducing the bug), including the two that surface as hangs rather than
assertion failures.
🤖 Generated with Claude Code
Update: replay rotation + dead-config removal
Two follow-ups landed on top of the original series.
Replay rotates memtables instead of overflowing. A single memtable holds at most
max_memtable_batchesbatches, but a WAL is unbounded — so replay stuffing every entry intoone memtable failed
open()with "MemTable batch store is full", leaving a shard whose WALhad grown past one memtable permanently unopenable. Replay now rotates on the same trigger
as the live path (a shared
memtable_reached_flush_thresholdpredicate carrying both thebyte and batch-count criteria) and, because the data is already durable, flushes each sealed
memtable straight to a Lance generation with the same
MemTableFlusher::flushthe live pathuses. Memory stays bounded to ~two memtables and the WAL truncates as it goes. The byte
trigger is load-bearing here, not cosmetic: it is what keeps a memtable under
max_memtable_rows, which is what the in-memory HNSW index is sized to.Removed the inert
sync_indexed_writeconfig family.sync_indexed_write,async_index_buffer_rows, andasync_index_intervalhad no behavioral consumers — they wereonly serialized into the config-defaults metadata. They configured the async index-update
buffering that the index-apply-task split replaced with unconditional per-put indexing, which
is exactly what
sync_indexed_writepromised. Removed across Rust core, benches, Python, andJava.
Also fixed a binding breakage the removal work surfaced: the writer-global-cursor commit
renamed
MemTableStats::max_flushed_batch_position→durable_batch_count+global_offsetbut never propagated to the Python/Java bindings, so neither excluded crate compiled. The
bindings now expose the new fields.