Skip to content

fix(mem_wal): seal the memtable on max_memtable_rows - #8837

Merged
jackye1995 merged 1 commit into
lance-format:mainfrom
hamersaw:bug/wal-max-memtable-rows
Sep 1, 2026
Merged

jackye1995 merged 1 commit into
lance-format:mainfrom
hamersaw:bug/wal-max-memtable-rows

Conversation

@hamersaw

Copy link
Copy Markdown
Contributor

The bug

max_memtable_rows is documented as a memtable seal trigger, and the in-memory indexes are pre-allocated to exactly that many rows. No arm of the seal predicate ever read it:

store.row_bytes() >= max_memtable_size
    || memtable_resident_bytes(memtable) >= max_resident_bytes
    || store.remaining_capacity() < incoming_batches

Bytes, resident bytes, batch count. A shard whose rows are smaller than max_memtable_size / max_memtable_rows therefore grows past the cap unchecked, and the row past an HNSW graph's capacity fails the index apply — which poisons the writer:

HNSW vector store capacity 64 exhausted: inserting rows [60..70);
the store is sized below the memtable's row capacity

Replay hits the same wall while rebuilding the tail memtable's indexes, so the shard cannot be reopened either.

On defaults (256MB / 100k rows) the byte arm only beats the row cap when rows average ≥ ~2.7KB. A 1024-dim f32 vector is safe; 512-dim or 128-dim is not.

When

Never enforced — a pickaxe over all history finds no commit comparing a row count to max_memtable_rows. What changed is the consequence: #6701 replaced the memtable's IVF-PQ index (an unbounded per-partition overflow map, where overrunning the cap merely degraded search) with a fixed-capacity HNSW that hard-errors, and in the same commit rewrote the config doc from "used to pre-allocate index storage" to "When the memtable reaches capacity, it will be flushed". The promise and the fatal consequence landed together; the enforcement never did. #7888 later codified the byte arm as the cap's proxy — true only if max_memtable_size / avg_row_bytes <= max_memtable_rows, which nothing validates.

The fix

A fourth arm on the shared predicate: total_rows + incoming_rows > max_memtable_rows.

Unlike the byte arms this is a hard capacity rather than a target, so the live path also checks it pre-insert. That half is load-bearing: by the time a post-insert check fires, the rows an index cannot hold are already in the memtable the index apply will run over. Post-insert callers pass (1, 1) — "room for one more batch holding at least one row" — preserving today's prompt seal. Replay measures the whole incoming entry.

A write larger than the cap has no landing place at all, since a write is never split across memtables and rotating only hands it to a fresh memtable that overflows the same way. put/delete now reject it as invalid input naming the knob, rather than letting it surface as an exhausted index. Replay is deliberately not gated by that check — a WAL written under a larger cap must still open.

Tests

Five added, each verified to fail without the fix:

Test Without the fix
test_row_arm_seals_on_max_memtable_rows predicate never fires on rows
test_put_seals_on_max_memtable_rows memtable grows unbounded past the cap
test_hnsw_index_survives_a_shard_that_outgrows_the_row_cap put 6 was refused: HNSW vector store capacity 64 exhausted
test_replay_rotates_when_wal_exceeds_the_row_cap open() fails — permanently unopenable shard
test_put_rejects_more_rows_than_a_memtable_holds oversized write poisons the writer instead of erroring

The pre-insert placement is pinned specifically: with the arm added but the pre-insert call removed, the HNSW test still fails at put #6.

cargo test -p lance --lib — 3142 passed, 0 failed. Integration tests pass. cargo clippy -p lance --tests --benches -- -D warnings clean, cargo fmt --all applied.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NrtBDRTTnQfoxrErYydtgQ

@github-actions github-actions Bot added the bug Something isn't working label Aug 27, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Gate recommendation: request changes.

