Skip to content

refactor(consensus): remove namespace from client headers - #3836

Merged
numinnex merged 22 commits into
masterfrom
remove_namespace_field
Aug 11, 2026
Merged

refactor(consensus): remove namespace from client headers#3836
numinnex merged 22 commits into
masterfrom
remove_namespace_field

Conversation

@numinnex

@numinnex numinnex commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Removes the client-supplied namespace word from the VSR client request and reply headers. The server derives the routing target itself: the plane from operation, the partition group from the request body. At the admission boundary a request is promoted to the server-internal RoutedRequestHeader, which carries the resolved consensus group in the tail of the client header's reserved area (bytes 248..256).

This is a behavior change, not only a refactor. Name-addressed sends previously shipped headers pointing at partition (0, 0, 0), and the Rust default producer config could not send over VSR at all. With the namespace derived server-side, balanced and message-key partitioning now work over VSR for the Rust and Node SDKs.

Review follow-ups included here:

  • RoutedRequestHeader::validate now enforces the same field rules as the client boundary (shared validate_request_fields), so a peer-wire Command2::Request frame cannot carry client = 0 into the client table's hard assert or replay a cached register reply via operation = Reserved.
  • The client-to-routed promotion is an in-place retype (Message::into_routed) that zeroes only the eight group bytes instead of rebuilding the 256-byte header; the terminal validate stays.
  • Cached client-table replies persist as raw ReplyHeader bytes, and this PR moves status from 224 to 216. Both persistence surfaces are versioned: the state-transfer artifact magic is bumped to ICT2, and MetadataSnapshot::decode refuses snapshots whose version differs from the current format version (bumped to 2).
  • Layout pins: ReplyHeader::status at offset 216, RoutedRequestHeader::group at 248..256, and the data-bearing reserved prefix surviving promotion.

numinnex and others added 13 commits August 3, 2026 12:19
Removing the namespace-exemption import left its #[cfg(debug_assertions)]
attribute orphaned onto the unconditional server_common import below it, so
every release build of the shard crate fails with 112 unresolved-name
errors. Debug builds keep the import and CI's -D warnings lane rides
clippy, which builds debug, so nothing upstream could observe it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Aug 7, 2026
@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.75796% with 206 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.52%. Comparing base (a3e256b) to head (b1fea62).

Files with missing lines Patch % Lines
core/shard/src/lib.rs 47.16% 54 Missing and 2 partials ⚠️
core/server-ng/src/dispatch.rs 37.73% 33 Missing ⚠️
core/simulator/src/client.rs 65.27% 25 Missing ⚠️
core/metadata/src/impls/metadata.rs 70.66% 22 Missing ⚠️
core/binary_protocol/src/consensus/header.rs 91.11% 12 Missing ⚠️
core/metadata/src/stm/snapshot.rs 77.27% 4 Missing and 1 partial ⚠️
core/partitions/src/state_transfer.rs 16.66% 5 Missing ⚠️
core/consensus/src/impls.rs 91.66% 4 Missing ⚠️
core/partitions/src/iggy_partitions.rs 75.00% 4 Missing ⚠️
core/shard/src/router.rs 75.00% 4 Missing ⚠️
... and 28 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3836      +/-   ##
============================================
- Coverage     76.95%   75.52%   -1.44%     
- Complexity     1021     1296     +275     
============================================
  Files          1366     1374       +8     
  Lines        176130   176464     +334     
  Branches     146173   146173              
