fix(cluster): anchor the reseed-marker clear to the marker, not the leader tail
A follower that latched `reseed_required` from a genuine `snapshot-required` refusal could clear its own marker ~200ms later and so never run the boot reseed that was the only way to close the gap. With `replication.reseed_self_restart: true` it exit-looped: latch -> clear -> exit(0) -> boot with no marker -> re-latch. Production tidaldb-0 did this 196 times in 21h on 2026-08-20 while the cluster ran on 2 of 3 voters. `clear_stale_reseed_marker_if_caught_up(applied >= leader_last_seq)` compared the applied frontier against the LEADER'S TAIL and documented the invariant "a node genuinely behind a COMPACTED gap never reaches caught_up". That is false: on a quiet shard any node meets the leader's tail, including one missing committed history it can never refetch. The clear also reset `tidaldb_cluster_reseed_required`, so the gauge flapped 1->0 every 30s and `TidalDBClusterReseedPending` (`== 1 for 10m`) could never fire - the code path that broke the reseed also erased the signal that would have reported it. The discharge decision now belongs to the marker. `ReseedMarker::discharged_by` requires a stream-dischargeable reason AND an applied frontier that reached the marker's own `from_seqno` - the very entry whose absence latched it. A compacted gap can never satisfy that, so the reseed runs; a node merely behind a shippable tail satisfies it as soon as the stream serves that entry, so the m12 false-alarm self-heal still works (and now clears sooner, since it no longer waits to meet a moving leader tail). The two conditions previously shared `ReseedReason::SnapshotRequired`, so reason alone could not discriminate. The term-join arm's `frontier > baseline` case deliberately sets `from_seqno = baseline`, BELOW the node's own frontier, so a bare `applied >= from_seqno` would discharge it instantly - it holds divergent post-baseline data only a snapshot can discard. It gets its own never-lag- dischargeable reason, `DivergentPostBaseline = 3`. Adding a discriminant is the sanctioned forward-only extension; a downgrade that meets one refuses to decode it, per the existing kind-3/kind-4 precedent. The election-won call site passed a hardcoded `true`; it now passes the leader's durable flushed frontier (`applied_seqno` never advances on a leader), and a `DivergentPostBaseline` node is not campaign-suppressed so it can reach there. Tests: three deterministic predicate tests pinning the incident's exact seqnos (13540653 vs earliest-available 13540657), the false-alarm discharge, and the never-discharge of every structural reason. Pre-existing and NOT introduced here: cluster_reseed's `mp_follower_reseeds_via_snapshot_after_compaction` and `mp_graceful_rolling_restart_under_load_no_reseed` fail on baseline main (verified by stashing this change). The first is the owner-test for this exact mechanism - its leader compaction no longer forces a `snapshot-required`, so it never reached the clear path and never guarded it. Tracked separately.
This commit is contained in:
jordan
parent
261d78d1f1
commit
da736b8eb2
@ -1,9 +1,10 @@
|
||||
# The tidalDB CLUSTER: ONE StatefulSet, every pod a region (m11p5 §4).
|
||||
#
|
||||
# MUTUALLY EXCLUSIVE with the standalone set in k8s/. Both source workloads are
|
||||
# parked at 0. `scripts/restore-fleet.sh` restores one selected data plane:
|
||||
# cardinality 1 in namespace `tidaldb`, or three `cluster --region` processes in
|
||||
# namespace `tidaldb-cluster` with real quorum-ack writes. Never run both.
|
||||
# MUTUALLY EXCLUSIVE with the standalone set in k8s/. THIS set is the production
|
||||
# data plane (three voters, quorum-ack writes, live since 2026-08-18); the
|
||||
# standalone set in `tidaldb` is SUPERSEDED and parked at 0. Never run both.
|
||||
# `scripts/restore-fleet.sh` is the guarded path for restoring either data plane
|
||||
# from the parked state — see the replicas note below for why that matters.
|
||||
#
|
||||
# WHY ONE StatefulSet (not one-per-region): the m11p5 bind/advertise split lets
|
||||
# every pod mount the SAME topology ConfigMap (peers are advertised by per-pod
|
||||
|
||||
@ -290,13 +290,8 @@ impl ElectionRuntime {
|
||||
JoinDecision::ReseedRequired => {
|
||||
// The node is BEHIND a leadership change: it lacks
|
||||
// `(own, prev_log]` of the previous stream — pre-baseline in the
|
||||
// new stream, never shippable. Latch the durable reseed marker
|
||||
// (reason `snapshot_required`: a pre-baseline gap, not a
|
||||
// divergent suffix) + the gauge, but DO NOT quarantine and DO
|
||||
// NOT block the join. The resume seqno is the first seqno this
|
||||
// node is missing in the PREVIOUS stream's numbering
|
||||
// (`own.frontier + 1`); the marker boot re-baselines onto the
|
||||
// leader's stream via the snapshot regardless.
|
||||
// new stream, never shippable. Latch the durable reseed marker +
|
||||
// the gauge, but DO NOT quarantine and DO NOT block the join.
|
||||
// m12p6: pick the resume seqno so the leader's `wal_covers` makes
|
||||
// the RIGHT needed-decision. When the node's frontier sits ABOVE the
|
||||
// baseline it has post-baseline old-term (DIVERGENT) data: `frontier
|
||||
@ -308,12 +303,22 @@ impl ElectionRuntime {
|
||||
// `frontier + 1` is correct and lets the leader's needed-decision
|
||||
// pick a cheap catch-up vs snapshot — forcing `baseline` here would
|
||||
// reseed an already-caught-up node and cascade restarts.
|
||||
let from_seqno = if position.frontier > baseline {
|
||||
baseline
|
||||
//
|
||||
// The REASON differs with the same test, and it is load-bearing for
|
||||
// the marker-clear path (2026-08-20 livelock): the divergent arm's
|
||||
// `from_seqno` is deliberately BELOW this node's own frontier, so a
|
||||
// clear predicate of `applied >= from_seqno` would discharge it
|
||||
// instantly — which is why that arm gets its own, never-lag-
|
||||
// dischargeable reason instead of sharing `SnapshotRequired`.
|
||||
let (reason, from_seqno) = if position.frontier > baseline {
|
||||
(ReseedReason::DivergentPostBaseline, baseline)
|
||||
} else {
|
||||
position.frontier.saturating_add(1)
|
||||
(
|
||||
ReseedReason::SnapshotRequired,
|
||||
position.frontier.saturating_add(1),
|
||||
)
|
||||
};
|
||||
node.latch_reseed_marker(ReseedReason::SnapshotRequired, from_seqno);
|
||||
node.latch_reseed_marker(reason, from_seqno);
|
||||
self.joined_term.store(term, Ordering::Release);
|
||||
true
|
||||
}
|
||||
|
||||
@ -2878,7 +2878,19 @@ impl ShardReplica {
|
||||
// (else a caught-up LEADER serves with `reseed_required:true` and reseeds on
|
||||
// its next restart). A quarantined node is campaign-suppressed and can never
|
||||
// reach here, so the helper's exclusion is doubly safe.
|
||||
self.clear_stale_reseed_marker_if_caught_up(true);
|
||||
//
|
||||
// Pass this shard's REAL applied frontier, not a hardcoded `true`: winning
|
||||
// proves the log beats a quorum's, which is not the same as proving a
|
||||
// specific compacted gap was closed, and a `DivergentPostBaseline` marker
|
||||
// is not campaign-suppressed so it CAN reach here. The marker decides.
|
||||
//
|
||||
// For a LEADER that frontier is the durable flushed WAL frontier, not
|
||||
// `applied_seqno` — a leader writes its WAL directly instead of applying
|
||||
// its own stream through the receiver, so `applied_seqno(own_shard)` stays
|
||||
// at its stale follower-era value (0 on a node elected without ever
|
||||
// following) and would refuse every discharge. Same derivation
|
||||
// `local_status` uses for the leader row.
|
||||
self.clear_stale_reseed_marker_if_discharged(self.ship_feed.flushed_seq());
|
||||
|
||||
// §3.2 — the activation membership record (the linchpin): once the
|
||||
// kind-3 marker is durable AND the membership era has begun (a kind-4
|
||||
@ -3125,18 +3137,23 @@ impl ShardReplica {
|
||||
/// then catches up via the normal stream / catch-up pull and never needs a
|
||||
/// snapshot reseed. The durable marker used to persist anyway, so the node
|
||||
/// reseeded on its next restart (and could even lead while still flagged — the
|
||||
/// observed `is_leader:true, lag:0, reseed_required:true` state). This clears it
|
||||
/// once the node has demonstrably caught up.
|
||||
/// observed `is_leader:true, lag:0, reseed_required:true` state).
|
||||
///
|
||||
/// Safe by construction: a node genuinely behind a COMPACTED gap never reaches
|
||||
/// `caught_up` (it cannot fetch the missing entries), so it keeps the marker
|
||||
/// and still reseeds; a QUARANTINED (divergent-suffix) node is excluded, its
|
||||
/// marker is real. So only a non-divergent node that actually reconverged
|
||||
/// clears — exactly the false-alarm case.
|
||||
fn clear_stale_reseed_marker_if_caught_up(&self, caught_up: bool) {
|
||||
if !caught_up {
|
||||
return;
|
||||
}
|
||||
/// The discharge decision belongs to the MARKER, not to this call site:
|
||||
/// [`ReseedMarker::discharged_by`] requires both a stream-dischargeable reason
|
||||
/// and an applied frontier that has reached the marker's own `from_seqno`.
|
||||
///
|
||||
/// The predicate used to be `applied >= leader_last_seq` — the leader's TAIL —
|
||||
/// with the stated invariant "a node genuinely behind a COMPACTED gap never
|
||||
/// reaches caught_up". Production falsified it on 2026-08-20: tidaldb-0 was
|
||||
/// short four entries the leader had already compacted away, met the tail
|
||||
/// comparison anyway, cleared its own marker ~200ms after latching it, and so
|
||||
/// never ran the boot reseed that was the only way to close the gap. With
|
||||
/// `reseed_self_restart` on, it exit-looped 196 times in 21h — and because
|
||||
/// the clear also reset `tidaldb_cluster_reseed_required`, the gauge flapped
|
||||
/// 1→0 every 30s and the `TidalDBClusterReseedPending` alert (`== 1 for 10m`)
|
||||
/// could never fire. Keep the comparison anchored to `from_seqno`.
|
||||
fn clear_stale_reseed_marker_if_discharged(&self, applied: u64) {
|
||||
if self
|
||||
.election_runtime
|
||||
.get()
|
||||
@ -3144,7 +3161,15 @@ impl ShardReplica {
|
||||
{
|
||||
return;
|
||||
}
|
||||
if !matches!(self.reseed_marker_store.load(), Ok(Some(_))) {
|
||||
let Ok(Some(marker)) = self.reseed_marker_store.load() else {
|
||||
return;
|
||||
};
|
||||
if !marker.discharged_by(applied) {
|
||||
// Refuse silently: this runs on every heartbeat (~300ms). The
|
||||
// undischarged state is already observable — the marker's latch logged
|
||||
// WARN once, `tidaldb_cluster_reseed_required` stays 1 (which is what
|
||||
// makes the 10m alert fire now), and `/cluster/status/local` reports
|
||||
// `reseed_required: true`.
|
||||
return;
|
||||
}
|
||||
match self.reseed_marker_store.clear() {
|
||||
@ -3155,9 +3180,12 @@ impl ShardReplica {
|
||||
self.reseed_marker_latched.store(false, Ordering::Release);
|
||||
tracing::info!(
|
||||
region = %self.region_name,
|
||||
"stale reseed marker cleared — caught up to the leader via the stream \
|
||||
without a reseed (false-alarm latch from a transient leadership-change \
|
||||
classification; a genuinely-compacted gap never reaches caught-up)"
|
||||
reason = marker.reason.as_str(),
|
||||
from_seqno = marker.from_seqno,
|
||||
applied,
|
||||
"stale reseed marker cleared — the stream served the entry that latched it \
|
||||
(applied >= from_seqno), so no reseed is needed (false-alarm latch from a \
|
||||
transient leadership-change classification)"
|
||||
);
|
||||
}
|
||||
Err(e) => tracing::warn!(
|
||||
@ -3476,10 +3504,12 @@ impl ShardReplica {
|
||||
if (self.install_boot || self.seed_joiner) && !self.converged.load(Ordering::Acquire) {
|
||||
self.note_lag_for_readiness(leader_last_seq.saturating_sub(applied));
|
||||
}
|
||||
// Self-heal a false-alarm reseed marker: this follower has caught up to the
|
||||
// leader's live frontier via the normal stream (no reseed happened), so a
|
||||
// marker a transient leadership-change classification latched is stale.
|
||||
self.clear_stale_reseed_marker_if_caught_up(applied >= leader_last_seq);
|
||||
// Self-heal a false-alarm reseed marker: if the stream served the very entry
|
||||
// whose absence latched it, no reseed is needed. Anchored to the marker's
|
||||
// `from_seqno`, NOT to `leader_last_seq` — see
|
||||
// `clear_stale_reseed_marker_if_discharged` for the livelock that comparison
|
||||
// caused.
|
||||
self.clear_stale_reseed_marker_if_discharged(applied);
|
||||
}
|
||||
|
||||
/// Whether this node is READY to serve (m11p5 §4 readiness predicate).
|
||||
|
||||
@ -82,6 +82,20 @@ pub enum ReseedReason {
|
||||
Quarantine = 1,
|
||||
/// An operator requested the reseed via `POST /cluster/reseed`.
|
||||
Operator = 2,
|
||||
/// A term-join found this node's frontier ABOVE the elected term's baseline:
|
||||
/// it holds post-baseline OLD-TERM data that only a snapshot can discard
|
||||
/// (`election_driver`'s `JoinDecision::ReseedRequired` with
|
||||
/// `frontier > baseline`).
|
||||
///
|
||||
/// Distinct from [`Self::Quarantine`]: that one is the m11p4 divergence
|
||||
/// quarantine, which also FENCES the node from the data plane and suppresses
|
||||
/// its campaigns. This node is neither fenced nor suppressed — it keeps
|
||||
/// serving and voting — but its marker is just as undischargeable by stream
|
||||
/// catch-up, which is exactly why it needs its own discriminant: while it
|
||||
/// shared `SnapshotRequired`, the marker-clear path could not tell "merely
|
||||
/// behind by a shippable tail" (dischargeable) from "holding divergent data"
|
||||
/// (never dischargeable) and cleared both.
|
||||
DivergentPostBaseline = 3,
|
||||
}
|
||||
|
||||
impl ReseedReason {
|
||||
@ -98,6 +112,7 @@ impl ReseedReason {
|
||||
0 => Ok(Self::SnapshotRequired),
|
||||
1 => Ok(Self::Quarantine),
|
||||
2 => Ok(Self::Operator),
|
||||
3 => Ok(Self::DivergentPostBaseline),
|
||||
other => Err(format!("unknown reseed reason {other}")),
|
||||
}
|
||||
}
|
||||
@ -109,8 +124,29 @@ impl ReseedReason {
|
||||
Self::SnapshotRequired => "snapshot_required",
|
||||
Self::Quarantine => "quarantine",
|
||||
Self::Operator => "operator",
|
||||
Self::DivergentPostBaseline => "divergent_post_baseline",
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether a marker with this reason can be discharged by ordinary stream
|
||||
/// catch-up — i.e. whether applying through the marker's `from_seqno` is
|
||||
/// PROOF that the node no longer needs a snapshot.
|
||||
///
|
||||
/// Only [`Self::SnapshotRequired`] qualifies, and even then only together
|
||||
/// with the `from_seqno` check in [`ReseedMarker::discharged_by`]: that reason
|
||||
/// records "the range starting at `from_seqno` was not shippable to me *at
|
||||
/// that moment*", which is true both of a genuinely compacted gap (the node
|
||||
/// can never apply through it, so the check keeps refusing and the reseed
|
||||
/// runs) and of a node merely behind at a leadership change (the stream
|
||||
/// serves it and the check discharges).
|
||||
///
|
||||
/// The other three are structural: a divergent post-baseline suffix, a
|
||||
/// quarantine, and an explicit operator request are all states no amount of
|
||||
/// forward progress resolves. A lag comparison must never discard them.
|
||||
#[must_use]
|
||||
pub const fn stream_dischargeable(self) -> bool {
|
||||
matches!(self, Self::SnapshotRequired)
|
||||
}
|
||||
}
|
||||
|
||||
/// The durable reseed marker's body: WHY the node latched it and the seqno the
|
||||
@ -126,6 +162,32 @@ pub struct ReseedMarker {
|
||||
pub from_seqno: u64,
|
||||
}
|
||||
|
||||
impl ReseedMarker {
|
||||
/// Whether an applied frontier of `applied` PROVES this marker no longer
|
||||
/// needs a snapshot reseed.
|
||||
///
|
||||
/// Two conditions, both required:
|
||||
///
|
||||
/// 1. the reason must be [`ReseedReason::stream_dischargeable`], and
|
||||
/// 2. the node must have applied THROUGH `from_seqno` — the very entry whose
|
||||
/// unavailability latched the marker.
|
||||
///
|
||||
/// Condition 2 is the one that makes this safe against a genuinely compacted
|
||||
/// gap: the leader refused the range *starting at* `from_seqno`, so those
|
||||
/// entries exist in no live WAL and `applied` can never reach it without a
|
||||
/// snapshot install. Comparing against the leader's TAIL instead (the
|
||||
/// pre-fix behavior) discharged the marker on any node whose frontier
|
||||
/// happened to meet the leader's last seqno, which on a quiet shard is every
|
||||
/// node — including one missing committed history it could never refetch.
|
||||
/// That inverted the guarantee: the clear suppressed the only action that
|
||||
/// closes the gap, so the node re-latched, re-cleared, and (with
|
||||
/// `reseed_self_restart`) exit-looped forever without ever reseeding.
|
||||
#[must_use]
|
||||
pub const fn discharged_by(self, applied: u64) -> bool {
|
||||
self.reason.stream_dischargeable() && applied >= self.from_seqno
|
||||
}
|
||||
}
|
||||
|
||||
/// Reader/writer for the durable reseed marker (`data_dir/reseed_required`).
|
||||
///
|
||||
/// Same file discipline as [`ElectionStore`](super::election_store::ElectionStore)
|
||||
@ -307,6 +369,7 @@ mod tests {
|
||||
ReseedReason::SnapshotRequired,
|
||||
ReseedReason::Quarantine,
|
||||
ReseedReason::Operator,
|
||||
ReseedReason::DivergentPostBaseline,
|
||||
] {
|
||||
let marker = ReseedMarker {
|
||||
reason,
|
||||
@ -317,6 +380,73 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// The compacted-gap livelock regression (2026-08-20 incident): a
|
||||
/// `SnapshotRequired` marker latched because the leader's WAL was compacted
|
||||
/// below `from_seqno` must NOT discharge while the node is still short of
|
||||
/// that seqno. The pre-fix predicate compared the applied frontier against
|
||||
/// the LEADER'S TAIL, so a node missing four unrefetchable entries cleared
|
||||
/// its own marker, skipped the reseed on the next boot, re-latched, and
|
||||
/// exit-looped 196 times over 21h without ever reseeding.
|
||||
#[test]
|
||||
fn compacted_gap_marker_never_discharges_below_from_seqno() {
|
||||
let marker = ReseedMarker {
|
||||
reason: ReseedReason::SnapshotRequired,
|
||||
from_seqno: 13_540_653,
|
||||
};
|
||||
// The live incident's exact numbers: the follower held 13_540_652 and the
|
||||
// leader's earliest available was 13_540_657, so the requested entry is
|
||||
// gone for good. Every frontier below `from_seqno` must refuse.
|
||||
assert!(!marker.discharged_by(13_540_652));
|
||||
assert!(!marker.discharged_by(0));
|
||||
// Even a frontier at/above the LEADER'S TAIL (13_540_660 at the time)
|
||||
// must refuse while it is below `from_seqno` — impossible here by
|
||||
// construction, but it is the comparison the bug actually made, so pin
|
||||
// that the predicate no longer consults anything but `from_seqno`.
|
||||
assert!(!marker.discharged_by(marker.from_seqno - 1));
|
||||
// Applying THROUGH the missing entry is the only proof, and it discharges.
|
||||
assert!(marker.discharged_by(13_540_653));
|
||||
assert!(marker.discharged_by(13_540_999));
|
||||
}
|
||||
|
||||
/// The false-alarm case the m12 reseed-loop-fix targeted still self-heals:
|
||||
/// a node merely behind a SHIPPABLE tail at a leadership change latches
|
||||
/// `SnapshotRequired` with `from_seqno = frontier + 1`, then the stream
|
||||
/// serves exactly that entry and the marker clears with no reseed.
|
||||
#[test]
|
||||
fn merely_behind_marker_discharges_once_the_tail_is_applied() {
|
||||
let marker = ReseedMarker {
|
||||
reason: ReseedReason::SnapshotRequired,
|
||||
from_seqno: 501, // latched at frontier 500
|
||||
};
|
||||
assert!(!marker.discharged_by(500));
|
||||
assert!(marker.discharged_by(501));
|
||||
}
|
||||
|
||||
/// Structural markers are never dischargeable by forward progress, whatever
|
||||
/// the frontier: a divergent post-baseline suffix must be DISCARDED by a
|
||||
/// snapshot, a quarantine is a fence, and an operator request is an
|
||||
/// instruction. `u64::MAX` stands in for "arbitrarily caught up".
|
||||
#[test]
|
||||
fn structural_reasons_never_discharge_by_lag() {
|
||||
for reason in [
|
||||
ReseedReason::DivergentPostBaseline,
|
||||
ReseedReason::Quarantine,
|
||||
ReseedReason::Operator,
|
||||
] {
|
||||
assert!(!reason.stream_dischargeable(), "{}", reason.as_str());
|
||||
let marker = ReseedMarker {
|
||||
reason,
|
||||
from_seqno: 10,
|
||||
};
|
||||
assert!(
|
||||
!marker.discharged_by(u64::MAX),
|
||||
"{} discharged by lag",
|
||||
reason.as_str()
|
||||
);
|
||||
}
|
||||
assert!(ReseedReason::SnapshotRequired.stream_dischargeable());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn corruption_refuses() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
||||
Loading…
Reference in New Issue
Block a user