Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 34 additions & 6 deletions core/binary_protocol/src/consensus/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1048,15 +1048,29 @@ pub struct StartViewHeader {
/// max(commit) from all DVCs.
pub commit: u64,
pub namespace: u64,
pub reserved: [u8; 104],
pub reserved: [u8; 88],
/// Sender's incarnation, echoed from the `RequestStartView` this answers so a
/// recovering replica can prove the reply post-dates its restart (see
/// `RequestStartViewHeader::incarnation`). `0` on an unsolicited `StartView`
/// (a normal view-change completion), which carries no freshness claim.
///
/// Carved from the tail of the former `reserved` region and placed LAST so it
/// lands 16-aligned with no padding WITHOUT moving `op`/`commit`/`namespace`.
/// A peer that predates it sends zeros, decoding as `incarnation == 0`, inert
/// per the `handle_start_view` guard, so a mixed-version rolling upgrade is

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.

not inert on the metadata plane. the handle_start_view guard only ignores incarnation when self_incarnation == 0, and production always sets it non-zero (rand::random::<u128>() | 1, bootstrap.rs:1946). a zero-incarnation StartView from a pre-upgrade peer has header.incarnation != self_incarnation, so it gets classified stale whenever the other three conditions hold.

the escape hatches cover the common failover shapes so reachability is low, but the claim as written is not right. treating header.incarnation == 0 as "no claim" and falling through would make it true.

/// wire-compatible.
pub incarnation: u128,
}
const _: () = {
assert!(size_of::<StartViewHeader>() == HEADER_SIZE);
// op/commit/namespace keep their pre-incarnation offsets.
assert!(
offset_of!(StartViewHeader, op)
== offset_of!(StartViewHeader, reserved_frame) + size_of::<[u8; 66]>()
);
assert!(offset_of!(StartViewHeader, reserved) + size_of::<[u8; 104]>() == HEADER_SIZE);
// `incarnation` is last and 16-aligned, so the struct has no padding.
assert!(offset_of!(StartViewHeader, incarnation) + size_of::<u128>() == HEADER_SIZE);
assert!(offset_of!(StartViewHeader, incarnation) % 16 == 0);
};

impl ConsensusHeader for StartViewHeader {
Expand Down Expand Up @@ -1097,8 +1111,11 @@ impl ConsensusHeader for StartViewHeader {
/// Recovering replica -> all replicas: resend me the current `StartView`.
///
/// Header-only; only the current view's primary answers, with a targeted
/// `StartView` (adoption is fenced by the receiver's view monotonicity and
/// the sender-is-primary check, so no nonce is needed).
/// `StartView`. Adoption is fenced by the receiver's view monotonicity and the
/// sender-is-primary check; `incarnation` additionally proves the reply
/// post-dates this replica's restart, so a `StartView` from a previous
/// incarnation still in flight cannot be adopted (see the `handle_start_view`
/// recovering-status guard).
#[derive(Debug, Clone, Copy, PartialEq, Eq, CheckedBitPattern, NoUninit)]
#[repr(C)]
pub struct RequestStartViewHeader {
Expand All @@ -1113,15 +1130,26 @@ pub struct RequestStartViewHeader {
pub reserved_frame: [u8; 66],

pub namespace: u64,
pub reserved: [u8; 120],
pub reserved: [u8; 104],
/// The requester's per-boot incarnation, echoed back in the answering
/// `StartView` so a reply from a previous incarnation is detectable.
///
/// Carved from the tail of the former `reserved` region and placed LAST so it
/// lands 16-aligned with no padding WITHOUT moving `namespace`. A peer that
/// predates it sends zeros, decoding as `incarnation == 0`, so a mixed-version
/// rolling upgrade is wire-compatible.
pub incarnation: u128,
}
const _: () = {
assert!(size_of::<RequestStartViewHeader>() == HEADER_SIZE);
// namespace keeps its pre-incarnation offset.
assert!(
offset_of!(RequestStartViewHeader, namespace)
== offset_of!(RequestStartViewHeader, reserved_frame) + size_of::<[u8; 66]>()
);
assert!(offset_of!(RequestStartViewHeader, reserved) + size_of::<[u8; 120]>() == HEADER_SIZE);
// `incarnation` is last and 16-aligned, so the struct has no padding.
assert!(offset_of!(RequestStartViewHeader, incarnation) + size_of::<u128>() == HEADER_SIZE);
assert!(offset_of!(RequestStartViewHeader, incarnation) % 16 == 0);
};

impl ConsensusHeader for RequestStartViewHeader {
Expand Down
1 change: 1 addition & 0 deletions core/consensus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ iggy_common = { workspace = true }
message_bus = { workspace = true }
rand = { workspace = true }
rand_xoshiro = { workspace = true }
serde = { workspace = true }
server_common = { workspace = true }
tracing = { workspace = true }

Expand Down
Loading
Loading