lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

spec(controller-status): gap analysis — new lean opcode (not extend clusterconf); config CRC already exists; dispatch-gate allow-list is the critical gap

Browse source
This commit is contained in:
Rongsong Shen 2026-06-20 03:44:24 +00:00
parent 5b14bcf832
commit 2afaf1cbe5
1 changed files with 68 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

68
.kiro/specs/controller-status/research.md Normal file
View file

@ -0,0 +1,68 @@
# 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`.