feat(mem_wal): drive the WAL append on a flush-interval ticker#2
Closed
hamersaw wants to merge 4 commits into
Closed
feat(mem_wal): drive the WAL append on a flush-interval ticker#2hamersaw wants to merge 4 commits into
hamersaw wants to merge 4 commits into
Conversation
This was referenced Jul 21, 2026
hamersaw
added a commit
to lance-format/lance
that referenced
this pull request
Jul 21, 2026
…7872) ## Stack: 1/3 — foundational hardening This is the first of three stacked PRs that split the former #7791 (WAL poison / read-visibility / flush-interval) into reviewable pieces. It carries the changes that stand on their own, independent of the visibility redesign: - **perf(mem_wal): index small batches inline instead of one thread per index** — stop spawning a thread per index for tiny batches; index inline. - **fix(mem_wal): fail reads on a poisoned writer** — once a writer self-fences, reads must surface the failure instead of returning a silently truncated view. - **fix(mem_wal): reject index configs that disagree with the schema at shard open** — validate FTS/HNSW/BTree/PK column types and existence once at `open()`, before any row is accepted. A config that fails deterministically on every insert (including WAL replay) makes poison-and-replay non-terminating; reject it up front. Also errors (instead of silently indexing nothing) when an FTS column is missing from a batch, and relabels HNSW capacity-exhaustion as `internal` rather than `invalid_input`. Reviewable independently of the two PRs stacked on top. **Stack** 1. **this PR** — foundational hardening → `main` 2. hamersaw#1 — read-visibility redesign → this branch 3. hamersaw#2 — flush-interval ticker → PR 2's branch Splits the former #7791 into three reviewable pieces; #7791 can be closed once this stack lands. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
hamersaw
force-pushed
the
feat/wal-read-visibility
branch
from
July 21, 2026 17:29
c99ce03 to
8e6dcea
Compare
This PR replays lance-format#7754 unchanged against `main`. lance-format#7754 was accidentally merged into `xuanwo/sparse-stack-2-empty-inline-bitpacked` instead of `main`. This PR only corrects that target mistake and introduces no changes beyond the original PR. All design discussion, review history, approvals, and validation are recorded in lance-format#7754. --------- Co-authored-by: Weston Pace <weston.pace@gmail.com>
hamersaw
force-pushed
the
feat/wal-durable-flush-interval
branch
from
July 21, 2026 20:07
1396b1d to
b3d7b54
Compare
…ity cursors (lance-format#7888) ## WAL read-visibility redesign The core of the former lance-format#7791. Rows become visible to readers only once they are **both** indexed **and** WAL-durable, resolved through two independent cursors instead of a single conflated watermark. Index application runs on its own task rather than as an arm of the WAL flush, giving read-your-writes in every mode. Commits (bottom → top): - **mark replayed batches durable so the next flush does not re-append** — replayed rows are already WAL-durable; stamp the cursor so the first post-reopen flush doesn't re-append or re-index them. - **delete dead WAL-tracking state from MemTable** - **make the WAL durability cursor writer-global** / **make the visibility cursor an exclusive count** — the two-cursor model: an `indexed` cursor and a writer-global durability cursor; readers snapshot `visible_count` derived from both. - **publish rows only once they are indexed and durable** - **run the index apply on its own task, not as an arm of the WAL flush** - **rotate memtables during replay instead of overflowing** - **validate single-column PKs and restore index-apply stats** - **keep the latched failure across a poisoned terminal_error lock** - **harden shard-open validation and freeze failure handling** — move PK/index/interval validation before `claim_epoch` (a doomed open must not fence the incumbent); reject a config whose `field_id` names a different column than its `column`; retain the outgoing memtable and poison on a failed freeze dispatch. - **reject a batch position past committed_len instead of skipping it** **Stack** (the former lance-format#7791, split for reviewability) 1. lance-format#7872 — foundational hardening → **merged** 2. **this PR** — read-visibility redesign → `main` 3. flush-interval ticker → follows this PR 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- 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>
`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>
Owner
Author
|
Superseded: PR-2 (read-visibility redesign) merged upstream as lance-format#7888. Rebased these commits onto upstream/main and reopening as a cross-fork PR targeting main directly. Two of the original commits (stats bindings + doc-link cleanups) were folded into lance-format#7888 during review. |
hamersaw
force-pushed
the
feat/wal-durable-flush-interval
branch
from
July 21, 2026 20:42
b3d7b54 to
c8c1644
Compare
|
Important This PR touches the Lance format specification. Substantive changes to the format specification — the If this is a meaningful format change:
|
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
2 participants
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.
Stack: 3/3 — WAL flush-interval ticker
The feature the original lance-format#7791 was titled for, now small because it sits on the read-visibility redesign.
flush_interval_mswas 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.MessageHandler::tickers) and take the append off the put path. The append is the only thing on that schedule — it's an S3 PUT, billed per call, and bounding that cost is the whole reason a flush interval exists. The index apply stays per-put on its own task.WalFlushSource::NextPendingis resolved when the message is handled, by walking live stores oldest-first and taking the first that still owes an append.sync_indexed_writeconfig family —sync_indexed_write,async_index_buffer_rows, andasync_index_intervalhad no behavioral consumers after the index-apply-task split made every write read-your-writes. Removed across Rust core, benches, and Python/Java bindings.Stack
mainfeat/wal-read-visibility(PR 2's branch)Based on PR 2; merge after PR 1 and PR 2.
🤖 Generated with Claude Code