Skip to content

feat(consensus): persist metadata VSR state in a durable superblock - #3767

Open
krishvishal wants to merge 5 commits into
masterfrom
vsrconsensus-persist
Open

feat(consensus): persist metadata VSR state in a durable superblock#3767
krishvishal wants to merge 5 commits into
masterfrom
vsrconsensus-persist

Conversation

@krishvishal

@krishvishal krishvishal commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

A metadata replica kept its consensus numbers only in RAM. After a crash it
rebuilt at view = 0 and inferred a view from the last WAL prepare, so it could
re-enter a superseded view and re-vote, splitting the log. Sessions and the
at-most-once dedup cache were volatile too, so a returning client hit
NoSession and a retried request re-executed. And once a checkpoint drained the
WAL prefix, nothing durably identified the snapshot it folded into.

What

journal::superblock (new). SuperblockStore trait plus PingPongSuperblock:
two files written alternately, each replaced temp, fsync, rename, dir-fsync,
highest valid sequence wins. Rename is atomic, so an update can never destroy
the last good record. read_latest returns three outcomes, kept distinct because
collapsing them into an Option is exactly what lets a lost superblock
masquerade as a fresh deployment:

Outcome Meaning Response
Empty never written fresh boot
Present(payload) newest checksum-clean record recover from it
Unreadable { version } written, now torn / foreign / unknown version refuse boot

consensus::VsrState (new). The 58-byte little-endian payload: cluster,
replica id and count, view, log_view, commit_max, and the paired
checkpoint_op / checkpoint_checksum. Offsets pinned by test.

Durable-before-send gate. VsrConsensus tracks view_durable /
log_view_durable, exposing needs_superblock_persist() and
mark_superblock_durable(view, log_view), which takes the values written rather
than re-reading self.view (the in-memory view can advance across the write's
await while a checkpoint holds the lock). Every metadata dispatch site (SVC /
DVC / StartView / RequestStartView, the tick, the stale-heartbeat reply,
send_prepare_ok) persists first and withholds on failure. A debug_assertions
tripwire in dispatch_vsr_actions asserts callers did so. RequestStartView is
exempt: it is a probe asking to learn the view.

Three-phase checkpoint. Persist snapshot, record the pairing, then drain the
WAL. A crash before the pairing write recovers the prior checkpoint with the WAL
intact; after it, the new one. One LocalGate serializes checkpoints against
view-change persists, since both drive the single superblock, whose write picks
its slot before it awaits, and in-process submits each run on their own task.

Client-table durability. ClientTableSnapshot folds into MetadataSnapshot
under #[serde(default)], preserving slot positions so eviction order survives.
Boot restores it from the snapshot and replays the committed suffix through the
same apply_committed_prepare the live commit walk uses.

Verify-then-decode recovery. Recovery cross-checks the superblock against the
on-disk snapshot before trusting its contents. New typed refusals: superblock
ahead of snapshot, checksum mismatch at the same op, undecodable payload,
unreadable on every slot. A superblock lagging a newer atomically-complete
snapshot is accepted (commit_max is only a recovery lower bound). An absent
superblock stays a fresh boot; refusing there would brick healthy nodes.

WAL body integrity. The primary seals checksum_body once and replicates it
verbatim. The scan truncates a genuinely torn tail but refuses boot on interior
bit-rot rather than discarding the committed entries behind it.

Incarnation nonce. RequestStartView carries a per-boot u128; the answering
StartView echoes it. While Recovering, a replica only adopts a StartView
provably post-restart.

Wire compatibility

StartViewHeader and RequestStartViewHeader gain incarnation: u128, carved
from the tail of their existing reserved regions. Placed last and
16-aligned, so op / commit / namespace keep their offsets and the structs
stay padding-free (static asserts enforce both). A peer that predates the field
sends zeros, decoding as incarnation == 0, inert per the adoption guard. Header
size unchanged, so a mixed-version rolling upgrade is safe.

Not in scope

  • Partition-plane consensus is not superblock-backed and is exempt from the gate.
  • No metadata state transfer, so a corrupt checkpoint refuses boot rather than
    repairing from a peer. Marked TODO(state-transfer) where the fetch belongs.
  • Header checksum and its parent chain stay unsealed: activating them needs
    the retransmit path to re-seal a re-stamped header.
  • checksum_body has no format-version gate, so a WAL written before this change
    refuses boot. Fail-safe, but a hard upgrade break; TODO(wal-integrity).

@numinnex numinnex 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.

Two blockers, tagged inline (reviewed in the context of the upcoming metadata state transfer work, which builds on this).

Comment thread core/journal/src/prepare_journal.rs Outdated
Comment thread core/journal/src/superblock.rs Outdated
Comment thread core/journal/src/superblock.rs Outdated
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.14719% with 320 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.76%. Comparing base (53a7ff9) to head (b6c8d72).