============================================
- Hits         135547   133274    -2273     
- Misses        36707    39263    +2556     
- Partials       3876     3927      +51     
Components Coverage Δ
Rust Core 75.92% <73.28%> (-0.15%) ⬇️
Java SDK 66.15% <ø> (+2.92%) ⬆️
C# SDK 47.00% <ø> (-29.01%) ⬇️
Python SDK 89.98% <ø> (ø)
PHP SDK 84.26% <ø> (ø)
Node SDK 96.25% <100.00%> (-0.08%) ⬇️
Go SDK 68.60% <100.00%> (-0.53%) ⬇️
Files with missing lines Coverage Δ
core/binary_protocol/src/namespace.rs 90.90% <ø> (ø)
core/binary_protocol/src/version.rs 99.22% <ø> (ø)
core/consensus/src/client_table.rs 88.21% <ø> (ø)
core/consensus/src/lib.rs 0.00% <ø> (ø)
core/consensus/src/observability.rs 40.08% <100.00%> (ø)
core/consensus/src/plane_helpers.rs 96.24% <100.00%> (-0.03%) ⬇️
core/server-ng/src/bootstrap.rs 24.89% <ø> (ø)
core/server-ng/src/http/session.rs 85.80% <ø> (ø)
core/server-ng/src/http/wire.rs 95.93% <100.00%> (ø)
core/server-ng/src/partition_reconciler.rs 93.02% <100.00%> (ø)
... and 61 more

... and 129 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.

mmodzelewski
mmodzelewski previously approved these changes Aug 8, 2026
spetz
spetz previously approved these changes Aug 8, 2026
@numinnex
numinnex dismissed stale reviews from spetz and mmodzelewski via 02b1937 August 8, 2026 06:37
@numinnex
numinnex marked this pull request as draft August 8, 2026 06:39
@github-actions github-actions Bot removed the S-waiting-on-review PR is waiting on a reviewer label Aug 8, 2026
@numinnex
numinnex marked this pull request as ready for review August 10, 2026 18:27
@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Aug 10, 2026
mmodzelewski
mmodzelewski previously approved these changes Aug 10, 2026
spetz
spetz previously approved these changes Aug 11, 2026

@hubcio hubcio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • cached client-table replies persist as raw ReplyHeader bytes in the checkpoint and the state-transfer artifact, and this PR moved status 224 -> 216. correct today only by coincidence: every cached reply is metadata-plane so the old namespace word was 1<<63 with a zero low half, which the relocated status reads as 0. the state-transfer artifact has a magic tag but the msgpack snapshot reply bytes have no version marker at all. version both before this can bite.

  • IggyNamespace::new masks (& STREAM_MASK) instead of rejecting and nothing caps live stream/topic count, so 4097 concurrently-live streams gives slab key 4096 which masks to stream 0 - a send to it lands in stream 0's partition and journals there. pre-existing (the resolve path is unchanged and string ids always masked server-side) so not a blocker here, but it's a silent cross-tenant write and this removes the last client-side fail-fast. worth its own issue: a fallible checked_new at the few sites that pack a runtime-derived key.

  • worth noting in the description: this also enables balanced and message-key partitioning over VSR for the rust and node SDKs (the rust default producer config couldn't send over VSR before), so it's a behavior change, not only a refactor.

the core change is good - the deleted SDK packers re-derived a value the server computed anyway, and name-addressed sends were shipping headers pointed at partition (0,0,0). the gap is that the client boundary got hardened while the peer boundary stayed wire-trusting.

Comment thread core/server_common/src/consensus_message.rs
Comment thread core/binary_protocol/src/consensus/header.rs Outdated
Comment thread core/binary_protocol/src/consensus/header.rs Outdated
Comment thread core/binary_protocol/src/consensus/header.rs Outdated
Comment thread core/binary_protocol/src/consensus/header.rs
Comment thread core/metadata/src/impls/metadata.rs
Comment thread core/server-ng/src/dispatch.rs Outdated
Comment thread core/binary_protocol/src/version.rs Outdated
@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 Aug 11, 2026
@numinnex
numinnex dismissed stale reviews from spetz and mmodzelewski via b1fea62 August 11, 2026 08:33
@numinnex

Copy link
Copy Markdown
Contributor Author

/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 Aug 11, 2026
@numinnex
numinnex merged commit 91672aa into master Aug 11, 2026
100 checks passed
@numinnex
numinnex deleted the remove_namespace_field branch August 11, 2026 09:50
@github-actions github-actions Bot removed the S-waiting-on-review PR is waiting on a reviewer label Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants