diff --git a/.kiro/specs/controller-status/design.md b/.kiro/specs/controller-status/design.md index 7f763251..4612de86 100644 --- a/.kiro/specs/controller-status/design.md +++ b/.kiro/specs/controller-status/design.md @@ -320,15 +320,16 @@ No new files (the capnp field is generated; all logic slots into existing TUs). User enhancement request: `ctrlstatus` should additionally report the term in which the current primary was elected and the wall-clock time it was elected, so operators can tell how long the current primary has served and correlate its election with the logs — with the same off-primary (standby) fidelity Req 10 already established for committed position, not a bare "unknown" off-primary. Scope of this delta = Req 12 only. Reqs 1–11 are implemented and unchanged. ### Boundary Commitments (delta) -- **This spec owns (added):** a new `ha_state_t.last_known_elected_at_ms` field (`_Atomic uint64_t`, declared alongside `last_known_committed_lsn`/`last_known_committed_config_tag` in `ha.h`), self-stamped by BOTH promotion roads at the exact point each commits to `CTRL_PRIMARY`; a new `HeartbeatMsg.electedAtMs` wire field the primary echoes on every outgoing heartbeat; the receiving standby's unconditional store of that value into its own `last_known_elected_at_ms`; a new `CtrlStatus.electedAtMs` reply field; `ctrlstatus_fill` populating it (one unconditional read, no role-branch — see Locked Decision 2); `cmd_ctrlstatus`'s two new printed lines (`electedTerm`, `electedAt`), gated on the same primaryName-known/unknown derivation Req 2 already computes. +- **This spec owns (added):** a new `ha_state_t.last_known_elected_at_ms` field (`_Atomic uint64_t`, declared alongside `last_known_committed_lsn`/`last_known_committed_config_tag` in `ha.h`), self-stamped by BOTH promotion roads at the exact point each commits to `CTRL_PRIMARY`; a new `HeartbeatMsg.electedAtMs` wire field the primary echoes on every outgoing heartbeat; the receiving standby's unconditional store of that value into its own `last_known_elected_at_ms`; a new `CtrlStatus.electedAtMs` reply field; `ctrlstatus_fill` populating it (one unconditional read, no role-branch — see Locked Decision 2); `cmd_ctrlstatus`'s two new printed lines (`electedTerm`, `electedAt`), gated on `elected_at_ms == 0` (corrected 2026-09-16 from an original primaryName-based gate — see Locked Decision 3). - **Out of Boundary (firm):** `electedTerm` gets **no new field** — it reuses the existing `CtrlStatus.term` (Req 6.1) with CLI-side "unknown" gating (Locked Decision 1); this spec does not change how elections are conducted, how `ha->term` advances, or how/when `ha->role` transitions — every stamp site added here is purely observational at an already-decided promotion instant (Req 12.5). - **Allowed Dependencies (added):** `ha->term` and the existing primaryName derivation (Req 2/3, unchanged); `clock_gettime(CLOCK_REALTIME)` (existing precedent, Req 11); `ctrlstatus_format_time_ms` (`src/include/xce/ctrlstatus_fmt.h`) reused verbatim for rendering — no new formatting code. - **Revalidation Triggers (added):** any change to where `ha->role` finally becomes `CTRL_PRIMARY` on either promotion road (`ha_do_promote` or `raft_cb_become_leader`) → re-check the `last_known_elected_at_ms` stamp site is still past every refusal/eligibility gate; any change to the `HeartbeatMsg`/`heartbeat_msg_t` shape → re-check the `electedAtMs` ordinal and decode; any change to the primaryName-unknown derivation (Req 2) → re-check the electedTerm/electedAtMs "unknown" gating that reuses it. ### Locked design decisions -1. **`electedTerm` reuses `ha->term` (Req 6.1's existing wire field) — no new state.** This codebase's Raft term already propagates correctly to every node via the existing heartbeat/vote term-sync path; there is no "stale/zero" class of bug here the way Req 10 had for `committedLsn` (which read an unrelated *primary-only* marker, `persist.committed_lsn`, that stays 0 off-primary by construction). `cmd_ctrlstatus` derives the displayed `electedTerm` as `primaryName known ? term : unknown` — purely a print-layer decision, no handler change for this half of the requirement. **Accepted trade-off:** in the narrow window where a standby has just voted for a new candidate (bumping its own `ha->term`) before its `voted_for_name`-derived primaryName reflects that same candidate, the displayed `electedTerm` could transiently be a half-step out of step with the displayed primary — the same class of transient standby-hint staleness already accepted for the original `primaryName`/`term` fields (see Architecture's "any-node hint then authoritative-from-primary" flow) and for Req 10's own documented staleness caveat. Not a regression, not new risk surface. +1. **`electedTerm` reuses `ha->term` (Req 6.1's existing wire field) — no new state.** This codebase's Raft term already propagates correctly to every node via the existing heartbeat/vote term-sync path; there is no "stale/zero" class of bug here the way Req 10 had for `committedLsn` (which read an unrelated *primary-only* marker, `persist.committed_lsn`, that stays 0 off-primary by construction). `cmd_ctrlstatus` derives the displayed `electedTerm` as `elected_at_ms != 0 ? term : unknown` (corrected 2026-09-16 — see Locked Decision 3; originally `primaryName known ? term : unknown`) — purely a print-layer decision, no handler change for this half of the requirement. **Accepted trade-off:** in the narrow window where a standby has just voted for a new candidate (bumping its own `ha->term`) before its `voted_for_name`-derived primaryName reflects that same candidate, the displayed `electedTerm` could transiently be a half-step out of step with the displayed primary — the same class of transient standby-hint staleness already accepted for the original `primaryName`/`term` fields (see Architecture's "any-node hint then authoritative-from-primary" flow) and for Req 10's own documented staleness caveat. Not a regression, not new risk surface. 2. **`electedAtMs` is genuinely new state, written unconditionally (no monotonic/term compare on write).** Both promotion roads write `last_known_elected_at_ms` exactly once, at the instant each road COMMITS to `CTRL_PRIMARY` — i.e., past every refusal/eligibility gate, never at function entry (see Research Items below for the exact confirm-at-implementation lines). The heartbeat-receive site writes it only when already processing a heartbeat from the node's currently-recognized leader — the same precondition `ha_observe_heartbeat_committed`'s existing call site already relies on for `committed_lsn`/`committed_config_tag`. Because every write site is already gated by an existing freshness precondition, no second "which term does my stored value correspond to" comparison is needed. **Simplification vs. Req 10's pattern:** since there is no pre-existing primary-only marker analogous to `persist.committed_lsn` for election time, `ctrlstatus_fill` needs **no role-branch** for this field — one unconditional read of `last_known_elected_at_ms` correctly serves both the primary (self-stamped) and a standby (heartbeat-learned) case. -3. **Unknown gating happens at output time, not by proactively zeroing state.** Both `electedTerm` and `electedAtMs` are gated in `ctrlstatus_fill`/`cmd_ctrlstatus` on the SAME primaryName-known/unknown derivation Req 2.2/2.3 already computes. A stale `last_known_elected_at_ms` left over from a since-lost primary is simply never surfaced once primaryName itself reports "unknown" — mirrors how the original design already treats a stale `voted_for_name`/`primary_host`. +3. **Unknown gating happens at output time, not by proactively zeroing state.** Both `electedTerm` and `electedAtMs` are gated in `cmd_ctrlstatus` on `elected_at_ms == 0` (see live-E2E correction below), not on the primaryName-known/unknown derivation Req 2.2/2.3 computes. A stale `last_known_elected_at_ms` left over from a since-lost primary is never surfaced because a fresh promotion or heartbeat-learn always overwrites it before it could be observed as stale — the reader never sees an in-between state, so no separate staleness check is needed beyond the zero-check itself. + > **Live-E2E correction (2026-09-16, task 7.6 finding):** this decision originally gated on the SAME primaryName-known/unknown derivation Req 2.2/2.3 already computes, reasoning it would mirror how the original design treats a stale `voted_for_name`/`primary_host`. A live 3-node cluster run proved this wrong: `primaryName` is *unconditionally* empty on every non-primary responder (Req 2's own `ctrlstatus_fill` only self-reports the primary's name when the responder itself is primary — `ha->primary_host` is vestigial, per the `ade0081` design note above), so Req 2.3's "standby knows the primary" branch is dead code in the current build via the plain `ctrlstatus` command. Gating on `primaryName` therefore made `electedTerm`/`electedAt` print "unknown" on *every* standby, permanently — even though `elected_at_ms` was correctly, non-zero, and convergently populated the whole time (independently proven live: `term`/`committedLsn`/`appliedLsn`, sourced the identical unconditional way, DID correctly display and converge on the same standby query). Gating on `status->elected_at_ms == 0` instead is both necessary and sufficient: it stays 0 until a real promotion stamps it (either road) or a heartbeat carries a learned value, and — unlike `term`, which can be legitimately nonzero from mere vote/candidacy activity before any primary is actually elected — `elected_at_ms` only ever becomes nonzero via an actual election. This is a `cmd_ctrlstatus`-only correction; `ctrlstatus_fill`'s own unconditional, no-role-branch read (Locked Decision 2) is unaffected and was already correct. 4. **Wire type: `electedAtMs @14: UInt64` epoch-milliseconds** — mirrors Req 11's `responderTimeMs` numeric-not-Text precedent exactly (machine-comparable; the client formats it via the same existing `ctrlstatus_format_time_ms`). ### Research items carried to implementation @@ -369,9 +370,9 @@ No new files (capnp fields are generated; all logic slots into existing TUs) — ### Requirements Traceability (delta) | Requirement | Summary | Components | Interfaces | |-------------|---------|------------|------------| -| 12.1 | Report the primary's election term + time alongside its identity | `ctrlstatus_fill` (`electedAtMs` read), `cmd_ctrlstatus` (`electedTerm` derived from `term`+`primaryName`) | `CtrlStatus.term` (reused), `CtrlStatus.electedAtMs` (new) | +| 12.1 | Report the primary's election term + time alongside its identity | `ctrlstatus_fill` (`electedAtMs` read), `cmd_ctrlstatus` (`electedTerm` derived from `term`, gated on `elected_at_ms`) | `CtrlStatus.term` (reused), `CtrlStatus.electedAtMs` (new) | | 12.2 | Standby reports the LEARNED election term/time, not stale/unset | heartbeat-carried `electedAtMs`; existing term-sync for `electedTerm` | `HeartbeatMsg.electedAtMs`, unconditional store into `last_known_elected_at_ms` | -| 12.3 | Unknown when no primary is known | `ctrlstatus_fill`/`cmd_ctrlstatus` gating on primaryName | shares Req 2.2/2.3's existing "unknown" derivation | +| 12.3 | Unknown when no primary is known | `cmd_ctrlstatus` gating on `elected_at_ms == 0` (corrected 2026-09-16, see Locked Decision 3) | independent of Req 2.2/2.3's `primaryName` derivation, which is unconditionally empty on a standby in the current build | | 12.4 | Election time on the same clock/format as logs | `ctrlstatus_fill` (`CLOCK_REALTIME` stamp), `cmd_ctrlstatus` render | reuses `ctrlstatus_format_time_ms` (Req 11.2 precedent) | | 12.5 | Read-only; no election/leadership behavior change | stamp sites are observational only; no gating logic altered | n/a | | 12.6 | Test client prints both fields | `cmd_ctrlstatus` | two new printed lines | @@ -381,10 +382,10 @@ No new files (capnp fields are generated; all logic slots into existing TUs) — - **Req 12.5 (read-only invariant) preserved:** the handler itself performs only atomic loads, exactly as every other field; all writes to `last_known_elected_at_ms` happen upstream, at promotion or heartbeat-processing time, never inside `ctrlstatus_fill`. - **`cmd_ctrlstatus` (Req 12.1, 12.3, 12.6):** print ``` - electedTerm: (or "unknown" when primaryName is unknown) - electedAt: (or "unknown") + electedTerm: (or "unknown" when elected_at_ms == 0) + electedAt: (or "unknown" when elected_at_ms == 0) ``` - placed after the existing `Primary:` line, grouping the primary's identity with its election term/time. + placed after the existing `Primary:` line, grouping the primary's identity with its election term/time. Gated on `elected_at_ms == 0`, not on `primaryName` (corrected 2026-09-16 — see Locked Decision 3). ### Testing Strategy (delta) **Unit (`test_ctrlstatus.c`):** diff --git a/src/harness/test.c b/src/harness/test.c index f9108539..9abbdcb7 100644 --- a/src/harness/test.c +++ b/src/harness/test.c @@ -3098,15 +3098,23 @@ int cmd_ctrlstatus(int argc, char* argv[]) NULLSTR(status->primaryName) ? "unknown" : status->primaryName); /* Election term + time of the CURRENT primary (Req 12.1/12.3/12.4/12.6): - * gated on the SAME primaryName-known/unknown derivation the Primary: - * line above already computes (Req 2.2/2.3) -- a stale/learned - * elected_at_ms left over from a since-lost primary must never surface - * once primaryName itself reports "unknown" (design.md Locked Decision - * 3). electedTerm is DERIVED client-side from the existing `term` field - * (no new wire field, design.md Locked Decision 1); electedAt renders - * `elected_at_ms` on the same clock/format as responderTime below, - * reusing ctrlstatus_format_time_ms (design.md Locked Decision 4). */ - if (NULLSTR(status->primaryName)) { + * gated on elected_at_ms itself, NOT on primaryName (design.md Locked + * Decision 3, corrected 2026-09-16 / task 7.6 live E2E finding). + * primaryName is unconditionally empty on every non-primary responder + * (ctrlstatus_fill only self-reports the primary's own name; a + * pre-existing Req 2 behavior, unrelated to this delta), so gating on it + * made electedTerm/electedAt read "unknown" on every standby forever, + * even once a real election had converged. elected_at_ms stays exactly 0 + * until a real promotion stamps it (ha_do_promote, task 7.2) or a + * heartbeat carries a learned value from an actual primary (task 7.3) -- + * unlike `term`, which can legitimately bump on mere vote/candidacy + * activity (raft term adoption) before any primary is actually elected, + * so `term == 0` would be the wrong gate. electedTerm is DERIVED + * client-side from the existing `term` field (no new wire field, + * design.md Locked Decision 1); electedAt renders `elected_at_ms` on the + * same clock/format as responderTime below, reusing + * ctrlstatus_format_time_ms (design.md Locked Decision 4). */ + if (status->elected_at_ms == 0) { printf("electedTerm: unknown\n"); printf("electedAt: unknown\n"); } else {