docs(rollout): correct add() contract to async visibility - #188
Merged
Conversation
`add` stopped sealing on the append path in #181 — it now performs only a durable `put`, with `force_seal_active` + `wait_for_flush_drain` moved to `flush` and driven by the server's periodic flush sweeper. The doc comment was not updated and still claimed rows are "immediately visible to reads on any instance" and described the old three-step per-append work, directly contradicting the inline comment a few lines below it. Rewrite the contract to state what the code does: durable on return, not visible on return, with the gap bounded by ROLLOUT_FLUSH_INTERVAL_SECS and an explicit pointer to `flush` for callers needing read-your-write. Also fix a stale link to the removed `ensure_write_writer` and note the `observe()` undercount that follows from the same asynchrony. Docs only; no behavior change. The contract is already covered by the existing `add_is_durable_but_not_visible_until_flush` test. Closes #183 Co-Authored-By: Claude <noreply@anthropic.com>
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.
Fixes #183.
Problem
#181 moved the seal off the append path:
addnow performs only a durableput, andforce_seal_active+wait_for_flush_drainlive inflush, driven by the server's periodic flush sweeper (default 30s).The doc comment on
addwas not updated. It still claimed:and still described the per-append work as
put → force_seal_active → wait_for_flush_drain.This directly contradicts the inline comment ~10 lines below it, which correctly says read-after-write is asynchronous. As a public rustdoc contract it tells every downstream caller they have read-your-write when in the default configuration they may wait up to 30 seconds.
Change
Docs only. The rewritten comment states:
addis durable on return —putwaits for the WAL entry to reach object storageaddis not visible on return; a row becomes readable only once its memtable is sealed byflush(orclose, or the merge path's internal close)ROLLOUT_FLUSH_INTERVAL_SECS; callers needing immediate visibility shouldadd().awaitthenflush().awaitRolloutObservation::row_countdoes not count durable-but-unflushed rowsAlso fixes a stale intra-doc link to
ensure_write_writer, which no longer exists (nowresident_writer).Verification
No behavior change. The documented contract is already asserted by the existing
add_is_durable_but_not_visible_until_flushtest added in #181 — I started to add an equivalent test before finding it, and dropped mine as redundant.cargo test -p lance-context-core --lib→ 161 passed, 0 failed.🤖 Generated with Claude Code