The live-write row admission is sound, but recovery must also preserve an atomic historical WAL entry when configuration lowers the row cap. Use replay-only capacity large enough for that entry, or another atomic flush path, so valid durable data can reopen before normal caps resume.

// holds more than a memtable can, a fresh one would overflow
// too, the same hard limit the live put path has, left to the
// insert below to surface.
let entry_rows: usize = batches.iter().map(|b| b.num_rows()).sum();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A final WAL entry larger than the reopen-time cap still makes the shard unopenable. When active is empty, this entry is inserted without rotation; at the WAL tip the final HNSW rebuild is still sized to max_memtable_rows, so open() fails. Replay needs a compatibility path that keeps the historical entry atomic while providing capacity for at least entry_rows, then seals it before normal admission resumes.

Reproducer

In test_replay_rotates_when_wal_exceeds_the_row_cap, replace the eight 4-row writer_a.put calls with:

writer_a.put(vec![vector_batch(0, 32)]).await.unwrap();

Then run:

cargo test -p lance --lib test_replay_rotates_when_wal_exceeds_the_row_cap

Expected: writer B reopens under cap 8. Observed: HNSW vector store capacity 8 exhausted: inserting rows [0..32) and the test fails.

@lance-gatekeeper lance-gatekeeper Bot added the K-changes Latest Gatekeeper recommendation requests changes. label Aug 27, 2026
`max_memtable_rows` is documented as a memtable seal trigger, and the in-memory
indexes are pre-allocated to exactly that many rows. No arm of the seal
predicate ever read it: only `max_memtable_size` bytes, the resident ceiling,
and batch-store capacity could seal. A shard whose rows were smaller than
`max_memtable_size / max_memtable_rows` therefore grew past the cap unchecked,
and the row past an HNSW graph's capacity failed the index apply — which
poisons the writer. Replay hit the same wall while rebuilding the tail
memtable's indexes, so the shard could not be reopened either.

This was never enforced, but it only became fatal in lance-format#6701, which replaced the
memtable's IVF-PQ index (an unbounded per-partition overflow map) with a
fixed-capacity HNSW that hard-errors — and, in the same commit, wrote the
config doc promising the seal.

The predicate grows a fourth arm, `total_rows + incoming_rows >
max_memtable_rows`. Unlike the byte arms this is a hard capacity rather than a
target, so the live path also checks it *pre*-insert: by the time a
post-insert check fires, the rows an index cannot hold are already in the
memtable the index apply will run over. Post-insert callers pass `(1, 1)` —
"room for one more batch holding at least one row" — preserving the prompt
seal. Replay measures the whole incoming entry.

A write larger than the cap has no landing place at all, since a write is never
split across memtables and rotating only hands it to a fresh memtable that
overflows the same way. `put`/`delete` now reject it as invalid input naming
the knob, rather than letting it surface as an exhausted index. Replay is
deliberately not gated by that check: a WAL written under a larger cap must
still open.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NrtBDRTTnQfoxrErYydtgQ
@hamersaw
hamersaw force-pushed the bug/wal-max-memtable-rows branch from 10fa55a to 7bf8f40 Compare August 27, 2026 20:47
@lance-gatekeeper lance-gatekeeper Bot removed the K-changes Latest Gatekeeper recommendation requests changes. label Aug 27, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Gate recommendation: request changes.

The durable-recovery finding remains on this revision. A single final WAL entry larger than the reopen-time row cap still fails the HNSW rebuild, so replay needs an atomic compatibility path with sufficient transient capacity before returning to normal limits.

@lance-gatekeeper lance-gatekeeper Bot added the K-changes Latest Gatekeeper recommendation requests changes. label Aug 27, 2026
@jackye1995
jackye1995 merged commit e877b58 into lance-format:main Sep 1, 2026
35 checks passed
@hamersaw
hamersaw deleted the bug/wal-max-memtable-rows branch September 1, 2026 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working K-changes Latest Gatekeeper recommendation requests changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants