Skip to content

fix(server): correct VSR client session routing and lifecycle#3407

Open
numinnex wants to merge 5 commits into
masterfrom
fix_clients
Open

fix(server): correct VSR client session routing and lifecycle#3407
numinnex wants to merge 5 commits into
masterfrom
fix_clients

Conversation

@numinnex
Copy link
Copy Markdown
Contributor

@numinnex numinnex commented Jun 2, 2026

  • Drop replicated sessions cluster-wide on disconnect. A lost connection now routes a synthetic Logout to the metadata owner, so the replicated session slot is released everywhere instead of lingering until peer
    eviction.

  • Mutate the SessionManager only after the register commits. Login/register no longer flips the connection to Authenticated before submit (with rollback on failure); it submits, awaits the commit (post-quorum),
    then transitions Connected → Bound in one step. A transient submit failure leaves the connection untouched and the SDK replays.

  • Split thebootstrap.rs into focused modules — wire, pat, responses, auth, dispatch — leaving bootstrap.rs with cluster boot/recovery/startup only. The request handlers, owner-forwarders
    (submit_*_on_owner), wire-response builders, and credential verification now live next to each other.

@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Jun 2, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 2, 2026

Codecov Report

❌ Patch coverage is 6.78796% with 1332 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.11%. Comparing base (5858410) to head (3aeea08).

Files with missing lines Patch % Lines
core/server-ng/src/dispatch.rs 0.00% 537 Missing ⚠️
core/server-ng/src/responses.rs 0.00% 525 Missing ⚠️
core/server-ng/src/auth.rs 0.00% 112 Missing ⚠️
core/server-ng/src/pat.rs 0.00% 60 Missing ⚠️
core/shard/src/lib.rs 0.00% 36 Missing and 1 partial ⚠️
core/metadata/src/impls/metadata.rs 20.83% 19 Missing ⚠️
core/server-ng/src/wire.rs 0.00% 12 Missing ⚠️
core/consensus/src/metadata_helpers.rs 83.33% 8 Missing ⚠️
core/server-ng/src/bootstrap.rs 0.00% 8 Missing ⚠️
core/shard/src/router.rs 0.00% 7 Missing ⚠️
... and 2 more

❌ Your patch check has failed because the patch coverage (6.78%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@              Coverage Diff              @@
##             master    #3407       +/-   ##
=============================================
- Coverage     74.46%   54.11%   -20.36%     
  Complexity      943      943               
=============================================
  Files          1231     1234        +3     
  Lines        120911   107335    -13576     
  Branches      97648    84100    -13548     
=============================================
- Hits          90042    58079    -31963     
- Misses        27918    46356    +18438     
+ Partials       2951     2900       -51     
Components Coverage Δ
Rust Core 49.54% <6.78%> (-26.08%) ⬇️
Java SDK 58.44% <ø> (ø)
C# SDK 69.41% <ø> (-0.55%) ⬇️
Python SDK 81.06% <ø> (ø)
Node SDK 91.53% <ø> (+0.01%) ⬆️
Go SDK 40.20% <ø> (ø)
Files with missing lines Coverage Δ
core/server-ng/src/login_register.rs 55.55% <100.00%> (+11.95%) ⬆️
core/shard/src/builder.rs 0.00% <0.00%> (ø)
core/server-ng/src/session_manager.rs 86.66% <91.48%> (-1.02%) ⬇️
core/shard/src/router.rs 0.00% <0.00%> (ø)
core/consensus/src/metadata_helpers.rs 91.72% <83.33%> (-0.07%) ⬇️
core/server-ng/src/bootstrap.rs 7.65% <0.00%> (+3.32%) ⬆️
core/server-ng/src/wire.rs 0.00% <0.00%> (ø)
core/metadata/src/impls/metadata.rs 35.24% <20.83%> (-0.19%) ⬇️
core/shard/src/lib.rs 78.69% <0.00%> (-4.00%) ⬇️
core/server-ng/src/pat.rs 0.00% <0.00%> (ø)
... and 3 more

... and 299 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@numinnex numinnex changed the title fix(server): fix get_clients in server-ng fix(server): correct VSR client session routing and lifecycle Jun 3, 2026
Copy link
Copy Markdown
Contributor

@hubcio hubcio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few findings landed on unchanged context lines, so they couldn't be attached inline - noting them here:

  • login_register.rs:47 - InvalidClientId is never constructed in production (only the enum def, the TryFrom arm and a test). the client_id==0 path it documents is rejected upstream by the header validation and would hit the assert!(client_id != 0) first. reads like deliberate non_exhaustive scaffolding, but if it isn't pinning a future contract it can go.

  • login_register.rs:104 - the doc on TryFrom<&LoginRegisterError> for EvictionReason says 'no production caller yet', but surface_login_failure calls it via .is_ok() on the live login/register branches. just drop the stale claim.

  • login_register.rs:97 - that TryFrom mapping is only consumed via .is_ok() today; a fn is_terminal(&LoginRegisterError) -> bool would do the same and save the mapping plus the NotEvictable type. documented as scaffolding for the eviction-frame dispatcher though, so fine to keep if that's landing soon.

  • session_manager.rs:250 - get_connection / connection_for_client / bound_count / connection_count have no production callers (tests only), and connection_for_client's 'for routing consensus replies' doc is obsolete now that the home shard routes by transport id. safe to delete - keep the client_to_connection map itself, bind_session still needs it. pre-existing, not introduced here.

Comment thread core/server-ng/src/dispatch.rs Outdated
Comment thread core/server-ng/src/responses.rs
Comment thread core/server-ng/src/dispatch.rs Outdated
Comment thread core/shard/src/lib.rs Outdated
Comment thread core/metadata/src/impls/metadata.rs
Comment thread core/server-ng/src/dispatch.rs Outdated
Comment thread core/server-ng/src/pat.rs Outdated
Comment thread core/server-ng/src/dispatch.rs
Comment thread core/server-ng/src/session_manager.rs Outdated
Comment thread core/server-ng/src/dispatch.rs
@github-actions github-actions Bot added S-waiting-on-author PR is waiting on author response and removed S-waiting-on-review PR is waiting on a reviewer labels Jun 3, 2026
@numinnex
Copy link
Copy Markdown
Contributor Author

numinnex commented Jun 3, 2026

/ready

@github-actions github-actions Bot added S-waiting-on-review PR is waiting on a reviewer and removed S-waiting-on-author PR is waiting on author response labels Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review PR is waiting on a reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants