Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4387 +/- ##
============================================
+ Coverage 87.96% 88.03% +0.07%
- Complexity 1579 1606 +27
============================================
Files 1290 1290
Lines 230145 235659 +5514
Branches 193480 198654 +5174
============================================
+ Hits 202440 207457 +5017
- Misses 22980 23288 +308
- Partials 4725 4914 +189
🚀 New features to boost your workflow:
|
hubcio
force-pushed
the
feat/durable-protocol-and-sessions
branch
from
October 2, 2026 22:32
98a2f15 to
63eaf2a
Compare
Lost replies and capacity eviction could erase retry evidence and permit duplicate mutations after promotion or restart. Persist original results by session, group and request identity. Replay the first receipt even when a retry changes its payload, while preserving principal, session and client-operation guards. Protect committed capacity and publish receipt checkpoints before reclaiming their WAL. Refuse recovery when protection is absent or corrupt. Bind connections using client-owned proof, retain logical sessions across reconnects, renew leases only on authenticated activity, and retire protection across every allocated group before finalization. Preserve uncertainty when a later attempt is refused. Replay the exact encoded request after same-session resume and across refused or unreachable roster peers within its deadline. Expired sessions can log in again without replaying an ambiguous write under a new identity. Keep HTTP attachments invalidatable and serialize writes per partition under one deadline. Retain NoAck gates and reply slots through resolution, clean cancelled registrations, and retransmit retirement progress until all fences are acknowledged. Restore recovered receipts before admission while allowing live pipeline-backed commits. Allow intact empty WALs to elect, fence missing history before replacement WAL creation, and compare bootstrap revisions in the same domain. Introduce protocol 0.11, mandatory storage admission and exact peer build checks. Require explicit fresh-cluster initialization and Persisted durability for explicit writes without changing defaults. Align Go, Java, C# and Node login proofs and shared binding frames. Validate unsigned binding epochs and expose explicit producer durability in Rust and Python. Cover changed-payload retries, reply loss, crash recovery and reclamation, corruption, capacity pressure, lifecycle fences and transport retries. Document first-receipt integrity stamps and stable binding epochs.
Empty storage and Replicated writes were rejected despite existing startup and SDK contracts. Restore automatic metadata initialization and scope crash-safe retries to durable writes. Complete metadata directory loss still requires verified restore. Orderly shutdown must publish buffered WAL writes, and recovery must checkpoint them before advancing reserved offsets. Retain repair bodies across the gap and establish segment links before sends so descriptor exhaustion cannot block the shutdown barrier. Align replica, session and simulator fixtures, standalone formatting and isolated Miri with the supported contracts.
Explicit QUIC and WebSocket disconnects reused remembered credentials. Clear them like TCP while preserving involuntary transport recovery. Python CI polluted empty storage with logs and expected terminal errors to retry. Move logs beside data and align fixtures and docs with the existing explicit non-admission retry contract.
hubcio
force-pushed
the
feat/durable-protocol-and-sessions
branch
from
October 3, 2026 08:38
7264016 to
a621d98
Compare
Singleton offsets could acknowledge an undurable WAL entry, purge recovery could replace the original receipt, and retirement could count reports for an older namespace set. Session replacement and later refusals also exposed retry identity and uncertainty gaps. Wait for the singleton WAL barrier, preserve original receipt stamps through purge, and fence missing partition history. Seal namespace revisions into retirement reports and discard stale quorum evidence. Queue checkpoint publication and reclaim only after completion. Keep retained SDK requests within their session and preserve uncertainty across later refusals. Fix explicit Node login after confirmed expiry, first-boot marker ordering, lost metadata wakeups and avoidable HTTP work. Set current and minimum binary protocol to 0.11.1 and regenerate Swift fixtures. Preserve automatic startup and Replicated writes; total metadata-directory loss remains outside automatic recovery. Document the public Rust API break: ConsensusSession bind and next_request_id return Result. Producers start fresh retries only after TransientNotAccepted; other failures return the unconfirmed batch. Explicit disconnect requires a new login. Validate with the full just nextest gate, affected foreign SDK suites, strict Clippy, configured hooks and Miri peer-frame tests.
Contributor
Author
|
/skill team-review-slim |
There was a problem hiding this comment.
Summary: Four experts reviewed the pull request at 75c26b5. The review found throughput changes on the HTTP write path, a fatal-vs-degrade contradiction in the partition commit walk, a misleading session error, stale foreign-SDK docs and guards, and one redundant journal barrier.
Counts: critical 0, warning 8, nit 4, simplification 1
This review was generated by Claude Code 2.1.284 on deepseek-flash[1m]. Review the output before you act on it.
Poll routes accepted malformed or mismatched bind replies, and the Node protocol guard missed patch drift. Validate bindings before polling and report both session identities on a resume mismatch. Avoid capacity scans when queue bounds prove space and retain only retirement entries during commit. Preserve receipt fences and HTTP ordering without adding reservation counters. Remove a redundant journal sync and correct session and protocol documentation.
Contributor
Author
|
/skill team-review-slim |
There was a problem hiding this comment.
Summary: One critical finding: a state-transfer install leaves pending_retry_checkpoint stale, so the next receipt reclaim deletes the live receipt file and the partition fails to reopen with StateFileCorrupted. The review also finds a full-reply copy on the primary commit path and a session-bind error that names the wrong cause, plus smaller items on a duplicate-reply copy, a redundant segment sync, HTTP retry docs, Java protocol parity, and a constant binding.
Counts: critical 1, warning 2, nit 4, simplification 1
Findings without an anchor on a changed line:
core/partitions/src/iggy_partition.rs:1062critical: After a state-transfer install,pending_retry_checkpointstill holds the pre-install frontier whilecheckpoint_op()has moved to the install op. This reclaim then deletes the receipt file the install wrote, so the partition fails to reopen withStateFileCorrupted. Reclaim frompersistence.checkpoint_op(), or clear the field on every reset.core/partitions/src/iggy_partition.rs:6636warning:reply.clone()allocates an aligned frame and copies the whole reply for every committed client operation on the primary. Freeze the reply once and share it with the commit cache, copying only for an in-process waiter.core/partitions/src/iggy_partition.rs:4573nit:cached.into_message()copies the retained reply into an owned frame, and the bus branch then converts it back toFrozen. Send the cachedFrozenon the bus path, and build the owned copy only for an in-process waiter.
This review was generated by Claude Code 2.1.284 on deepseek-flash[1m]. Review the output before you act on it.
Delayed persistence completion could delete the receipt checkpoint installed by state transfer. Reclaim against the published frontier. Share immutable replies on bus paths and avoid redundant retained-file barriers while preserving recovery truncation durability. Correct SDK session diagnostics and retry docs, and check Java protocol parity.
RequestIdExhausted was added to the Rust catalog without updating Swift's mirror, failing the golden check. Regenerate the fixture and add the matching Swift case so error-code parity stays intact.
numinnex
reviewed
Oct 3, 2026
| .client_table | ||
| .borrow() | ||
| .ended_sessions() | ||
| .min_by_key(|identity| (identity.metadata_watermark, identity.client_id)); |
Contributor
There was a problem hiding this comment.
critical: retirement is O(ended sessions x partitions) of fsynced RetireSession ops, one identity at a time, including partitions the session never touched.
session_retired needs commit_min == commit_max, so busy partitions keep failing, the cursor resets and the sweep resends every ~200ms. A fatal or transferring partition on the primary node, or a TruncatePartition revision bump, stalls every finalize.
Tables never evict, so churn fills them and logins are refused cluster-wide.
Suggest one frontier op per partition covering all sessions ended at <= W, skipping tombstoned or fatal namespaces, and counting the primary like any other reporter.
spetz
requested changes
Oct 3, 2026
| // a write refused transiently here is replayed after its | ||
| // successors committed. The slice therefore keeps a committed-id | ||
| // window under the watermark (`consensus::COMMITTED_WINDOW_BITS`) | ||
| // and admits an unmarked id inside it instead of absorbing it. | ||
| if !is_auto_commit_client(client_id) { | ||
| if consensus.pipeline_has_message_from_client_request(client_id, request) { | ||
| if let Some((pending_session, pending_request, pending_operation)) = |
Contributor
There was a problem hiding this comment.
The preflight replaced the exact-replay check pipeline_has_message_from_client_request with pending_request, so on the binary transports a pipelined second request from the same client in the group gets TransientNotAccepted. Pipelining clients such as the Java async SDK handle that code as a routing failure, retry on a timer and run leader rechecks for a lane that is only busy. The thread at core/server/src/http/submit.rs:370 covers the HTTP side effect of the double resolve. This one hits any pipelining binary client.
Retirement could race with queued writes and reuse obsolete quorum reports. Transferred retry tables could also be checkpointed before their effects were applied, making restart recovery fail. Exclude writes while their session retirement is pending, qualify reports by view, and checkpoint only after applied state catches up. Retain one partition receipt, distinguish uncommitted transfer capacity, and isolate corrupt receipt recovery to its partition. Reuse resolved HTTP partitions and capture the admission frontier before routing waits. Warm peer identity outside shard runtimes.
Transport loss reset established sessions in Go, Java and C#, leaving old protection slots reserved and obscuring retry identity. Java, Node and C# also let later refusals overwrite an unknown mutation outcome. Resume established sessions with their original proof and permit fresh registration only after a definitive bind refusal. Preserve uncertainty across later refusals, fence retained Rust frames after transport locking, and restore retries for proven unsent attempts. Document session and API contracts and synchronize error catalogs.
This branch has not been deployed
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
3 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.
Lost replies and capacity eviction could erase retry evidence and allow duplicate mutations after promotion or restart. This PR retains the original result by session, group and request identity, and publishes receipt checkpoints before reclaiming the corresponding WAL. Retries return the first receipt even when their payload changes, while preserving principal, session and operation checks.
Shared sessions use client-owned binding proof, authenticated lease renewal and ordered retirement across every allocated partition group. Retirement reports carry the swept namespace revision, so reports collected before a namespace change cannot finalize protection. Missing or corrupt retry history refuses recovery before destructive repair.
Persisted singleton acknowledgements wait for the WAL barrier. Purge/restart preserves the original send receipt without restoring purged messages. SDK reconnects retain encoded request identity, and later refusals cannot turn an uncertain request into a fresh mutation. Checkpoint publication runs through persistence completion; HTTP moves owned bodies and avoids unrelated session/gate scans.
Compatibility and limits:
breaking:storagelabel waives the legacy-upgrade check; no migration or rolling-upgrade adapter is supplied.ack=noneconfirms dispatch only. A detached reply waiter retains the same-session/partition lane until the reply or bounded deadline; queued writers acquire admission permits after that gate.ConsensusSession::bindandnext_request_idnow returnResult;register_request_idpreserves registration identity.VsrSessionControladds requiredsession_identityandsession_bind_secretmethods for external transport implementations; its public marker is not a seal. The proof accessor is hidden from generated docs and must stay private to session control. Explicit disconnect clears remembered login, while configured AutoLogin can authenticate a subsequent connection.TransientNotAcceptedor proven presubmissionNotConnected/CannotEstablishConnection. HTTP failures use separate transport retry settings. Uncertain and terminal failures return the unconfirmed batch. An application resend can duplicate an earlier uncertain mutation. Authorized receipt access after permission revocation and general routing/cancellation APIs remain follow-up work.clients_table_maxanddedup_clients_maxare fixed by the first committed prepare in each group. Recovery and transfer preserve that limit and warn on configuration mismatch. Live retry protection is not evicted; capacity becomes available after ordered retirement. Retirement remains per identity across every allocated group.The bind proof is an independent session credential: password changes and PAT revocation/expiry do not end an established session. Binding rechecks owner existence and Active status; current permissions govern every operation. Logout, lease expiry and user deactivation end session access.
Go, Java, C#, Node and Rust-backed bindings use the coordinated login/binding contract. Established Go/Java/C# sessions now also resume coordinator connections with BindSession, retaining identity and sequence; only terminal 40/30 bind refusal permits fresh registration. Swift golden fixtures and error mirrors are regenerated from Rust.
Validation: the initial full
just nextestgate passed 7,240 tests with 17 existing skips. This review round passed 2,731 affected Rust library tests with six existing skips, Node 353 unit tests/build/lint, Go 590 unit tests with the race detector/build/lint, C# 649 unit tests on each of net8/net10/build/formatting, and Java 1,179 non-integration unit tests. The affected Java uncertainty regression and static checks also pass after the final helper extraction. Required formatting, strict workspace Clippy, build, TOML checks and applicable hooks pass. Seven selected real-server tests pass, covering singleton/cluster lost receipts, purge, WAL reclamation, registry retirement, HTTP concurrency and repeated metadata snapshot installation. The previously published head has 107 passing CI checks and no failures; CI will rerun for these new commits.Depends on merged #4365.