feat(consensus): persist metadata VSR state in a durable superblock - #3767
feat(consensus): persist metadata VSR state in a durable superblock#3767krishvishal wants to merge 5 commits into
Conversation
ec7aa18 to
c1699cf
Compare
numinnex
left a comment
There was a problem hiding this comment.
Two blockers, tagged inline (reviewed in the context of the upcoming metadata state transfer work, which builds on this).
Codecov Report❌ Patch coverage is 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
🚀 New features to boost your workflow:
|
c1699cf to
5fa9cf9
Compare
hubcio
left a comment
There was a problem hiding this comment.
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_preflightpasses on equal view,panic_if_hash_chain_would_break_in_same_viewis bypassed, andcommit_journalthen applies the stale body as committed with the vote counted toward quorum.fence_old_prepare_by_commitfences oncommit_min, notcommit_max, so it is narrower than it looks, andsend_prepare_ok'spersistedcheck 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 oncelog_viewis 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 - comparechecksum_body(lettingCHECKSUM_BODY_UNSEALEDfall through) before re-acking. -
the metadata WAL has no uncommitted-suffix truncation primitive. that is the root cause of both the
log_viewcomment onbootstrap.rs:1962and the re-ack path above, and it deserves its own issue. -
core/consensus/src/observability.rs:347-SendStartView { op, commit, .. }dropsincarnationfrom 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.
A metadata replica kept its consensus numbers only in RAM. After a crash it
rebuilt at
view = 0and inferred a view from the last WAL prepare, so it couldre-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
NoSessionand a retried request re-executed. And once a checkpoint drained theWAL prefix, nothing durably identified the snapshot it folded into.
What
journal::superblock(new).SuperblockStoretrait plusPingPongSuperblock:two files written alternately, each replaced
temp, fsync, rename, dir-fsync,highest valid
sequencewins. Rename is atomic, so an update can never destroythe last good record.
read_latestreturns three outcomes, kept distinct becausecollapsing them into an
Optionis exactly what lets a lost superblockmasquerade as a fresh deployment:
EmptyPresent(payload)Unreadable { version }consensus::VsrState(new). The 58-byte little-endian payload: cluster,replica id and count,
view,log_view,commit_max, and the pairedcheckpoint_op/checkpoint_checksum. Offsets pinned by test.Durable-before-send gate.
VsrConsensustracksview_durable/log_view_durable, exposingneeds_superblock_persist()andmark_superblock_durable(view, log_view), which takes the values written ratherthan re-reading
self.view(the in-memory view can advance across the write'sawait 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. Adebug_assertionstripwire in
dispatch_vsr_actionsasserts callers did so.RequestStartViewisexempt: 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
LocalGateserializes checkpoints againstview-change persists, since both drive the single superblock, whose
writepicksits slot before it awaits, and in-process submits each run on their own task.
Client-table durability.
ClientTableSnapshotfolds intoMetadataSnapshotunder
#[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_preparethe 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_maxis only a recovery lower bound). An absentsuperblock stays a fresh boot; refusing there would brick healthy nodes.
WAL body integrity. The primary seals
checksum_bodyonce and replicates itverbatim. The scan truncates a genuinely torn tail but refuses boot on interior
bit-rot rather than discarding the committed entries behind it.
Incarnation nonce.
RequestStartViewcarries a per-bootu128; the answeringStartViewechoes it. WhileRecovering, a replica only adopts aStartViewprovably post-restart.
Wire compatibility
StartViewHeaderandRequestStartViewHeadergainincarnation: u128, carvedfrom the tail of their existing
reservedregions. Placed last and16-aligned, so
op/commit/namespacekeep their offsets and the structsstay padding-free (static asserts enforce both). A peer that predates the field
sends zeros, decoding as
incarnation == 0, inert per the adoption guard. Headersize unchanged, so a mixed-version rolling upgrade is safe.
Not in scope
repairing from a peer. Marked
TODO(state-transfer)where the fetch belongs.checksumand itsparentchain stay unsealed: activating them needsthe retransmit path to re-seal a re-stamped header.
checksum_bodyhas no format-version gate, so a WAL written before this changerefuses boot. Fail-safe, but a hard upgrade break;
TODO(wal-integrity).