kiro-validate-impl's feature-level requirements-coverage pass found two low-severity gaps: Requirement 4.1 (discard removed-field values without error) had no citing task, though it was already satisfied structurally by task 1.1's direct removal + capnp's default decode behavior -- cited now on task 5.1, where it's live-exercised. Requirement 3.3's citing task (5.2) didn't document how worker-liveness was verified -- added a clarifying bullet recording the actual (indirect, code-path-untouched) verification method. No functional changes.
10 KiB
Implementation Plan
-
1. Schema & Struct Foundation
-
1.1 Remove obsolete fields and the
MasterCandidatestruct from theClusterConfigschema, regenerate bindings- Remove
port(@1),candidates(@2),vcpus(@7),jobmgr(@13),workermgr(@14),qmgr(@15),resmgr(@16),xstrings(@17) fromClusterConfigincontroller.capnp - Remove the
MasterCandidatestruct entirely - Confirm
clusterName,heartbeatInterval,inactiveHeartbeatCount,fetchJobInterval,netioTimeout,resources,queues,ugroups,hgroups,hostConfigs,hproviders,limits,configGenerationremain unchanged in type and semantics; renumber them to a contiguous@0..@12range (Cap'n Proto forbids ordinal gaps — verified empirically, seeresearch.md's "Design Revision 2" — so direct removal of the 8 fields necessarily renumbers every retained field declared after a removed one) - Regenerate the capnp C bindings via the existing meson codegen step; do not hand-edit generated output
- Observable: capnp compiler succeeds against the edited schema; the generated
ClusterConfigreader/builder exposes exactly 13 fields, ordinals@0..@12, and noMasterCandidatetype - Requirements: 1.1, 1.2, 1.6, 2.1, 3.1, 4.2
- Boundary: Schema Component
- Remove
-
1.2 Remove the matching members from
cluster_config_t- Delete
port,ncandidates,candidates/master_candidate_t**,vcpus,jobmgr,workermgr,qmgr,resmgr,xstringsmembers fromcluster_config_tinclusterconfig.h - Leave all retained members (matching the schema's surviving 13 fields) untouched
- Observable:
clusterconfig.h's field list matches the regenerated schema field-for-field; any remaining reference to a removed member elsewhere in the tree is now a compile error, not a silent gap - Requirements: 1.5, 2.3, 2.4
- Depends: 1.1
- Boundary: C Struct Component
- Delete
-
1.3 Remove the orphaned
HashZArrayConfig/ResmgrConfigcapnp structs- Discovered during task 2.1's partial compile check: these structs (used only by the fields task 1.1 already removed) were deliberately left in
controller.capnp, but task 1.2 removed their corresponding hand-written C typedefs (hash_zarray_config_t,resmgr_config_t) — the capnp-generated bindings still reference those now-nonexistent types via$C.mapname(...), breaking compilation - Remove the
HashZArrayConfigandResmgrConfigstruct definitions entirely fromcontroller.capnp(grep-verify first that neither is referenced anywhere else in the schema, same method used forMasterCandidatein task 1.1) - Regenerate the capnp C bindings
- Observable: capnp compiler succeeds; the generated bindings no longer emit any
HashZArrayConfig/ResmgrConfig/hash_zarray_config_t/resmgr_config_treference anywhere - Requirements: 2.5
- Depends: 1.1, 1.2
- Boundary: Schema Component
- Discovered during task 2.1's partial compile check: these structs (used only by the fields task 1.1 already removed) were deliberately left in
-
2. Core: cluster.c Port Resolution, Debug-Dump & Port-Defaulting Cleanup
-
2.1 Update
cluster_open_listener's port-resolution fallback- Change the fallback branch from reading
cluster_config->portto a new hardcodedXCE_CONTROLLER_DEFAULT_PORTconstant (9990), reached only when--peerswas never passed at all (peers_cfg.candidates_count == 0) - Do NOT add a new CLI flag or
myconf_tfield — the--peerscandidate-match branch (including its existing single self-referencing--peers <name>=<host>:<port>usage for single-node mode) is left completely unchanged; this is the only mechanism an operator uses to get a non-default single-node port - Preserve the exact precedence and branch structure: a
--peersmatch with nonzero port binds to it unchanged; a matched candidate with port == 0 falls through to the hardcoded-default tier exactly as today's tolerated case;--peerspassed but local name absent still returns-EINVALand refuses to start, unchanged - Observable: a controller started with no
--peersbinds to 9990; a controller started with a single self-referencing--peersentry binds to that entry's port; a controller started with--peersnot containing the local name still exits with the existing-EINVALstartup error - Requirements: 1.3, 1.7
- Depends: 1.2
- Boundary: Listener/Port-Resolution Component (cluster.c)
- Change the fallback branch from reading
-
2.2 Remove the
candidatesdebug-dump mirror- Delete the
candidates/ncandidatesKV-mirror loop fromcluster_build_config - Confirm
init_controller_tunables()itself is left unchanged — the hardcoded tunable constants and their consumption are untouched by this spec - Observable:
cluster_build_configcompiles without referencingcandidatesorncandidates; the controller's internal debug/status config dump no longer includes those keys - Requirements: 1.4, 2.2
- Depends: 1.2
- Boundary: Debug-Dump Component (cluster.c)
- Delete the
-
2.3 Remove the now-dead
ClusterConfigport defaulting inmyconf.candconfig_repl.c- Delete the
SET_DEFAULT_IF_UNSET(c->port, 0, 9990)call inmyconf.c - Delete the equivalent
SET_DEFAULT_IF_UNSET(c->port, 0, 9990)call inconfig_repl.c - Observable:
myconf.candconfig_repl.ccompile without referencingcluster_config_t'sportmember - Requirements: 1.7
- Depends: 2.1
- Boundary: myconf.c, config_repl.c
- Delete the
-
3. (P) Update struct-literal test fixtures for the shrunk
cluster_config_t- Drop the
cfg.port = 9990;literal attest_commit_barrier_rollout.c:1359 - Drop the
cfg.port = 9991;literal attest_config_repl.c:983 - Grep the full tree for any other direct
cluster_config_tstruct-literal reference to a removed field and update each the same way - Observable: both known test files compile against the shrunk struct with no remaining reference to a removed field; a repo-wide grep for the removed field names against struct-literal assignments returns no further hits
- Requirements: 5.1
- Depends: 1.2
- Boundary: Test Fixture Updates
- Drop the
-
4. Build the controller and run the full unit test suite
- Build
controller_unit_test(meson target) with the regenerated capnp bindings and all updated source/test files - Run the full existing suite and compare against the pre-spec baseline
- Observable: build completes with zero compile errors;
controller_unit_testpasses with no new failures attributable to the removed fields - Requirements: 5.1, 5.2
- Depends: 1.1, 1.2, 1.3, 2.1, 2.2, 2.3, 3
- Build
-
5. Validation: Live HA Verification
-
5.1 Regenerate the
ha/data/cluster.binfixture against the renumbered schema- Decode the current (pre-spec)
ha/data/cluster.bin's retained-field values under the OLD (pre-renumbering) schema — build against a pre-1.1 checkout or otherwise capture the pre-spec field values before they become unreadable - Write and run a small throwaway capnp-encoding tool (not part of the shipped codebase — discard it after use) that constructs a
ClusterConfigwith those same retained-field values and serializes it under the NEW (post-task-1.1) schema - Overwrite
ha/data/cluster.binwith the newly-encoded blob - Observable: the regenerated
ha/data/cluster.bindecodes correctly under the post-spec schema with every retained field matching the pre-spec blob's semantic values;deploy-ha.sh's existing verbatim-copy usage (deploy-ha.sh:33-36) requires no changes and keeps working - Requirements: 3.2, 4.1, 5.3
- Depends: 4
- Decode the current (pre-spec)
-
5.2 3-node HA cluster config-push verification
- Deploy a 3-node HA cluster using the existing deploy tooling (which seeds ctrl-1 from the regenerated
ha/data/cluster.bin) and confirm peer discovery via--peers, listener binding, and worker-liveness tracking behave identically to pre-spec - Worker-liveness tracking (Req 3.3) is verified indirectly:
heartbeatInterval/inactiveHeartbeatCountare retained, unmodified fields, and this spec touched no worker-liveness code path (cluster.c's delay-task scheduling consuming them, e.g. around thecheck_worker_timeout_/liveness-poll call sites) — the observable is that the regenerated config activates cleanly on all 3 nodes with no error from that subsystem, not a live worker daemon exercising a timeout - Observable: the 3-node cluster reaches steady state with unchanged peer/listener/worker-liveness behavior, using the regenerated fixture with no error or warning
- Requirements: 3.3, 5.4
- Depends: 4, 5.1
- Deploy a 3-node HA cluster using the existing deploy tooling (which seeds ctrl-1 from the regenerated
-
5.3 Single-node startup, self-referencing-peers port override, and preserved error-path verification
- Start a controller with no
--peersflag and confirm it binds its listener to the hardcoded default port (9990) from task 2.1 - Start a controller with a single self-referencing
--peers <name>=<host>:<port>entry (own-n/--mynamematching, custom port) and confirm it binds to that port instead of the default — this is the sole replacement mechanism for a non-default single-node port under this spec - Start a controller with a
--peerslist that does not contain the controller's own name and confirm it still refuses to start with the existing-EINVALerror, unchanged from pre-spec behavior - Observable: the no-peers controller reaches a running state bound to port 9990; the self-referencing-peers controller reaches a running state bound to its specified custom port; the name-absent-from-peers controller exits with the same startup error it produced before this spec
- Requirements: 5.5
- Depends: 4
- Start a controller with no
-
6. (Optional — Requirement 6, nice-to-have, deferrable without blocking the rest of this spec) CLI-Configurable Performance Tunables
-
6.1 Add CLI flags for the hash-table tunables
- Add
--jobmgr-nhash,--workermgr-nhash,--qmgr-nhash,--resmgr-nhash,--xstrings-nhash(and corresponding-delta/-instvariants where the tunable has more than one dimension) following the sameOPT_*+struct arguments+myconf_tpattern already used for--vcpus - Observable:
xce_controller --helplists all new flags; passing one populates the correspondingmyconf_tfield - Requirements: 6.1
- Depends: 1.2
- Boundary: CLI Tunables Component (main.c, myconf.h)
- Add
-
6.2 Wire the tunables into
init_controller_tunables()- Read the new
myconf_tfields ininit_controller_tunables(), falling back to the existing hardcoded default for any field left at 0 (unset) - Observable: passing a tunable flag changes the corresponding controller tunable at startup; omitting all of them reproduces today's exact hardcoded values
- Requirements: 6.2
- Depends: 6.1
- Boundary: Tunables Component (cluster.c)
- Read the new