Skip to content

fix(rollout): surface unflushed rows in observe(), which undercounts - #192

Merged
beinan merged 1 commit into
mainfrom
fix/rollout-observe-unflushed
Jul 25, 2026
Merged

fix(rollout): surface unflushed rows in observe(), which undercounts#192
beinan merged 1 commit into
mainfrom
fix/rollout-observe-unflushed

Conversation

@beinan

@beinan beinan commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

Closes #187.

Problem

RolloutObservation::row_count sums the base table and flushed MemWAL generations. Rows in an unsealed memtable are in neither.

Before the seal moved off the append path, every add sealed before returning, so this was exact. Now, in steady state, row_count is permanently short by roughly one flush interval's worth of writes — and nothing anywhere exposed that.

Consumers affected: the control-plane stats scanner, capacity and compaction decisions derived from it, and the row counts shown in the UI. A store can accept writes, report fewer rows than it holds, and give no indication why.

Change

Add RolloutObservation::unflushed_rows, read from the resident writer's memtable_stats(). row_count + unflushed_rows is every row this instance has durably accepted, and a persistently non-zero value is the signal that the flush sweeper is not keeping up (or is disabled).

Best-effort and non-failing by design: no resident writer, a WAL-only writer with no memtable, or a fenced writer all report 0. An observability read should never fail because of writer state.

Not plumbed into the master's stats table. The value reads this process's in-memory writer and cannot see another instance's memtable, so aggregating it cross-instance would be actively misleading. It stays a local signal; pending_wal_generations remains the cross-instance one.

Documented add's return value as carrying no information: the base version does not advance on a MemWAL append, so it is a constant unrelated to the rows just written, and since the seal moved it no longer signals visibility either. The doc now says explicitly not to use it as a write handle or to poll for visibility.

I did not change the signature to LanceResult<()> as the issue floated — it would break the public AddRolloutsResponse.version DTO and every caller. Worth doing, but as its own breaking change with a version bump rather than smuggled into an observability fix.

Testing

observe_reports_unflushed_rows_excluded_from_row_count walks the full lifecycle: no writer → 0; two rows added → row_count == 0 while unflushed_rows == 2; after flush → row_count == 2, unflushed_rows == 0. This pins the undercount as intentional-and-visible rather than accidental.

cargo test --workspace → all green (162 core, 49 server, rest as before). fmt + clippy clean.

🤖 Generated with Claude Code

@beinan
beinan force-pushed the fix/rollout-observe-unflushed branch from a462ed6 to aa71ddc Compare July 25, 2026 07:29
`RolloutObservation::row_count` sums the base table and flushed MemWAL
generations. Rows sitting in an unsealed memtable are in neither, so since
the seal moved off the append path the count is permanently short by roughly
one flush interval's worth of writes. Before that change every append was
sealed before returning and the count was exact.

That undercount is inherent to asynchronous flush and not a bug on its own —
the problem is that it was invisible, and it feeds the control-plane stats
scanner, capacity decisions, and the row counts shown in the UI.

Add `unflushed_rows`, read from the resident writer's memtable stats, so the
gap is observable: `row_count + unflushed_rows` is every row this instance
has durably accepted, and a persistently non-zero value means the flush
sweeper is not keeping up. Best-effort and non-failing — no resident writer,
a WAL-only writer, or a fenced writer all report 0, so an observability read
never fails on writer state.

Deliberately not plumbed into the master's stats table: it reads this
process's in-memory writer and cannot see another instance's memtable, so
aggregating it across instances would be misleading.

Also documents that `add`'s return value now carries no information about
the append at all — the base version does not advance, and since the seal
moved it no longer signals visibility either. Changing the signature would
break `AddRolloutsResponse.version`, so that is left for a version bump.

Closes #187

Co-Authored-By: Claude <noreply@anthropic.com>
@beinan
beinan force-pushed the fix/rollout-observe-unflushed branch from aa71ddc to 6a7b1e5 Compare July 25, 2026 07:47
@beinan
beinan merged commit c59a14c into main Jul 25, 2026
9 checks passed
@beinan
beinan deleted the fix/rollout-observe-unflushed branch July 25, 2026 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(rollout): observe() row_count systematically undercounts unflushed rows; add() return value is now meaningless

1 participant