refactor(consensus): remove namespace from client headers - #3836
Conversation
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>
Codecov Report❌ Patch coverage is 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
🚀 New features to boost your workflow:
|
hubcio
left a comment
There was a problem hiding this comment.
-
cached client-table replies persist as raw
ReplyHeaderbytes in the checkpoint and the state-transfer artifact, and this PR movedstatus224 -> 216. correct today only by coincidence: every cached reply is metadata-plane so the oldnamespaceword was1<<63with a zero low half, which the relocatedstatusreads 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::newmasks (& 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 falliblechecked_newat 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.
|
/ready |
Removes the client-supplied
namespaceword from the VSR client request and reply headers. The server derives the routing target itself: the plane fromoperation, the partition group from the request body. At the admission boundary a request is promoted to the server-internalRoutedRequestHeader, 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::validatenow enforces the same field rules as the client boundary (sharedvalidate_request_fields), so a peer-wireCommand2::Requestframe cannot carryclient = 0into the client table's hard assert or replay a cached register reply viaoperation = Reserved.Message::into_routed) that zeroes only the eightgroupbytes instead of rebuilding the 256-byte header; the terminal validate stays.ReplyHeaderbytes, and this PR movesstatusfrom 224 to 216. Both persistence surfaces are versioned: the state-transfer artifact magic is bumped toICT2, andMetadataSnapshot::decoderefuses snapshots whoseversiondiffers from the current format version (bumped to 2).ReplyHeader::statusat offset 216,RoutedRequestHeader::groupat 248..256, and the data-bearing reserved prefix surviving promotion.