save status

.kiro/steering/config-design.md

@@ -0,0 +1,72 @@
+# Configuration Design: What Belongs in Shared Config
+
+Governs any XCE distributed-config schema decision — `ClusterConfig` (controller.capnp), and any future worker/scheduler/apiserver shared-config struct. Applies whenever a new field is proposed for a config blob that gets pushed/replicated to multiple nodes (e.g. via `OP_upgradeConfig` or an equivalent future mechanism).
+
+## The Question
+
+A parameter needs a home. Two choices:
+- **Shared config** — part of the cluster-wide config blob, distributed to every node, changed via a single coordinated push (build → validate → replicate → majority ACK).
+- **Local/instance config** — set per-node (CLI flag, local config file, env var), changed independently, no cluster-wide coordination.
+
+Getting this wrong in either direction has a cost: putting a per-instance value in shared config forces every routine node-level change through a cluster-wide config push; putting a cluster-invariant value in local config lets nodes silently drift out of sync.
+
+## Rule 0: Bootstrap availability (checked first, overrides everything else)
+
+> If a parameter is needed to establish consensus, elect a coordinator, or otherwise receive the shared config in the first place, it CANNOT live inside that shared config.
+
+This isn't a judgment call like rules 1–2 below — it's a structural/chicken-egg constraint. A node can't depend on config it doesn't have yet to obtain that same config. Example: an HA/Raft election heartbeat interval/timeout — a controller needs this value to participate in the election that determines who's primary and delivers the cluster config; it must be locally available (CLI flag, hardcoded default) before any config exists. This is independent of whether the field would otherwise pass or fail the independence/frequency rules below — even a field that's cluster-invariant and rarely-changing (which would normally mean "shared") is disqualified from shared config if it's needed to bootstrap access to that config.
+
+If a candidate field passes Rule 0 (i.e., it isn't a bootstrap dependency), proceed to the two rules below.
+
+## The Two Rules
+
+Apply both to every candidate field. **Excluding on either rule is sufficient** — a field only belongs in shared config if it fails both. Note: failing both rules means shared config is the *default* home, not automatic — see "Operational-uniformity override" below for when a field that would otherwise qualify for local config gets deliberately centralized anyway.
+
+### 1. Independence rule
+
+> If a parameter can change on one node without requiring every other node to match, it does NOT belong in shared config.
+
+Test: does divergence between nodes on this field cause a correctness problem, or is it just... fine? If fine, it's independent — keep it local.
+
+### 2. Frequency rule
+
+> If a parameter is expected to change more often than the rest of the config, it does NOT belong in shared config.
+
+Test: would this field's natural rate of change force config pushes far more often than everything else in the blob needs? If yes, bundling it in shared config makes routine operational changes as heavyweight as a schema change.
+
+### Default: both rules fail → include
+
+A field that must be identical across every node AND changes rarely belongs in shared config. That's the actual reason shared config exists — to keep genuinely cluster-invariant values from drifting.
+
+## Operational-uniformity override
+
+Passing the independence rule (no correctness requirement to match across nodes) means a field is *eligible* for local config — it does not mean it *must* go local. A field can still be deliberately kept in shared config when centralizing it has operational value beyond pure correctness: predictable behavior across the cluster, one less per-node setting for an operator to track, avoiding "why does this node behave differently" support questions. This is a judgment call, not automatic — when exercising it, document the reasoning inline (don't just leave a field in shared config by default without stating why it's an exception).
+
+Worked example: `ClusterConfig`'s `heartbeatInterval`/`inactiveHeartbeatCount` fields govern controller→**worker** liveness (when a controller marks one of its own workers unavailable) — not the HA/Raft controller-to-controller election heartbeat (see Rule 0 example above; those are two different "heartbeat" concepts sharing similar names, easy to conflate). Each controller only manages its own connected workers, so nothing breaks if this value differs node-to-node — it passes the independence rule cleanly. Decided (2026-07-20) to keep it in shared config anyway: operators want uniform, predictable worker-timeout behavior across the cluster rather than per-node drift, even though no correctness invariant demands it.
+
+## Worked Examples
+
+| Field | Rule 0 (bootstrap) | Independence | Frequency | Verdict | Why |
+|---|---|---|---|---|---|
+| Cluster name | passes (not a bootstrap dep) | fails — every node must agree | fails — set once at bootstrap | **Shared** | Identity mismatch across nodes is a correctness bug |
+| HA/Raft election heartbeat interval/timeout | **fails Rule 0** — needed to run the election that delivers config in the first place | n/a — disqualified before reaching this rule | n/a | **Local** (CLI flag; currently hardcoded, see roadmap) | Can't depend on config you don't have yet to obtain that config |
+| Controller→worker liveness heartbeat/timeout | passes (not a bootstrap dep) | passes — one controller's worker-timeout tolerance has no cross-node dependency | — | **Shared** (operational-uniformity override, not a rule outcome — see above) | Would qualify for local by the rules alone; kept shared for predictable cluster-wide behavior |
+| Listen port | passes | passes — each node's own bind port has no cross-node dependency | — | **Local** | Purely a per-process network setting |
+| Peer/candidate list (cluster membership) | passes | passes — adding a standby doesn't require the rest of the config to change | passes — routine operational action | **Local** (own mechanism) | Coupling membership changes to a full config push makes scaling the cluster as heavyweight as a schema migration |
+| Internal hash-table sizes, buffer sizes (perf tuning) | passes | passes — effect is local to the process that sets them | — | **Local** | No node needs to know another node's internal tuning |
+| Any monotonic version/generation counter | n/a — not operator input | n/a | n/a | **Not config at all** | Protocol state the coordinator (primary) assigns, not a value an operator sets |
+
+The last row is a distinct fourth category, not a rule outcome: some fields aren't "config" in the operator-input sense — they're state the system itself maintains (e.g. a config generation counter incremented by the primary on every push). Don't force these into the shared-config-vs-local-config framing; they're neither.
+
+## Applying This
+
+When a new field is proposed for a shared config blob:
+1. Check Rule 0 first — if the field is needed to bootstrap/obtain the config itself, it's local, full stop, regardless of what rules 1–2 would say.
+2. Otherwise, run it through the independence rule, then the frequency rule.
+3. If it fails either, its default home is local (CLI flag, per-node config file, a dedicated dynamic-membership mechanism, etc.) — unless the operational-uniformity override applies and is explicitly documented as a deliberate exception.
+4. If it passes both (fails neither exclusion), add it to shared config — and document why in the owning spec, so the next person doesn't have to re-derive it.
+5. If it's coordinator-assigned protocol state rather than operator input, keep it out of the operator-facing config schema entirely.
+
+## Origin
+
+Derived 2026-07-20 while classifying `ClusterConfig`'s 21 fields for `xceadmin config build` (`.kiro/specs/admin-config-tool/`) — 6 fields were being pulled into shared YAML config by default with no principled reason to include vs. exclude them. See `brief.md`'s "Configuration Inclusion Rules" section in that spec for the full field-by-field classification of `ClusterConfig` specifically; this file holds the general rule for reuse beyond that one spec.

.kiro/steering/roadmap.md

@@ -88,7 +88,8 @@ Smaller items that likely don't warrant a full spec; pick up opportunistically o
 - [ ] xce_test WAL/snapshot inspection tooling — extend the offline inspector family in `xce_test` (sibling of the shipped `waldump`, `src/controller/testsuite/waldump.{c,h}` + `cmd_waldump`). Two remaining commands: **(a) `snapdump <snap_dir|snap_file>`** — decode the `XCESNAP\0` header (8-byte LSN; `persist.c:1266` `load_snapshot`) and report whether the snapshot carries job/queue entities or is an LSN-only stub. This directly settles Open Fork #3 of the failover job-loss investigation (`.kiro/specs/ha-commit-durability/INVESTIGATION-failover-job-loss.md`): if snapshots are entity-less, any empty-rejoin/WAL-prune below committed is unrecoverable BY DESIGN. **(b) `walcompare <dirA> <dirB>`** — diff two nodes' WALs by `lsn → crc/jobid` to flag divergence/prefix-gaps between members instantly. Reuse the `waldump.c` reader/decoder; same zero-alloc capnp jobid decode + `c_capnproto_dep`/`zlib_dep` wiring already added to `xce_test_deps`. Nonzero exit on entity-less snapshot / divergence so both double as E2E assertions. _(recorded 2026-06-20; `waldump` already done.)_
 - [ ] xce_test shell command support — add `!<command>` prefix to `xce_test` test scripts to execute arbitrary shell commands inline during test runs. Enables test scripts to drive environment setup/teardown (e.g., kill a node, write a file, run a helper) without calling out to a separate harness layer. Scope: parser change in the xce_test runner to detect `!`-prefixed lines, fork/exec the remainder as a shell command, fail the test on non-zero exit, capture stdout/stderr to the test log.
 - [→] scheduler framework enhancement — **PROMOTED to spec 2026-07-06** (`/kiro-discovery`; brief at `.kiro/specs/scheduler-framework/brief.md`). Original backlog seed superseded by the full brief with iterator-driven allocation architecture, configurable job picker, combinatorial parallel job allocation, and session-level unsatisfiable resreq cache. Scope narrowed from the original seed: commit barrier wiring deferred (separate hardening item); GPU/affinity allocation deferred (pluggable via new allocator interface); new job selection policies deferred (framework enables them, ships separately). Next: `/kiro-spec-requirements scheduler-framework`.
-- [ ] **perf-tuning-extraction** — **NEW 2026-07-06 from admin-config-tool discovery** (`/kiro-discovery`). Audit `ClusterConfig` fields in `controller.capnp` and identify parameters used only for controller performance tuning (e.g., heartbeat intervals, timeout values, pool sizes, WAL segment sizing). Extract them from the cluster config and create a separate integration plan document for future performance tuning (how to expose, configure, validate, and apply perf tunables without requiring a full cluster config upgrade). This is direct-implementation work (documentation + field audit), not a spec.
+- [→] **perf-tuning-extraction** — **PROMOTED to spec 2026-07-20** (`clusterconfig-schema-split`, `.kiro/specs/clusterconfig-schema-split/`). Originally recorded 2026-07-06 from admin-config-tool discovery as a documentation/audit item; the audit (2026-07-20) found the fields are load-bearing enough to need real code changes, not just a doc pass — `candidates`/`vcpus`/hash-tuning fields (`jobmgr`/`workermgr`/`qmgr`/`resmgr`/`xstrings`) are already dead weight in the `ClusterConfig` capnp blob (superseded by `--peers`/`--vcpus` CLI flags or never read into runtime state — see `src/docs/cluster-config-split-analysis.md`). **`port` is NOT dead** — a second review pass (2026-07-20) found `cluster_open_listener` (`cluster.c:1216-1258`) falls back to the blob's `port` field, and single-node mode (no `--peers` flag at all, `main.c:293-294`) depends on that fallback entirely; the spec now requires a replacement port-sourcing mechanism before the field can be removed (`clusterconfig-schema-split` Requirement 1.7), not a bare deletion. `heartbeatInterval`/`inactiveHeartbeatCount` (controller→worker liveness, NOT the HA/Raft election heartbeat) were investigated and deliberately kept IN `ClusterConfig` for operational uniformity despite passing the independence rule — see `.kiro/steering/config-design.md`. `fetchJobInterval`/`netioTimeout` left unresolved, flagged out-of-scope in the new spec for a future pass.
+- [ ] **Raft/HA heartbeat interval+timeout as CLI parameters** — **NEW 2026-07-20 from clusterconfig-schema-split discovery.** The controller-to-controller HA/Raft election heartbeat (`ha->config.heartbeat_interval_ms`/`heartbeat_timeout_ms` — the one where a cross-node mismatch risks spurious failover) is 100% hardcoded today (`HA_DEFAULT_HEARTBEAT_INTERVAL_MS`/`HA_DEFAULT_HEARTBEAT_TIMEOUT_MS`, `cluster.c:1131`) — there is no `ClusterConfig` field for it, and per user decision (2026-07-20) there should never be one: these values must be available to a controller BEFORE it has validated/received cluster configuration (a controller needs its own election-timing parameters to even participate in the election that delivers config in the first place — a bootstrap/chicken-egg constraint, not an independence/frequency judgment call). Target: `--heartbeat-interval-ms`/`--heartbeat-timeout-ms` CLI flags following the exact `--vcpus` pattern (`OPT_*` + `struct arguments` + `myconf_t` field, `0` sentinel = use hardcoded default). Kept hardcoded for now; this is a future item, not blocking `clusterconfig-schema-split`.
 
 ## Superseded / Absorbed Items