Rongsong Shen
7556856e5c
Adds requirements, design, research, and tasks for reporting the term and wall-clock time the current primary was elected, including standby-side learned fidelity (mirroring the Req 10 pattern), following the existing controller-status spec's delta convention (Req 10/11). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RLSNJhNoULvVcUhXEFafQC
230 lines
26 KiB
Markdown
230 lines
26 KiB
Markdown
# Gap Analysis — controller-status (ctrlstatus)
|
||
|
||
Analyzed the requirements against the existing codebase. The existing `clusterconf` opcode is a clean **opcode-only read** to model the plumbing on, and **all the status data already exists in-process**. The single requirements-critical new piece is the HA **dispatch-gate allow-list** (so the op answers on a standby/electing/no-quorum node).
|
||
|
||
## Summary
|
||
|
||
- **Recommended approach: a NEW dedicated lean opcode** (`ctrlstatus`), NOT an extension of `clusterconf`. This decisively resolves the spec's open "extend clusterconf vs new op" question: `clusterconf` is **primary-only when serving** (absent from every standby allow-list) and returns the heavy resource/queue/worker dump — extending it would still be primary-only (fails Req 7 any-node, Req 2.3 standby-reports-primary) and would carry the inventory (fails Req 1.4 focused view).
|
||
- **Config CRC: already exists, reuse.** `config_version_tag` = first 8 bytes of SHA-256 over the committed config blob (`myconf.h:31`, `config_repl.c:120-123`); full digest cached in `config_sha256[32]` (`myconf.h:32`) and round-trip-verified on load (`config_repl.c:652`). Requirement 5's version + checksum are both directly readable — no new computation.
|
||
- **All other status data exists**: role/term/applied_lsn/committed_config_tag are `_Atomic` (lock-free reads); `primary_host`/`primary_port` need `ha->lock`; peers via `c->myconf->peers_cfg.candidates[]`; fenced via `in_catch_up` + `persist_recovery_unrecoverable()`.
|
||
- **The one real gap**: `ha_dispatch_gate` (`ha.c:2059`) is an allow-list that defaults to `-XCE_ERR_STANDBY` (standby), `-XCE_ERR_NOT_READY` (init/electing), and gates the primary no-quorum branch. `OPCode_ctrlstatus` must be explicitly allow-listed in STANDBY + INIT + ELECTING + PRIMARY-no-quorum so a status read always answers — including when fenced/no-quorum, which is exactly when an operator queries it.
|
||
- **Generated codegen helps**: capnp structs are generated at build (`src/lib/meson.build`), so adding the schema struct + rebuild auto-produces `xce_ctrlstatus_t`, `RequestReply_data_ctrlstatus`, and `free_CtrlStatus`.
|
||
|
||
## Requirement → Asset Map (gaps tagged)
|
||
|
||
| Req | Need | Existing asset | Gap |
|
||
|-----|------|----------------|-----|
|
||
| 1 single-call read | opcode-only read + lean reply | `clusterconf` opcode-only pattern (`request.c:1734` `nomsg`); no-alloc reply model `ha_nop_reply_state_report` (`ha.c:1784`) | **Missing**: new `CtrlStatus` struct + `handle_ctrlstatus` (clone the no-alloc model, not the heap-xlist clusterconf one) |
|
||
| 2 primary identity (`unknown`) | current primary name | `ha->primary_host`/`primary_port` (`ha.h:149`) | **Constraint**: read under `ha->lock`; map empty→`"unknown"` |
|
||
| 3 answered-by node + role | own name + role | `c->myconf->name` (`myconf.h:29`); `ha->role` atomic (`ha.h:146`) | Reuse (lock-free) |
|
||
| 4 peers + count | configured controllers | `c->myconf->peers_cfg.candidates[]` + `candidates_count` (`cluster.h:32-38`) | Reuse, **no gap** |
|
||
| 5 config version + CRC | version + content checksum | `config_version_tag` (8-byte SHA-256 prefix) + `config_sha256[32]` (`myconf.h:31-32`); `committed_config_tag` (`ha.h:173`) | Reuse, **no gap** — expose, don't compute |
|
||
| 6 term/commit/apply/fenced/name | HA health | `ha->term`, `ha->applied_lsn` atomics; `persist.committed_lsn` atomic (no accessor — read directly); `in_catch_up` + `persist_recovery_unrecoverable()` (`persist.h:432`); cluster name | Reuse |
|
||
| 7 failover-aware any-node | retryable read on any node | `XCECLI_FAILOVER_CALL(is_write=false)` (`client.c:315`) | **Missing (CRITICAL)**: dispatch-gate allow-list entries for STANDBY/INIT/ELECTING/PRIMARY-no-quorum (`ha.c:2059`) |
|
||
| 8 test-client command | `ctrlstatus` print command | `cmd_*` table + display patterns in `test.c` | **Missing**: `cmd_ctrlstatus` (small) |
|
||
| 9 wait-primary uses it | switch the probe | `cmd_wait_primary` probe at `test.c:1161` | **Missing**: one-line probe swap + the new client fn |
|
||
|
||
## Implementation Approach Options
|
||
|
||
### Option A — Extend `clusterconf` with HA-status fields
|
||
Add primary/role/term/config-version fields to `ClusterConf` + `handle_clusterconf`.
|
||
- ✅ Fewest new symbols.
|
||
- ❌ **Fails Req 7 / 2.3**: `clusterconf` is not standby-allow-listed → primary-only when serving; a status op must answer on any node. ❌ **Fails Req 1.4**: still returns the resource/queue/worker inventory. ❌ Bloats a heap-xlist handler.
|
||
- **Rejected.**
|
||
|
||
### Option B — New dedicated lean opcode `ctrlstatus` (RECOMMENDED)
|
||
New `OPCode_ctrlstatus @40`, lean `struct CtrlStatus`, `handle_ctrlstatus` on the no-alloc `state_report` model, **dispatch-gate allow-listed on all roles**, libxce `xce_ctrlstatus`/`xcecli_ctrlstatus` (is_write=false), `cmd_ctrlstatus`, wait-primary probe swap.
|
||
- ✅ Lean focused view (Req 1.4), any-node answerable (Req 7/2.3), no heap alloc, clean separation from `clusterconf`.
|
||
- ✅ Clones the proven opcode-only read plumbing; capnp regen auto-generates the C types.
|
||
- ❌ Touches ~9 files across schema/controller/libxce/client + the HA dispatch gate.
|
||
- **Recommended.**
|
||
|
||
### Option C — Hybrid (reuse plumbing pattern, new op)
|
||
This is Option B in practice: reuse the `clusterconf` *plumbing pattern* (request/reply/failover/free) and the `state_report` *handler pattern*, but a brand-new opcode + struct + dispatch-gate work. No separate value beyond B.
|
||
|
||
## Effort & Risk
|
||
|
||
- **Effort: M (3–7 days).** All data exists and the clone pattern is proven (much is mechanical: schema + regen + req/api/cli/free + handler + command). The non-mechanical parts are the **dispatch-gate allow-listing** (HA-critical path, needs care + tests) and per-layer tests (capnp round-trip, handler-on-standby, client failover, wait-primary swap).
|
||
- **Risk: Low–Medium.** Low: no new infrastructure, content CRC already exists, read-only/no HA-behavior change. Medium: the dispatch gate is on the request-handling critical path — a wrong allow-list entry could let a standby serve something it shouldn't or fail to answer; must be scoped strictly to this read-only status op and covered by tests proving it answers on standby/electing/no-quorum **without** widening any other opcode's gating.
|
||
|
||
## Research Items to Carry into Design
|
||
|
||
1. **Reply field set & types**: expose `committed_config_tag` (8-byte version) as the version AND the full `config_sha256[32]` (hex) as the checksum? The tag is already a content-hash prefix — design should decide whether the full 32-byte digest is also returned for stronger same-version divergence detection (Req 5.3).
|
||
2. **Dispatch-gate semantics (key decision)**: a status read should answer **regardless of quorum/serving/fence** (that's when you most want it). Confirm `ctrlstatus` bypasses the quorum/serving gate in all role branches, and define precisely what each role returns (e.g. an ELECTING node returns role=electing, primary=unknown).
|
||
3. **Handler location**: `reply.c` (next to `handle_clusterconf`) vs `ha.c` (next to `ha_nop_reply_state_report`, the no-alloc model it should follow). Lean toward the `state_report` model.
|
||
4. **`"unknown"` representation** in the capnp reply (empty Text vs explicit sentinel) and how the client/test command renders it.
|
||
5. **wait-primary "serving primary present" definition** from the status reply: primary identity != unknown — and/or the responding node reports role=primary? Define the exact success predicate so the probe is unambiguous, preserving Req 9.4 (timeout/exit semantics unchanged).
|
||
6. **`committed_lsn` accessor**: none exists (`persist.committed_lsn` is read directly elsewhere). Design may add a thin `persist_committed_lsn()` for the handler, or read the atomic directly — minor.
|
||
|
||
## Files in the clone map (for design/tasks)
|
||
|
||
- `src/schema/controller.capnp` — `OPCode ctrlstatus @40`; `struct CtrlStatus $C.mapname("xce_ctrlstatus_t")`; reply member `ctrlstatus @17`. (regenerates `controller.capnp.h/.c`)
|
||
- `src/controller/cluster.c:335` — `case OPCode_ctrlstatus → handle_ctrlstatus`.
|
||
- `handle_ctrlstatus` — new (model: `ha_nop_reply_state_report`, `ha.c:1784`; reply send via `xce_reply`, `init_reply_by_req`).
|
||
- `src/controller/ha.c:2059` — **add `OPCode_ctrlstatus` to STANDBY + INIT + ELECTING + PRIMARY-no-quorum allow-lists (the critical gap).**
|
||
- `src/lib/request.c` — `xce_ctrlstatus_req` (opcode-only, clone `request.c:1734`); `src/include/request.h` decl.
|
||
- `src/lib/xceapi.c:400` — `xce_ctrlstatus` (`MOVE_PTR` reply union).
|
||
- `src/lib/client.c:315` — `xcecli_ctrlstatus` (`XCECLI_FAILOVER_CALL`, is_write=false); `src/include/client.h:55,90` decl + free macro.
|
||
- `src/controller/testsuite/test.c` — `cmd_ctrlstatus` command + registration; swap `cmd_wait_primary` probe at `test.c:1161`.
|
||
|
||
---
|
||
|
||
## Design Synthesis (decisions resolved for design.md)
|
||
|
||
The 6 carried research items are resolved as:
|
||
1. **Reply field set**: expose BOTH `configVersion` (the 8-byte `committed_config_tag`) AND `configChecksum` (the full 32-byte `config_sha256`) — version for quick compare, full digest for strong same-version divergence detection (Req 5.3).
|
||
2. **Dispatch-gate semantics**: `ctrlstatus` is allow-listed in ALL role branches (PRIMARY incl. no-quorum, STANDBY, ELECTING, INIT) and bypasses the quorum/serving/fence gate — a status read must answer precisely when the cluster is degraded. Each role returns its local view (PRIMARY→self; STANDBY→its known primary or unknown; ELECTING/INIT→unknown).
|
||
3. **Handler placement & shape**: `handle_ctrlstatus` in `reply.c` next to `handle_clusterconf`, following the no-alloc `ha_nop_reply_state_report` model; only the small bounded peer list allocates. `primary_host`/`port` read under `ha->lock`; everything else atomic/immutable.
|
||
4. **"unknown" representation**: empty `Text primaryName` on the wire; the client renders it as `"unknown"` (Req 1.3/2.2/2.3).
|
||
5. **wait-primary predicate**: success = reply reports a non-"unknown" primary (NOT "responder==PRIMARY", which could starve if a standby answers first under failover rotation). Accepted caveat: a brief standby-stale-primary window; still strictly more informative than the old joblist probe. Preserves all existing wait-primary timeout/exit semantics (Req 9.4).
|
||
6. **`committed_lsn` access**: read the persist atomic directly (no accessor exists); optionally add a thin `persist_committed_lsn()` — minor, implementer's choice.
|
||
|
||
**Build-vs-adopt**: ADOPT the `clusterconf` opcode-only plumbing pattern + the `state_report` no-alloc handler model; BUILD only the new schema structs, handler body, dispatch-gate entries, and the thin client/test wiring. No new infrastructure; config checksum reused (not built).
|
||
|
||
---
|
||
|
||
## Design-review updates (2026-06-19, /kiro-validate-design)
|
||
|
||
Two review issues resolved with the user; design.md updated:
|
||
|
||
1. **wait-primary predicate → AUTHORITATIVE from the primary (user decision).** A standby's reply is a non-authoritative HINT (it may name a just-failed primary from stale `primary_host`). The op still answers on any node (Req 2.3/7), but for an authoritative answer the client follows the redirect — maps the reply's `primaryName` to an address via the reply's `peers` list, else rotates the configured master list — until a reply with `responderRole == PRIMARY`. New `xcecli_ctrlstatus_primary` provides this; `wait-primary` uses it (success = the primary itself answered), eliminating the stale-standby false-success (the same false-positive class removed from the durability soak). The plain `xcecli_ctrlstatus` (any-node hint view) backs the operator `ctrlstatus` command.
|
||
2. **Config fields → loaded pair + committed tag (user decision).** Report `configVersion`+`configChecksum` from the LOADED config (self-consistent: version = first 8 bytes of the checksum) AND `committedConfigVersion` (`committed_config_tag`) separately — "what I run" vs "what is committed". Avoids the earlier mismatched committed-tag + loaded-checksum pairing.
|
||
|
||
Also fixed: mermaid render issues — removed `/`, parentheses, and quotes from diagram labels/messages (the LR architecture graph + the new redirect sequence diagram).
|
||
|
||
---
|
||
|
||
## Gap Analysis — Delta for Requirements 10 & 11 (2026-06-20)
|
||
|
||
Reopened after a live HA-deploy E2E finding: a standby answering `ctrlstatus` showed
|
||
`committedLsn: 0` / `committedConfigVersion: 0` while `appliedLsn: 38` (an apparent
|
||
applied-greater-than-committed inversion), and there was no way to correlate a captured
|
||
status against the per-node logs. Scope of this delta = Req 10 (off-primary committed
|
||
fidelity) + Req 11 (responder snapshot timestamp). Reqs 1–9 are implemented and unchanged.
|
||
|
||
### Requirement-to-Asset Map
|
||
|
||
| Need (Req) | Existing asset | Gap |
|
||
|---|---|---|
|
||
| 10.1 standby reports *learned* committed LSN | `ctrlstatus_fill` reads `c->persist.committed_lsn` (`ctrlstatus.c:130`) — a **primary-only** marker, 0 on a node that never led | **Constraint**: wrong source field; the learned value `ha->last_known_committed_lsn` (`ha.h:170`) exists but is unused here |
|
||
| 10.1 standby reports *learned* committed config tag | `ctrlstatus_fill` reads `ha->committed_config_tag` (`ctrlstatus.c:109`) — also primary-only | **Constraint**: learned `ha->last_known_committed_config_tag` (`ha.h:172`) exists but is unused here |
|
||
| 10.1 freshness of the learned value | `last_known_committed_lsn` written only at `cluster_apply_ctrlhello_reply` (`cluster.c:291`, `ha_convergence.c:375`) and an election safe-commit path (`ha.c:925`) | **Constraint (key)**: refreshed only at **CtrlHello join / RDHUP-failover handshake**, NOT per heartbeat. The `OP_nop` heartbeat (`cluster.c:300`) carries no committed payload; the richer `HeartbeatMsg{committedLsn@2, committedConfigTag@3}` (`controller.capnp:761`) is **defined but never consumed** on receive → the learned value can LAG between joins |
|
||
| 10.2 genuinely-unknown → 0/unknown | existing `NULLSTR`/0 conventions (Req 1.3) | none — `last_known_committed_lsn` is 0 before first learn, which is the correct "unknown" |
|
||
| 11.1 responder wall-clock at sample time | none on `CtrlStatus` (schema `controller.capnp:496`, fields @0–@12; next free ordinal **@13**) | **Missing**: new field + a `CLOCK_REALTIME` stamp in `ctrlstatus_fill`. Note `ha_on_heartbeat_received` uses `CLOCK_MONOTONIC` — **not** reusable for log correlation |
|
||
| 11.2 same clock/format as logs | logger header `src/include/xcelog.h` produces `[YYYY-MM-DD HH:MM:SS,mmm]` | **Research Needed**: confirm the exact formatter/helper in `xcelog.h` to reuse (or match) for byte-comparable output |
|
||
| 11.3 test client prints timestamp | `cmd_ctrlstatus` in `src/controller/testsuite/test.c` (C `xce_test` client; print block ~line 1297) — the "client" is xce_test, not Go | **Missing**: one print line; libxce decode is codegen from schema (auto after regen) |
|
||
|
||
### Implementation Approach Options
|
||
|
||
**Req 10 — Option A (Extend `ctrlstatus_fill`) — RECOMMENDED.** When `responderRole != CTRL_PRIMARY`,
|
||
source `committedLsn` from `ha->last_known_committed_lsn` and `committedConfigVersion` from
|
||
`ha->last_known_committed_config_tag`; keep the primary's own `persist.committed_lsn` /
|
||
`committed_config_tag` when it IS primary (those are the authoritative live values). Read-only,
|
||
satisfies Req 10.4, mirrors the existing `last_known_*` usage in the RDHUP/freshness paths
|
||
(`ha.c:2570`, `ha.c:4126`). A simpler variant — report `max(persist.committed_lsn,
|
||
last_known_committed_lsn)` uniformly (no role branch) — also kills the 0-inversion; design picks.
|
||
*Trade-off*: ✅ tiny, in-boundary, fixes the inversion. ❌ value can lag between joins (see below).
|
||
|
||
**Req 10 — out-of-boundary (do NOT pull into this spec).** Making the off-primary committed
|
||
value *live-fresh* requires consuming `HeartbeatMsg.committedLsn` (or extending `OP_nop`) and
|
||
storing it into `last_known_committed_lsn` on **every** heartbeat — a heartbeat/replication
|
||
protocol change. That violates Req 10.4 ("shall not change how committed positions are produced,
|
||
replicated, or persisted") and belongs to **ha-heartbeat-liveness** / **ha-repl-hardening**.
|
||
controller-status should report the honest best-known value and the design must state the lag caveat.
|
||
|
||
**Req 11 — Option A (Extend) — RECOMMENDED.** Add `responderTime @13: UInt64` (epoch millis) to
|
||
`CtrlStatus`; stamp it in `ctrlstatus_fill` from `CLOCK_REALTIME`; render in `cmd_ctrlstatus`
|
||
formatted via the `xcelog.h` convention so it lines up with `ctrl-N.log`. *Design decision*:
|
||
numeric epoch-millis (client formats — locale-safe, clean) vs pre-formatted `Text` (zero client
|
||
work, mirrors the `configChecksum`-as-Text precedent). Recommend numeric epoch-millis.
|
||
*Trade-off*: ✅ one field + one stamp + one print line. ❌ adds a schema ordinal (codegen regen).
|
||
|
||
### Effort & Risk
|
||
|
||
- **Req 10**: **S** (≤1 day) — one function's field sourcing + TDD (standby-known-primary asserts learned value; pre-learn asserts 0). **Risk Low** — read-only, no HA-state change.
|
||
- **Req 11**: **S** (1–2 days) — schema field + `ctrlstatus_fill` stamp + `cmd_ctrlstatus` print + codegen regen + round-trip decode test. **Risk Low**.
|
||
- **Combined**: **S**. The one **Medium-risk temptation to refuse**: expanding into heartbeat committed-propagation — keep it out of this spec.
|
||
|
||
### Research Items to Carry to Design
|
||
|
||
1. Confirm the exact wall-clock formatter/helper in `src/include/xcelog.h`; reuse it (or document the chosen format) so Req 11's value is byte-comparable to log lines.
|
||
2. Decide Req 11 wire type: `UInt64` epoch-millis (recommended) vs `Text`.
|
||
3. Decide Req 10 sourcing: role-branch (`persist.*` on primary, `last_known_*` off-primary) vs uniform `max()`; keep LSN and config-tag symmetric.
|
||
4. State the **staleness caveat** in design (learned value refreshes at join, not per heartbeat) and explicitly route live-fresh-committed to **ha-heartbeat-liveness / ha-repl-hardening** as out-of-boundary.
|
||
|
||
---
|
||
|
||
## Gap Analysis — Delta for Requirement 12 (2026-09-15)
|
||
|
||
Reopened by a user enhancement request: `ctrlstatus` should report the term in which the
|
||
current primary was elected and the wall-clock time it was elected, with standby-side
|
||
fidelity (learned, not "unknown") explicitly requested — mirroring Req 10's off-primary
|
||
fidelity pattern rather than the simpler primary-only alternative. Scope of this delta =
|
||
Req 12 only. Reqs 1–11 are implemented and unchanged.
|
||
|
||
### Requirement-to-Asset Map
|
||
|
||
| Need (Req) | Existing asset | Gap |
|
||
|---|---|---|
|
||
| 12.1 term of the current primary's election | `ha->term` (`ha.h`, already reported as `CtrlStatus.term`, Req 6.1) | **No gap** — this codebase's Raft term already propagates correctly to every node via the existing heartbeat/vote term-sync path (`heartbeat_msg_t.term`, `src/include/typemap.h:613`). Reuse, don't add a field. |
|
||
| 12.1 wall-clock time of the current primary's election | none — `ha_state_t` has no election timestamp of any kind (unlike `last_known_committed_lsn`/`last_known_committed_config_tag`, which already exist for committed position) | **Missing**: a new `ha_state_t` field, a new promotion-time stamp (both roads), a new heartbeat-carried field, and a new `CtrlStatus` reply field |
|
||
| 12.2 off-primary (standby) fidelity for election time | `ha_observe_heartbeat_committed` (`ha.c:9085`) is the existing precedent for learning a primary-sourced value via heartbeat, monotonic-on-LSN | **Missing**: `heartbeat_msg_t` (`typemap.h:611-616`) carries `term`/`committed_lsn`/`committed_config_tag` but no election timestamp — must add `electedAtMs` to both the C struct and its capnp schema (`HeartbeatMsg`, next free ordinal after `committedConfigTag@3`) |
|
||
| 12.2 primary's own promotion-time self-observation | `raft_cb_become_leader` (`ha.c:1615`) already self-observes `last_known_committed_lsn` at its promotion instant (`ha.c:1665`, inside `if (promo_ectx)`); `ha_do_promote` (`ha.c:2927`) is the non-raft road's promotion function | **Gap (needs implementation-time confirmation, not just reuse)**: the committed-LSN self-observe point in `raft_cb_become_leader` sits BEFORE the function's later `ha_promotion_gate_decision` eligibility gate, which can still refuse promotion — the NEW election-time stamp must sit AFTER that gate, at the point the function is irrevocably committed to `CTRL_PRIMARY`, a different line than the existing precedent. This delta's own codegraph exploration did not reach that later line (function body was truncated by explore-tool budget); flagged as a Research Item in design.md rather than guessed. |
|
||
| 12.3 unknown when no primary known | existing primaryName-unknown derivation (Req 2.2/2.3) | **No gap** — gate the new fields' *output* on this existing derivation; no new "unknown" logic needed |
|
||
| 12.4 same clock/format as Req 11 | `ctrlstatus_format_time_ms` (`src/include/xce/ctrlstatus_fmt.h:33`) already extracted as a shared, reusable formatter (confirmed via the discovery-phase CodeGraph hint — it is already used by both `ctrlstatus.c` and `joblist_display.c`) | **No gap** — reuse verbatim, zero new formatting code |
|
||
| 12.6 test client prints both fields | `cmd_ctrlstatus` (`src/controller/testsuite/test.c`) | **Missing**: two print lines |
|
||
|
||
### Implementation Approach Options
|
||
|
||
**Req 12 term half — Option A (reuse `ha->term`, CLI-side gate) — RECOMMENDED.** No new field;
|
||
`cmd_ctrlstatus` derives the displayed `electedTerm` as `primaryName known ? term : unknown`.
|
||
*Trade-off*: ✅ zero new state, zero schema change for this half. ❌ accepts the same narrow
|
||
standby-hint transient-staleness window already accepted elsewhere in this spec (a standby that
|
||
just voted for a new candidate, before its primaryName catches up) — judged acceptable, not a
|
||
regression, and explicitly called out as a trade-off in design.md rather than glossed over.
|
||
|
||
**Req 12 term half — Option B (a dedicated learned `last_known_elected_term`, symmetric with
|
||
`last_known_committed_lsn`) — REJECTED.** Would be fully symmetric with Req 10's own pattern, but
|
||
Req 10 needed a *learned* channel specifically because the field it replaced
|
||
(`persist.committed_lsn`) is genuinely primary-only and wrong (0) off-primary — no such
|
||
pre-existing wrong-value bug exists for `term`, which is already correctly synced cluster-wide.
|
||
Adding a parallel field here would be unjustified duplication (design-synthesis Simplification
|
||
lens): same value, two storage locations, extra room for the two to drift.
|
||
|
||
**Req 12 time half — Option A (new `last_known_elected_at_ms`, unconditional heartbeat-carried
|
||
overwrite) — RECOMMENDED.** Mirrors `ha_observe_heartbeat_committed`'s existing shape (new
|
||
`heartbeat_msg_t` field, primary-side self-observe at promotion, standby-side store on receive)
|
||
but does NOT need a monotonic/value compare on write, because every write site is already gated
|
||
by an existing freshness precondition (see design.md Locked Decision 2). *Trade-off*: ✅ small,
|
||
in-boundary, symmetric with the existing learned-state pattern. ❌ two new promotion-road stamp
|
||
sites to get exactly right (see the gate-ordering gap above) — the one non-trivial risk in this
|
||
delta, called out explicitly as Research Items in design.md rather than assumed correct.
|
||
|
||
**Req 12 time half — Option B (derive an approximate elected time from the WAL_NOOP own-term
|
||
gate entry's timestamp, `ha_arm_own_term_gate`, `ha.c:5079`) — REJECTED.** The own-term no-op is
|
||
appended AFTER promotion completes and is not guaranteed to exist on every promotion road in
|
||
every configuration (comment at `ha.c:5088-5093` shows it can fail and leave the gate "UNSET,
|
||
degraded-but-safe" without aborting promotion) — an unreliable proxy for "the moment of
|
||
election." A direct, dedicated stamp at the actual role-transition instant is simpler and
|
||
strictly correct.
|
||
|
||
### Effort & Risk
|
||
|
||
- **Req 12 term half**: **XS** (a few hours) — one `cmd_ctrlstatus` print-layer change, no schema/handler change. **Risk: Low.**
|
||
- **Req 12 time half**: **S** (1–2 days) — one new `ha_state_t` field, two promotion-road stamp sites (needs the exact post-eligibility-gate line confirmed per-road, see Research Items), one heartbeat schema field + send/receive wiring, one `ctrlstatus_fill` read, one print block, TDD coverage across a unit test file, a heartbeat sibling test, and a promotion-road integration test. **Risk: Low–Medium** — the only real risk is stamping the election time at the WRONG point in `raft_cb_become_leader` (before its final eligibility gate, like the existing `last_known_committed_lsn` self-observe does), which would show a bogus election time for a promotion attempt that was actually refused; mitigated by an explicit Research Item + a dedicated "no stamp on a refused promotion" test.
|
||
- **Combined**: **S**. No heartbeat/replication *protocol* semantics change (the heartbeat message only gains one more echoed field, exactly like Req 10/11's own precedent) — stays in-boundary, no need to route anything to ha-heartbeat-liveness/ha-repl-hardening this time (unlike Req 10, which did need that routing for its "live-fresh" out-of-boundary item).
|
||
|
||
### Design Synthesis (decisions resolved for design.md)
|
||
|
||
1. **Generalization**: `electedAtMs`'s heartbeat-carry + promotion-self-observe shape generalizes
|
||
the SAME pattern `committedLsn`/`committedConfigTag` already established — no new abstraction
|
||
invented, the existing `heartbeat_msg_t`/`HeartbeatMsg` extension point is reused directly.
|
||
2. **Build vs. Adopt**: ADOPT the existing heartbeat-carried learned-state pattern wholesale
|
||
(no alternative transport considered — piggybacking on the already-flowing heartbeat is
|
||
strictly simpler than any new channel); BUILD only the one new field + two stamp sites + one
|
||
receive-side store. `electedTerm` needs no build at all (pure reuse).
|
||
3. **Simplification**: rejected a parallel `last_known_elected_term` field (Option B above) and
|
||
rejected a role-branch in `ctrlstatus_fill` for `electedAtMs` (unlike Req 10, one unconditional
|
||
read suffices here since there is no pre-existing primary-only marker to disambiguate against)
|
||
— both concrete instances of preferring the smaller design that still satisfies every
|
||
acceptance criterion.
|