From ce8c333be05cbfab08eefd4934f242dddc62e53d Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sat, 20 Jun 2026 04:11:52 +0000 Subject: [PATCH] =?UTF-8?q?spec(controller-status):=20design=20review=20fi?= =?UTF-8?q?xes=20=E2=80=94=20authoritative-from-primary=20wait-primary=20(?= =?UTF-8?q?redirect),=20loaded+committed=20config=20fields,=20mermaid=20re?= =?UTF-8?q?nder=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .kiro/specs/controller-status/design.md | 106 ++++++++++++---------- .kiro/specs/controller-status/research.md | 11 +++ 2 files changed, 68 insertions(+), 49 deletions(-) diff --git a/.kiro/specs/controller-status/design.md b/.kiro/specs/controller-status/design.md index 50f1f1b..64256c1 100644 --- a/.kiro/specs/controller-status/design.md +++ b/.kiro/specs/controller-status/design.md @@ -10,9 +10,9 @@ ### Goals - A new read-only opcode returning the status fields of Requirements 2–6 in one request/response (1.1, 1.4). -- Answerable on **any** node and across failover — a standby reports the primary it knows (2.3, 7.1–7.3). -- Report a content checksum of the committed configuration so same-version divergence is detectable (5.2, 5.3). -- A `ctrlstatus` test-client command (8.x) and a `wait-primary` probe switched to this query (9.x). +- Answerable on **any** node — a standby reports a hint view (the primary it knows); an **authoritative** answer comes only from the primary, reached by following the redirect / rotating the master list (2.3, 7.1–7.3). +- Report the loaded config version + content checksum (and the committed version) so same-version divergence is detectable (5.2, 5.3). +- A `ctrlstatus` test-client command (8.x) and a `wait-primary` probe switched to an authoritative (primary-only) status query (9.x). ### Non-Goals - No operator-facing CLI/REST `lsid` surface (Go tier) — later pass. @@ -56,11 +56,11 @@ Extend-in-place along the existing request path. Dependency direction (unchanged ```mermaid graph LR - TestClient[xce_test ctrlstatus / wait-primary] --> Libxce[libxce xcecli_ctrlstatus] + TestClient[xce_test ctrlstatus and wait_primary] --> Libxce[libxce xcecli_ctrlstatus] Libxce --> Failover[XCECLI_FAILOVER_CALL is_write false] - Failover --> Gate[ha_dispatch_gate allow-list all roles] - Gate --> Handler[handle_ctrlstatus snapshot local state] - Handler --> HAState[ha_state_t and myconf_t read-only] + Failover --> Gate[ha_dispatch_gate allow_list all roles] + Gate --> Handler[handle_ctrlstatus snapshots local state] + Handler --> HAState[ha_state_t and myconf_t] Handler --> Reply[xce_reply CtrlStatus] ``` @@ -73,30 +73,27 @@ graph LR - `src/controller/ha.c` — add `OPCode_ctrlstatus` to the `ha_dispatch_gate` allow-lists for `CTRL_PRIMARY` (incl. the no-quorum branch), `CTRL_STANDBY`, `CTRL_ELECTING`, `CTRL_INIT`. **(the requirements-critical change)** - `src/lib/request.c` + `src/include/request.h` — add `xce_ctrlstatus_req` (opcode-only sender, clone of `xce_clusterconf_req`). - `src/lib/xceapi.c` — add `xce_ctrlstatus` (send req, `cbuf_get` status, `MOVE_PTR` reply union). -- `src/lib/client.c` + `src/include/client.h` — add `xcecli_ctrlstatus` (`XCECLI_FAILOVER_CALL`, `is_write=false`) and the `xcecli_free_ctrlstatus` macro. -- `src/controller/testsuite/test.c` — add `cmd_ctrlstatus` + table registration; switch `cmd_wait_primary`'s probe (currently `joblist_query(...)` at the deadline-loop call site) to `xcecli_ctrlstatus`. +- `src/lib/client.c` + `src/include/client.h` — add `xcecli_ctrlstatus` (`XCECLI_FAILOVER_CALL`, `is_write=false`), the authoritative `xcecli_ctrlstatus_primary` (follows redirect / rotates the master list until `responderRole == PRIMARY`), and the `xcecli_free_ctrlstatus` macro. +- `src/controller/testsuite/test.c` — add `cmd_ctrlstatus` + table registration (uses `xcecli_ctrlstatus`, shows the responder's view); switch `cmd_wait_primary`'s probe (currently `joblist_query(...)` at the deadline-loop call site) to `xcecli_ctrlstatus_primary` (authoritative, primary-only success). ### New Files - None (capnp C types are generated; all handlers/APIs slot into existing TUs). ## System Flows -### Status query (any role) + dispatch gate +### Status query: any-node hint then authoritative-from-primary ```mermaid sequenceDiagram participant C as Client - participant F as Failover wrapper - participant N as Controller node - participant G as ha_dispatch_gate - participant H as handle_ctrlstatus - C->>F: xcecli_ctrlstatus - F->>N: ctrlstatus request (opcode-only) - N->>G: gate(opcode, role) - G-->>N: ALLOW (all roles, incl no-quorum) - N->>H: snapshot local state - H-->>C: CtrlStatus reply - Note over C,N: On a standby, primaryName = its known primary (or "unknown"); responderRole = STANDBY + participant S as Standby node + participant P as Primary node + C->>S: ctrlstatus request + S-->>C: reply responderRole STANDBY plus known primary hint + Note over C,S: Standby answer is a non authoritative hint view + C->>P: re-query targeting the reported primary + P-->>C: authoritative reply responderRole PRIMARY ``` +The dispatch gate allow-lists `ctrlstatus` in every role (incl. no-quorum), so any node answers with its local view (satisfying any-node visibility). An **authoritative** answer comes only from the node whose `responderRole == PRIMARY`. When the first reply is from a non-primary, the client follows the redirect — it maps the reply's `primaryName` to an address via the reply's `peers` list (or rotates the configured master list) and re-queries — until it obtains the primary's authoritative reply or exhausts its budget. The plain `ctrlstatus` command may display the first (hint) reply with its "answered by" framing; `wait-primary` and any authoritative use require the primary's reply. Gate decision: `ctrlstatus` is allow-listed in every role branch and does not require quorum or a committed read — it returns the node's local view even when fenced or without quorum (precisely when an operator most wants it). ### wait-primary probe (Req 9) @@ -111,11 +108,11 @@ Gate decision: `ctrlstatus` is allow-listed in every role branch and does not re | 2.1, 2.2, 2.3 | Current primary identity (unknown when unelected; standby's known primary) | `handle_ctrlstatus` (reads `ha->primary_host` under `ha->lock`) | `CtrlStatus.primaryName` | | 3.1, 3.2, 3.3 | Answered-by node + role, distinct from primary | `handle_ctrlstatus` | `CtrlStatus.responderName`, `responderRole` | | 4.1, 4.2 | Configured controllers + count | `handle_ctrlstatus` (reads `peers_cfg`) | `CtrlStatus.peers`, `peerCount` | -| 5.1, 5.2, 5.3 | Config version + content checksum | `handle_ctrlstatus` (reads `committed_config_tag`, `config_sha256`) | `CtrlStatus.configVersion`, `configChecksum` | +| 5.1, 5.2, 5.3 | Config version + content checksum (loaded pair + committed) | `handle_ctrlstatus` (reads loaded `config_version_tag`/`config_sha256` + `committed_config_tag`) | `CtrlStatus.configVersion`, `configChecksum`, `committedConfigVersion` | | 6.1–6.4 | Term, commit/apply, fenced, cluster name | `handle_ctrlstatus` | `CtrlStatus.term`, `committedLsn`, `appliedLsn`, `fenced`, `clusterName` | | 7.1, 7.2, 7.3 | Failover-aware any-node delivery | `xcecli_ctrlstatus`, dispatch-gate allow-lists | `XCECLI_FAILOVER_CALL` (is_write=false), gate entries | | 8.1, 8.2, 8.3 | Test-client `ctrlstatus` command | `cmd_ctrlstatus` | client command + exit code | -| 9.1–9.4 | wait-primary uses the status query | `cmd_wait_primary` probe swap | `xcecli_ctrlstatus` | +| 9.1–9.4 | wait-primary uses an authoritative (primary-only) status probe | `cmd_wait_primary` probe swap, `xcecli_ctrlstatus_primary` | redirect-to-primary, `responderRole == PRIMARY` success | ## Components and Interfaces @@ -124,9 +121,9 @@ Gate decision: `ctrlstatus` is allow-listed in every role branch and does not re | `CtrlStatus`/`CtrlPeer` schema | Schema | Reply payload for the status query | 2–6 | capnp codegen (P0) | API (reply) | | `handle_ctrlstatus` | Controller | Snapshot local HA/config state into the reply | 1–6 | `ha_state_t`, `myconf_t`, persist accessors (P0) | Service | | dispatch-gate allow-lists | Controller HA | Permit `ctrlstatus` in every role | 7 | `ha_dispatch_gate` (P0) | Service | -| `xce_ctrlstatus` / `xcecli_ctrlstatus` | libxce | Failover-aware client call + free | 1, 7 | failover wrapper (P0) | Service | -| `cmd_ctrlstatus` | Test client | Print the status | 8 | `xcecli_ctrlstatus` (P0) | State | -| `cmd_wait_primary` probe | Test client | Use status as the serving-primary probe | 9 | `xcecli_ctrlstatus` (P0) | State | +| `xcecli_ctrlstatus` / `xcecli_ctrlstatus_primary` | libxce | Failover-aware read (any-node) + authoritative reach-primary read + free | 1, 7, 9 | failover wrapper, redirect hint (P0) | Service | +| `cmd_ctrlstatus` | Test client | Print the status (responder view) | 8 | `xcecli_ctrlstatus` (P0) | State | +| `cmd_wait_primary` probe | Test client | Authoritative serving-primary probe | 9 | `xcecli_ctrlstatus_primary` (P0) | State | ### Schema — reply payload (Cap'n Proto) ```capnp @@ -137,20 +134,24 @@ struct CtrlPeer $C.mapname("xce_ctrlpeer_t") { } struct CtrlStatus $C.mapname("xce_ctrlstatus_t") { - clusterName @0: Text; - primaryName @1: Text; # empty ⇒ "unknown" (no elected/known primary) - responderName @2: Text; # the node that answered - responderRole @3: UInt8; # ctrl_role_t: INIT/ELECTING/PRIMARY/STANDBY - term @4: UInt64; - configVersion @5: UInt64; # committed_config_tag (8-byte SHA-256 prefix) - configChecksum @6: Data; # full committed-config SHA-256 (32 bytes) - committedLsn @7: UInt64; - appliedLsn @8: UInt64; - fenced @9: Bool; # in_catch_up OR recovery_unrecoverable - peers @10: List(CtrlPeer) $C.mapname("peers.list") $C.maplistcount("peers.count"); - peerCount @11: UInt32; + clusterName @0: Text; + primaryName @1: Text; # empty ⇒ "unknown" (no elected/known primary) + responderName @2: Text; # the node that answered + responderRole @3: UInt8; # ctrl_role_t: INIT/ELECTING/PRIMARY/STANDBY + term @4: UInt64; + # Loaded config (consistent pair: version is the 8-byte prefix of the checksum) + configVersion @5: UInt64; # loaded config_version_tag (8-byte SHA-256 prefix) + configChecksum @6: Data; # loaded config_sha256 (full 32 bytes) + # Committed config version (may differ from loaded mid config-change) + committedConfigVersion @7: UInt64; # committed_config_tag + committedLsn @8: UInt64; + appliedLsn @9: UInt64; + fenced @10: Bool; # in_catch_up OR recovery_unrecoverable + peers @11: List(CtrlPeer) $C.mapname("peers.list") $C.maplistcount("peers.count"); + peerCount @12: UInt32; } ``` +- **Config fields (Req 5):** `configVersion`/`configChecksum` are the **loaded** config's tag and full SHA-256 — a self-consistent pair (the version is the first 8 bytes of the checksum), so two nodes with identical loaded content match on both and divergent content differs on both (5.3). `committedConfigVersion` is the separately-tracked committed tag, exposing "what I run" vs "what is committed"; in steady state all three agree. - New OPCode ordinal: `ctrlstatus @40` (next free after `preVoteReply @39`). New reply-union member: `ctrlstatus @17` (next free after `walackreplymsg @16`). Field ordinals are append-only — never reuse/reorder. #### `handle_ctrlstatus` — Service contract @@ -161,7 +162,7 @@ struct CtrlStatus $C.mapname("xce_ctrlstatus_t") { **Responsibilities & Constraints** - Read-only: must not modify any HA, persist, or config state (1.2). -- Field sourcing: `clusterName`/`responderName` from `c->myconf`/own peerinfo; `responderRole`, `term`, `appliedLsn`, `committedLsn`, `committed_config_tag` via `atomic_load_explicit(..., acquire)`; `primaryName`/peers as below; `fenced = in_catch_up || persist_recovery_unrecoverable(...)`; `configChecksum` from `config_sha256[32]`. +- Field sourcing: `clusterName`/`responderName` from `c->myconf`/own peerinfo; `responderRole`, `term`, `appliedLsn`, `committedLsn`, `committedConfigVersion` (`committed_config_tag`) via `atomic_load_explicit(..., acquire)`; `configVersion` from the loaded `config_version_tag` and `configChecksum` from the loaded `config_sha256[32]` (a self-consistent pair — Req 5.3); `primaryName`/peers as below; `fenced = in_catch_up || persist_recovery_unrecoverable(...)`. - **Locking:** `primary_host`/`primary_port` are guarded by `ha->lock` — take it for a consistent snapshot, copy out, release. All other fields are atomics or immutable config. - **"unknown" mapping (1.3, 2.2, 2.3):** if `primary_host[0] == '\0'` (no known primary), emit an empty `primaryName`, which the client renders as `"unknown"`. - **Peers (4.x):** iterate `c->myconf->peers_cfg.candidates[0..count-1]` → one `CtrlPeer{name,host,port}` each; set `peerCount`. @@ -182,26 +183,32 @@ struct CtrlStatus $C.mapname("xce_ctrlstatus_t") { int xce_ctrlstatus_req(endpoint_t* ep); /* send + await reply; on success MOVE_PTR the reply union out */ int xce_ctrlstatus(xce_ctx_t* ctx, xce_ctrlstatus_t** status); -/* failover-aware read (is_write=false) */ +/* failover-aware read (is_write=false): returns whatever node answers (may be a + * standby hint view). Use for the operator command. */ int xcecli_ctrlstatus(xcecli_ctx_t c, xce_ctrlstatus_t** status); +/* AUTHORITATIVE read: returns only a reply whose responderRole == PRIMARY. If the + * first answer is from a non-primary, follow the redirect — target the reported + * primaryName via the reply's peers list, else rotate the configured master list + * — until the primary answers or the failover budget is exhausted. */ +int xcecli_ctrlstatus_primary(xcecli_ctx_t c, xce_ctrlstatus_t** status); #define xcecli_free_ctrlstatus(s) xce_free_ctrlstatus(s) ``` - Preconditions: valid client context. -- Postconditions: on success `*status` owns a decoded `CtrlStatus` (caller frees); on failure returns a typed negative (7.3) and `*status` untouched. +- Postconditions: on success `*status` owns a decoded `CtrlStatus` (caller frees); on failure returns a typed negative (7.3) and `*status` untouched. For `xcecli_ctrlstatus_primary`, success additionally guarantees `responderRole == PRIMARY` (an authoritative answer). - Invariants: `is_write=false` — retryable/rotatable across the configured controllers. #### `cmd_ctrlstatus` (test client) — State contract -- Calls `xcecli_ctrlstatus`; on success prints the fields in human-readable form (8.1), framing the responder as `answered by ()` distinct from `Primary:` (3.3); renders empty `primaryName` as `unknown`; renders `configChecksum` as hex. Returns 0 on success, `<0` on failure with the error printed (8.2, 8.3). Frees the reply. +- Calls `xcecli_ctrlstatus`; on success prints the fields in human-readable form (8.1) — `Primary`, `answered by ()` distinct from `Primary:` (3.3), peers + count, `configVersion` and `committedConfigVersion`, `configChecksum` as hex, term, commit/apply, fenced, cluster name; renders empty `primaryName` as `unknown`. Returns 0 on success, `<0` on failure with the error printed (8.2, 8.3). Frees the reply. #### `cmd_wait_primary` probe swap — State contract -- Replace the single `joblist_query(...)` probe call in the deadline loop with `xcecli_ctrlstatus`. -- **Success predicate (9.1, 9.2):** the status reply reports a non-`"unknown"` `primaryName`. On success, complete and print the served primary's identity. +- Replace the single `joblist_query(...)` probe call in the deadline loop with `xcecli_ctrlstatus_primary` (the authoritative variant). +- **Success predicate (9.1, 9.2):** the probe obtains an authoritative reply with `responderRole == PRIMARY` (the answer came from the serving primary). On success, complete and print that primary's identity. A standby-only/hint answer is NOT success — the probe follows the redirect to the primary (or keeps polling) until the primary answers or the deadline elapses. +- **Decision (resolves review Issue 1):** require the answer to come **from the primary**, not merely "a primary name was reported by some node." This avoids a false success on a standby's stale `primary_host` after a primary death (the false-positive class removed from the durability soak) and faithfully meets Req 9.1's "**serving** primary." The any-node hint view remains available for the operator `ctrlstatus` command via `xcecli_ctrlstatus`. - **Preserved (9.4):** the timeout-argument parsing, monotonic deadline, poll interval, log-level quieting, and success/failure exit codes are unchanged; only the probe call and the success message change. -- **Decision & caveat:** the predicate is "a known primary is reported" rather than "the responder is itself the primary," because the failover read may legitimately be answered by a standby; requiring `responderRole == PRIMARY` could starve the predicate if a standby always answers first. The accepted caveat is a brief window where a standby may report a just-failed primary before it clears; this is acceptable for a "wait until a primary exists" gate and is strictly more informative than the prior job-list probe (it names the primary). ## Data Models -The only new data is the reply payload above. `CtrlStatus` is a flat value object (no persistence, no aggregate); `CtrlPeer` is a bounded value list (≤ configured controller count, typically 3). `configChecksum` is the 32-byte committed-config SHA-256; `configVersion` is its 8-byte prefix as a `UInt64`. Two nodes with identical committed config content produce identical `configChecksum` and `configVersion`; differing content differs in both (5.3). +The only new data is the reply payload above. `CtrlStatus` is a flat value object (no persistence, no aggregate); `CtrlPeer` is a bounded value list (≤ configured controller count, typically 3). `configChecksum` is the loaded config's 32-byte SHA-256 and `configVersion` is its own 8-byte prefix (a self-consistent pair); `committedConfigVersion` is the separately-tracked committed tag. Two nodes with identical loaded config content produce identical `configVersion` + `configChecksum`; differing content differs in both (5.3). A node mid config-change may show `configVersion != committedConfigVersion`. ## Error Handling @@ -216,15 +223,16 @@ The only new data is the reply payload above. `CtrlStatus` is a flat value objec - `handle_ctrlstatus` on a PRIMARY returns `primaryName==self`, `responderRole==PRIMARY`, populated peers/version/checksum (2.1, 3.x, 4.x, 5.x). - `handle_ctrlstatus` on a STANDBY with a known primary returns that primary and `responderRole==STANDBY`; with no known primary returns empty `primaryName` (2.2, 2.3). - `fenced` reflects `in_catch_up`/`recovery_unrecoverable` (6.3); the handler mutates no state (1.2). -- `configChecksum`/`configVersion` equal across two nodes with identical config, differ when content differs (5.3). +- `configVersion`/`configChecksum` are a self-consistent loaded pair — equal across two nodes with identical loaded config, differ when content differs; `committedConfigVersion` is exposed separately (5.1, 5.2, 5.3). ### Integration - Dispatch-gate: a `ctrlstatus` request is answered (not `-XCE_ERR_STANDBY`/`-XCE_ERR_NOT_READY`) on STANDBY, ELECTING, INIT, and PRIMARY-no-quorum nodes; no other opcode's gating changed (7.1, 7.2). - libxce round-trip: `xcecli_ctrlstatus` decodes all fields; `is_write=false` rotates across configured controllers; no-reachable-controller returns a typed failure (7.3). +- Authoritative read: `xcecli_ctrlstatus_primary`, when the first answer is from a standby, follows the redirect to the primary and returns a reply with `responderRole == PRIMARY`; it never returns a standby/hint reply as the authoritative result (9.1, 9.2). ### E2E (live `/xce-deploy-ha`) -- `ctrlstatus` against a healthy 3-node cluster prints the primary, peers, version+checksum; querying a standby still reports the primary (2.3, 8.1). -- `wait-primary` (status-probe) returns immediately on a healthy cluster and names the primary; after a primary kill, blocks until a new primary is reported, then names it; times out when none (9.1–9.4). Directly useful to the `ha-commit-durability` Phase 6 liveness/>60s-election investigation. +- `ctrlstatus` against a healthy 3-node cluster prints the primary, peers, version+checksum; querying a standby still reports the primary with `answered by ` (2.3, 8.1). +- `wait-primary` (authoritative probe) returns immediately on a healthy cluster and names the primary; after a primary kill, blocks until the **new primary itself** answers, then names it; does NOT false-succeed on a standby's stale primary view; times out when no primary serves (9.1–9.4). Directly useful to the `ha-commit-durability` Phase 6 liveness/>60s-election investigation. ## Security Considerations diff --git a/.kiro/specs/controller-status/research.md b/.kiro/specs/controller-status/research.md index 0b381ed..d57fa5b 100644 --- a/.kiro/specs/controller-status/research.md +++ b/.kiro/specs/controller-status/research.md @@ -80,3 +80,14 @@ The 6 carried research items are resolved as: 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).