add cursor-after-apply watch combinator#1
Merged
Conversation
Every hand-rolled watch loop in the system advanced the resume cursor on receipt of an update rather than after applying it. On crash+resume the persisted cursor claims "caught up to rev N" while rev N is still in an unapplied batch buffer, so the watch re-arms past it and silently skips it — a hole in the "resume after any restart" guarantee. watch_applied encodes the safe discipline once: INVARIANT: a persisted/reported cursor C => every update with rev <= C has been applied (the caller's apply() returned for it). On each flush it runs apply(batch) to completion, THEN advances the cursor, checkpoints the snapshot at that cursor, and fires on_applied — never before. parse-rejected entries (corrupt/irrelevant) still advance the cursor, since there's nothing to apply for them. CursorExpired falls back to a full-scope watch. Snapshot writes stream raw updates as they arrive but checkpoint at the post-apply cursor, so a loaded snapshot's cursor is always consistent with the state it carries. API: watch_applied(), WatchScope (All|Prefix), BatchConfig (10ms/100). Generic over slipstream's own types; apply/on_applied are the only caller seams. Both the tunnel router and edge origin watcher fold onto this. Verification: - 8 unit tests (stub KvWatcher + paused clock) for deterministic timing and fault injection: cursor-after-apply ordering, flush on window/max/shutdown/channel-close, CursorExpired fallback, rejected entries advance the cursor, snapshot checkpoint == applied cursor. - 4 integration tests against a live nats-server proving the resume guarantee on real JetStream revision ordering: end-to-end apply, snapshot-cursor resume with no skip/no dup across a restart, cursor advances over rejected updates, survives a compacted resume cursor. - Batch-throughput bench (~1.25M updates/s). Docs: ARCHITECTURE.md "Applied-Cursor Watch" section (cites Saltzer/ Reed/Clark 1984 and Consul anti-entropy/blocking-queries) and a README "Applied watch" section. Additive: no existing exports change. Bumps tokio features (macros, rt, time) for the combinator's spawn/select/sleep. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI runs `cargo clippy --all-targets -- -D warnings`, but mise installs the Rust toolchain with the minimal rustup profile, which omits clippy — so the step failed with "'cargo-clippy' is not installed for the toolchain". Declare the components in mise.toml so the toolchain carries clippy and rustfmt everywhere (CI and local). Co-Authored-By: Claude Opus 4.8 (1M context) <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
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.
Every hand-rolled watch loop in the system advanced the resume cursor on receipt of an update rather than after applying it. On crash+resume the persisted cursor claims "caught up to rev N" while rev N is still in an unapplied batch buffer, so the watch re-arms past it and silently skips it — a hole in the "resume after any restart" guarantee.
watch_applied encodes the safe discipline once:
INVARIANT: a persisted/reported cursor C => every update with rev <= C
has been applied (the caller's apply() returned for it).
On each flush it runs apply(batch) to completion, THEN advances the cursor, checkpoints the snapshot at that cursor, and fires on_applied — never before. parse-rejected entries (corrupt/irrelevant) still advance the cursor, since there's nothing to apply for them. CursorExpired falls back to a full-scope watch. Snapshot writes stream raw updates as they arrive but checkpoint at the post-apply cursor, so a loaded snapshot's cursor is always consistent with the state it carries.
API: watch_applied(), WatchScope (All|Prefix), BatchConfig (10ms/100). Generic over slipstream's own types; apply/on_applied are the only caller seams. Both the tunnel router and edge origin watcher fold onto this.
Verification:
Docs: ARCHITECTURE.md "Applied-Cursor Watch" section (cites Saltzer/ Reed/Clark 1984 and Consul anti-entropy/blocking-queries) and a README "Applied watch" section.
Additive: no existing exports change. Bumps tokio features (macros, rt, time) for the combinator's spawn/select/sleep.