From 54d1353103a638283b3f04668b121e96d9ac8266 Mon Sep 17 00:00:00 2001 From: jordan Date: Fri, 21 Aug 2026 03:01:58 -0600 Subject: [PATCH] fix(cluster): make a multi-group node's status readable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two status defects turned this incident into a day of misreading. Both are observability, both are why the functional bug survived, and neither changes readiness or replication behaviour. 1. PER-GROUP RESEED STATE. `reseed_required` / `reseeding` existed only as flat fields on LocalStatusResponse, and `status_local` fills those from `replica_for(sel.shard_id())` — the LOWEST hosted group id when no `?shard=` is given. On a 3-group node they therefore describe one group and say nothing about the other two. tidaldb-0 answered `reseed_required: false` while a different hosted group sat behind a compacted leader, and every operator reading and every diagnosis in this incident took that as converged. ShardStatusRow now carries both per group. 2. `lag_events: 0` WAS UNREADABLE. Lag is `leader_seqno_for(key) - applied`, an unsigned subtraction against a gauge that is 0 until this node learns the leader's frontier. A freshly-booted node that knows NOTHING computes 0 - 0 = 0 and reports itself perfectly caught up. Measured in the multi-group repro at the moment the node declared itself settled: shard 0: applied_events 24, lag_events 0, leader us-east shard 1: applied_events 14, lag_events 0, leader eu-west shard 2: applied_events 0, lag_events 0, leader null 5600 items had been written. All three groups claimed zero lag. Expose `leader_seqno` (the value lag subtracts from) on both the flat response and each shard row, so `lag_events: 0` with `leader_seqno: 0` reads as NO INFORMATION rather than converged. This is additive: `lag_events` keeps its value and readiness keeps its semantics, deliberately, because changing the readiness predicate during a live incident is not a change worth bundling here. Also tightens the multi-group repro's settle predicate to require EVERY hosted group's row to be clean. The first version trusted the flat fields, so it announced "settled after 0 restarts" and then failed the content probe — fooled by exactly the under-reporting above. --- tidal-server/src/cluster/node.rs | 61 ++++++++++++++++++++++++++-- tidal-server/tests/cluster_reseed.rs | 22 +++++++++- 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/tidal-server/src/cluster/node.rs b/tidal-server/src/cluster/node.rs index 8454564..b0fa737 100644 --- a/tidal-server/src/cluster/node.rs +++ b/tidal-server/src/cluster/node.rs @@ -3718,13 +3718,26 @@ impl ShardReplica { // that still holds a previously-followed leader's HWM (which left a // converged node reporting a permanent phantom lag). A leader has zero lag // against itself. - let lag_events = if is_leader { - 0 + // This node's KNOWLEDGE of the leader's frontier for the group it follows. + // Surfaced (as `leader_seqno`) because `lag_events` alone is ambiguous: it + // is an unsigned subtraction against this gauge, so a freshly-booted node + // that has learned NOTHING reports `0 - 0 = 0` — indistinguishable from + // genuinely caught up. That ambiguity is what made the 2026-08-20 incident + // unreadable: every group answered `lag_events: 0` while holding + // `applied_events` of 24, 14 and 0 against 5600 written items. With this + // exposed, `lag_events: 0 AND leader_seqno: 0` reads as "no information", + // which is the truth. + let leader_seqno = if is_leader { + last_seq } else { db.control_plane() .lag_gauge() .leader_seqno_for(leader_shard) - .saturating_sub(applied_events) + }; + let lag_events = if is_leader { + 0 + } else { + leader_seqno.saturating_sub(applied_events) }; let partitioned: Vec = read_recovered(&self.partitioned, "partitioned") @@ -3792,6 +3805,7 @@ impl ShardReplica { election_tail_term: election_pos.tail_term, election_frontier: election_pos.frontier, leader_acked: self.leader_acked_frontier(), + leader_seqno, reseed_required, reseeding, self_restart_refused: self.self_restart_refused.load(Ordering::Acquire), @@ -4732,8 +4746,19 @@ impl ClusterNode { term: s.term, role: s.role, applied_events: s.applied_events, + leader_seqno: s.leader_seqno, lag_events: s.lag_events, commit_index: s.commit_index, + // PER-GROUP reseed state. The flat `reseed_required` / + // `reseeding` fields above describe ONE group — whichever + // `replica_for(None)` resolves (the lowest id) — so on a + // multi-group node they say nothing about the others. That is + // how the 2026-08-20 incident hid: tidaldb-0 answered + // `reseed_required: false, lag_events: 0` while a different + // hosted group was stuck behind a compacted leader, and every + // operator (and every diagnosis) read it as converged. + reseed_required: s.reseed_required, + reseeding: s.reseeding, }) }) .collect() @@ -5136,6 +5161,17 @@ pub struct LocalStatusResponse { /// new leadership's election baseline. Exposed for divergence diagnosis. #[serde(default)] leader_acked: u64, + /// This node's KNOWLEDGE of the followed group's leader frontier — the value + /// `lag_events` is subtracted from. + /// + /// Publish it because `lag_events` alone cannot be read: it is an unsigned + /// subtraction against this number, so a node that has learned nothing reports + /// `0 - 0 = 0` and looks perfectly caught up. In the 2026-08-20 incident every + /// group answered `lag_events: 0` while holding `applied_events` of 24, 14 and + /// 0 against 5600 written items. `lag_events: 0` together with + /// `leader_seqno: 0` means NO INFORMATION, not converged. + #[serde(default)] + leader_seqno: u64, /// Whether this node is quarantined with a divergent suffix (m11p4): /// fenced from the data plane until reseeded. quarantined: bool, @@ -5194,10 +5230,29 @@ pub struct ShardStatusRow { role: String, /// Replication events this node has applied for this group. applied_events: u64, + /// This node's knowledge of THIS GROUP's leader frontier — the value + /// `lag_events` subtracts from. `lag_events: 0` with `leader_seqno: 0` means + /// NO INFORMATION (a node that has learned nothing), not converged. + #[serde(default)] + leader_seqno: u64, /// Events this node lags this group's leader by (0 when leading). lag_events: u64, /// This group's quorum commit index (meaningful when leading). commit_index: u64, + /// Whether THIS GROUP has a durable reseed marker latched. + /// + /// Load-bearing on a multi-group node: the flat `reseed_required` on + /// [`LocalStatusResponse`] reports only the group `replica_for(None)` picks + /// (the lowest hosted id), so a marker on any OTHER group was invisible in the + /// status surface. That is how the 2026-08-20 incident hid — tidaldb-0 answered + /// `reseed_required: false, lag_events: 0` while another hosted group sat behind + /// a compacted leader. `#[serde(default)]` so a gateway can still deserialize a + /// pre-fix peer during a mixed-version window. + #[serde(default)] + reseed_required: bool, + /// Whether THIS GROUP is mid-reseed (install/seed-join not yet converged). + #[serde(default)] + reseeding: bool, } /// Local replication / leadership status for THIS region. diff --git a/tidal-server/tests/cluster_reseed.rs b/tidal-server/tests/cluster_reseed.rs index 362ca95..288835c 100644 --- a/tidal-server/tests/cluster_reseed.rs +++ b/tidal-server/tests/cluster_reseed.rs @@ -747,11 +747,29 @@ fn mp_multi_group_node_converges_after_reseeding_several_groups() { match cluster.local_status(AP_SOUTH) { Some(s) => { unreachable_polls = 0; - if s["reseed_required"].as_bool() == Some(false) + // EVERY hosted group must be settled, not just the flat fields. + // The flat `reseed_required` / `reseeding` describe only the group + // `replica_for(None)` resolves (the lowest hosted id), so on a + // 3-group node they can read "clean" while another group is still + // marked. That under-reporting is exactly what made the production + // incident look converged, and it fooled the first version of this + // predicate too — the node announced "settled after 0 restarts" and + // was missing an item. The per-group rows now carry the state. + let rows = s["shards"].as_array().cloned().unwrap_or_default(); + let all_groups_clean = !rows.is_empty() + && rows.iter().all(|r| { + r["reseed_required"].as_bool() == Some(false) + && r["reseeding"].as_bool() == Some(false) + }); + if all_groups_clean + && s["reseed_required"].as_bool() == Some(false) && s["reseeding"].as_bool() == Some(false) && s["quarantined"].as_bool() == Some(false) { - println!("[multi] node 2 settled after {restarts} orchestrator restart(s)"); + println!( + "[multi] node 2 settled after {restarts} orchestrator restart(s); \ + per-group rows: {rows:?}" + ); break; } }