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 3 commits into
Conversation
ec7aa18 to
c1699cf
Compare
numinnex
left a comment
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).
| body_buf = vec![0u8; body_len]; | ||
| } | ||
| body_buf = storage.read_at(pos + HEADER_SIZE as u64, body_buf).await?; | ||
| if u128::from(XxHash3_64::oneshot(&body_buf)) != header.checksum_body { |
There was a problem hiding this comment.
Blocker: no format gate on checksum_body, so any pre-existing WAL refuses boot.
Every entry written before this change carries checksum_body == 0, so this check fails all of them and the interior-corruption branch below refuses boot. It also breaks mixed-version clusters: an old-version primary sends zero-checksum prepares, a new-version backup journals them verbatim, and that backup's next restart refuses boot, so a rolling upgrade is unsafe in that direction.
The TODO(wal-integrity) above already names this gap, but I think it needs to be part of this PR rather than a follow-up: treat checksum_body == 0 as unsealed and skip verification for that entry, or gate on a WAL entry format version.
| payload: payload_b, | ||
| }, | ||
| ) => SuperblockContents::Present(if seq_a >= seq_b { payload_a } else { payload_b }), | ||
| (SlotClass::Valid { payload, .. }, _) | (_, SlotClass::Valid { payload, .. }) => { |
There was a problem hiding this comment.
Blocker: (Valid, Corrupt) silently regresses durable state on newest-slot bit-rot.
A torn write dies in the .tmp (rename is atomic), so a corrupt non-temp slot can only be bit-rot. When that hits the newest slot, this arm falls back to the older generation: older view/log_view, and the replica can re-vote in a view it already acted in, which is exactly the split-brain this PR exists to prevent.
Since the corrupt slot's sequence is unreadable, there is no way to tell whether it was the newer one, so I believe the sound response for Valid + Corrupt is refusing boot (Unreadable), the same as both-corrupt.
| } | ||
| } | ||
|
|
||
| async fn read_sequence(path: &Path) -> io::Result<Option<u64>> { |
There was a problem hiding this comment.
Compounding the combine_slots fallback above: read_sequence maps both Absent and Corrupt to None, so open aims the next write at the corrupt (possibly newest) slot and overwrites the evidence with sequence = valid + 1, retroactively legitimizing the regression. Distinguishing Absent from Corrupt here would fall out of the same fix.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3767 +/- ##
============================================
+ Coverage 75.53% 75.62% +0.08%
Complexity 969 969
============================================
Files 1317 1319 +2
Lines 154895 156131 +1236
Branches 128519 129833 +1314
============================================
+ Hits 117004 118070 +1066
- Misses 34344 34380 +36
- Partials 3547 3681 +134
🚀 New features to boost your workflow:
|
c1699cf to
5fa9cf9
Compare
2 participants
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.
Determinism invariant
Recovery trusts a checkpoint by recomputing
hash(encode(decode(disk)))againstthe persist-time
hash(encode(state)). That holds only while every collection inthe serialized form keeps a deterministic order, so an unordered map anywhere in
MetadataSnapshotwould make a healthy node refuse boot after a restart.Documented on the three snapshot types and guarded by two byte-stability tests.
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).