fix: audit hardening — resync OOM, cursor hygiene, Synadia timeout fallback#9
Merged
Conversation
…llback Six fixes from a deep technical audit, all inside the model-checked envelope (protocol.rs untouched; all 4 Stateright models incl. the three deep-bounds proofs re-verified green): - resync diff streams fold keys via for_each_in_range instead of range(): an All-scope cursor-expired resync against an on-disk backend no longer materializes the entire fold (values included) on the repair path - KvReader::entry() is now a required method: the get()-delegating default silently hid tombstones on any backend that forgot to override it, breaking ExportLease takeover of abandoned leases - unparseable ACK subjects yield VersionToken::unknown() instead of a fabricated revision 0, and watch_applied skips unknown versions for batch_high — they can neither mint nor clobber a cursor position (pre-fix: persisted cursor regressed to 0, full replay on next restart); pinned by unknown_version_update_does_not_move_or_clobber_cursor - create_key_value timeout now falls through to the Synadia Cloud raw fallback instead of ?-propagating past the exact workaround built for that deployment - BatchConfig grows channel_capacity (default 256, previously hardcoded) so hydration tuning covers the real batch boundary - LeaseGuard::abandon/complete: documented that the inherent future must_use + crate-level deny(unused_must_use) already make an unawaited call a compile error (an explicit #[must_use] attribute would mis-fire on the awaited () output) Deliberately NOT fixed: a prefix-scope live floor guard (the model checker rejected periodic-only detection, and sparse delivery makes probes false-positive into spurious resyncs) — documented as a Failure Modes row instead; replay_log's truncation/corruption conflation (needs a format change, stays a documented limitation). BREAKING: KvReader::entry() lost its default impl — trait implementors must add it (consumers of Arc<dyn KvReader> unaffected). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e breaking KvReader::entry() lost its default impl (trait implementors must add it) and BatchConfig gained channel_capacity (breaks struct-literal construction without FRU). Pre-1.0 semver: breaking → minor bump. Co-Authored-By: Claude Fable 5 <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.
Six fixes from a deep technical audit, all inside the model-checked envelope:
protocol.rsuntouched, all 4 Stateright models — including the three#[ignore]-gated deep-bounds proofs — re-verified green.Fixes
for_each_in_rangeinstead ofrange(); an All-scope resync against an on-disk backend no longer materializes the entire fold (values included) on the repair path. Both fjall and RocksDB override the streaming scan.KvReader::entry()is now required — theget()-delegating default silently hid tombstones on any backend that forgot to override it, which breaksExportLease::try_acquire's takeover of abandoned (CAS-deleted) leases. Backends without tombstone semantics delegate explicitly.VersionToken::unknown()instead of a fabricated revision 0, andwatch_appliedskips unknown versions forbatch_high: they can neither mint nor clobber a cursor position. Pre-fix, a single unparseable ACK on the hand-built multi-prefix consumer path regressed the persisted cursor to 0 (full replay on next restart). Pinned byunknown_version_update_does_not_move_or_clobber_cursor.create_key_valuetimeout now falls through to the raw JetStream fallback instead of?-propagating past the exact workaround built for that deployment.BatchConfig::channel_capacity— the watch-task → loop channel depth (previously hardcoded 256) is now tunable alongsidemax, since during state-sync hydration it can become the effective batch boundary.LeaseGuardmust-use — documented that the inherent futuremust_use+ crate-leveldeny(unused_must_use)already make an unawaitedabandon()/complete()a compile error (an explicit attribute mis-fires on the awaited()output).Verification
--all-features --all-targets, rustfmt🤖 Generated with Claude Code