Files with missing lines Patch % Lines
core/metadata/src/impls/recovery.rs 80.05% 68 Missing and 8 partials ⚠️
core/server-ng/src/bootstrap.rs 20.45% 70 Missing ⚠️
core/metadata/src/impls/metadata.rs 82.68% 40 Missing and 9 partials ⚠️
core/consensus/src/client_table.rs 80.99% 33 Missing and 9 partials ⚠️
core/journal/src/superblock.rs 91.39% 14 Missing and 15 partials ⚠️
core/simulator/src/deps.rs 76.78% 9 Missing and 4 partials ⚠️
core/shard/src/lib.rs 72.97% 10 Missing ⚠️
core/journal/src/prepare_journal.rs 96.58% 1 Missing and 7 partials ⚠️
core/consensus/src/vsr_state.rs 92.13% 7 Missing ⚠️
core/simulator/src/lib.rs 97.68% 4 Missing and 3 partials ⚠️
... and 2 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3767      +/-   ##
============================================
+ Coverage     75.58%   75.76%   +0.18%     
  Complexity      969      969              
============================================
  Files          1317     1319       +2     
  Lines        155244   156955    +1711     
  Branches     128866   130658    +1792     
============================================
+ Hits         117339   118923    +1584     
- Misses        34361    34377      +16     
- Partials       3544     3655     +111     
Components Coverage Δ
Rust Core 75.82% <86.14%> (+0.27%) ⬆️
Java SDK 62.71% <ø> (ø)
C# SDK 71.16% <ø> (-1.17%) ⬇️
Python SDK 92.27% <ø> (ø)
PHP SDK 84.52% <ø> (ø)
Node SDK 95.31% <ø> (+0.11%) ⬆️
Go SDK 43.08% <ø> (ø)
Files with missing lines Coverage Δ
core/binary_protocol/src/consensus/header.rs 81.23% <ø> (+0.87%) ⬆️
core/consensus/src/lib.rs 0.00% <ø> (ø)
core/journal/src/lib.rs 25.00% <100.00%> (+25.00%) ⬆️
core/metadata/src/stm/snapshot.rs 86.63% <100.00%> (+0.10%) ⬆️
core/metadata/src/stm/stream.rs 65.59% <100.00%> (+3.60%) ⬆️
core/metadata/src/stm/user.rs 87.95% <ø> (ø)
core/server-ng/src/dispatch.rs 39.85% <100.00%> (+0.04%) ⬆️
core/server-ng/src/partition_reconciler.rs 87.33% <100.00%> (ø)
core/consensus/src/impls.rs 83.85% <97.61%> (+8.22%) ⬆️
core/simulator/src/replica.rs 96.35% <90.90%> (-3.65%) ⬇️
... and 10 more

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

@krishvishal
krishvishal force-pushed the vsrconsensus-persist branch from c1699cf to 5fa9cf9 Compare July 30, 2026 12:14

@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.

three things worth filing separately - all outside this diff's hunks, so they cannot be line comments:

  • core/metadata/src/impls/metadata.rs:988-1003 - the "journal already holds this op, re-forward and re-ack it" fast path compares nothing between the stored and the incoming header. it forwards the NEW bytes, keeps the OLD bytes locally, and acks the NEW op. replicate_preflight passes on equal view, panic_if_hash_chain_would_break_in_same_view is bypassed, and commit_journal then applies the stale body as committed with the vote counted toward quorum. fence_old_prepare_by_commit fences on commit_min, not commit_max, so it is narrower than it looks, and send_prepare_ok's persisted check is the same op-only lookup, so the ack asserts durability of the new op while the WAL holds the old bytes. live on master today and independent of this PR, but materially more reachable once log_view is durable, since that is exactly the path that makes an un-truncated local suffix survive a restart into a live view. this diff supplies the fix for free - compare checksum_body (letting CHECKSUM_BODY_UNSEALED fall through) before re-acking.

  • the metadata WAL has no uncommitted-suffix truncation primitive. that is the root cause of both the log_view comment on bootstrap.rs:1962 and the re-ack path above, and it deserves its own issue.

  • core/consensus/src/observability.rs:347 - SendStartView { op, commit, .. } drops incarnation from the control-action record, so the sim oracle is blind to exactly the probe-reply-vs-unsolicited distinction the new guard rests on.

the core design holds up, for what it is worth: gate coverage is complete across every metadata emitter of SVC/DVC/StartView/PrepareOk, the three-phase ordering is right, lock discipline has no inversion, view/log_view are monotone so the durable value cannot regress, and a pre-change WAL still boots through the sentinel. placing client_table LAST in MetadataSnapshot is what makes the forward decode work at all - a mid-struct insert would have silently misassigned fields by position. every blocker above sits in the recovery half, not the gate.

Comment thread core/server-ng/src/bootstrap.rs
Comment thread core/metadata/src/impls/metadata.rs Outdated
Comment thread core/server-ng/src/bootstrap.rs
Comment thread core/consensus/src/vsr_state.rs Outdated
Comment thread core/consensus/src/impls.rs Outdated
Comment thread core/server-ng/src/bootstrap.rs Outdated
Comment thread core/metadata/src/impls/metadata.rs
Comment thread core/journal/src/prepare_journal.rs Outdated
Comment thread core/integration/Cargo.toml Outdated
Comment thread core/shard/src/lib.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 Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author PR is waiting on author response

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants