update documents

This commit is contained in:
Rongsong Shen 2026-08-10 17:51:51 +08:00
parent c705cadadc
commit d70bf6a402
51 changed files with 995 additions and 1997 deletions

View file

@ -83,7 +83,7 @@ graph TB
**Architecture Integration**:
- Selected pattern: no new CLI surface — `port` resolution reuses the existing `--peers` candidate-match loop (a single self-referencing `--peers <name>=<host>:<port>` entry already gives single-node mode a custom port today) plus a hardcoded fallback constant for the true zero-`--peers` case.
- Domain/feature boundaries: `ClusterConfig` (schema) retains only fields that are genuinely cluster-shared (per `config-design.md`); `peers_cfg` (CLI-sourced, per-process, already existing) owns port resolution for everything this spec touches.
- Existing patterns preserved: `--peers` remains the sole port/membership source; `init_controller_tunables()`'s hardcoded constants are untouched (Requirement 2.2 — no behavior change).
- Existing patterns preserved: `--peers` remains the sole port/membership source; `init_controller_tunables()`'s hardcoded constants are the default values and reproduce today's exact behavior when no tunable flag is passed (Requirement 2.2 — no behavior change by default). (Note: the optional Requirement 6 CLI-sourcing was in fact implemented — `init_controller_tunables()` now reads the corresponding `myconf_t` field first, falling back to the hardcoded constant only when that field is 0/unset — so the function itself is no longer literally untouched, though default behavior is unchanged.)
- New components: one new hardcoded constant (`cluster.c` or `clusterconfig.h`) — the minimum needed to satisfy Requirement 1.7 without any invocation-contract or CLI-surface change.
- Steering compliance: enforces `.kiro/steering/config-design.md`'s independence/frequency/Rule-0 classification; follows `src/controller/CLAUDE.md`'s HA protocol verification checklist for the touched files.
@ -100,7 +100,7 @@ graph TB
### Modified Files
- `src/schema/controller.capnp` — remove `port` (@1), `candidates` (@2), `vcpus` (@7), `jobmgr` (@13), `workermgr` (@14), `qmgr` (@15), `resmgr` (@16), `xstrings` (@17) from `ClusterConfig`; remove the `MasterCandidate` struct entirely; remove the now-orphaned `HashZArrayConfig`/`ResmgrConfig` structs (used only by the removed fields — discovered during `/kiro-impl` task 2.1, their `$C.mapname` bindings broke the capnp-generated code once task 1.2 removed the C typedefs they referenced). Cap'n Proto requires gapless ordinals, so the 13 retained fields are renumbered to a contiguous `0..12` range as a direct consequence — see `research.md`'s ordinal-gap finding (Req 1.1, 1.2, 1.6, 2.1, 2.5, 3.1).
- `ha/data/cluster.bin` (binary fixture, not source) — regenerated in place against the renumbered schema, preserving the retained fields' semantic values from the pre-spec blob (Req 3.2, 5.3).
- `ha/data/cluster.bin` (binary fixture, not source) — regenerated in place against the renumbered schema, preserving the retained fields' semantic values from the pre-spec blob (Req 3.2, 5.3). (Note: superseded by `admin-config-tool` (landed 2026-07-22, one day after this spec closed) — `ha/data/` was deleted entirely and `deploy-ha.sh` now builds `cluster.bin` fresh via `xceadmin config build` from `src/configs/*.yaml` on every deploy run, not from a checked-in regenerated blob. See that spec's design.md.)
- `src/include/clusterconfig.h` — remove the matching `cluster_config_t` members: `port`, `ncandidates`, `candidates`/`master_candidate_t**`, `vcpus`, `jobmgr`, `workermgr`, `qmgr`, `resmgr`, `xstrings` (Req 1.5, 2.4).
- `src/controller/cluster.c` — (a) remove the `candidates`/`ncandidates` KV-mirror loop in `cluster_build_config` (Req 1.4); (b) change `cluster_open_listener`'s fallback branch from reading `cluster_config->port` to a hardcoded default constant, reached only when `--peers` was never passed at all — the `--peers` candidate-match branch itself is unchanged (Req 1.3, 1.7); `init_controller_tunables()` itself is unchanged (Req 2.2).
- `src/controller/myconf.c` — remove the `SET_DEFAULT_IF_UNSET(c->port, 0, 9990)` call (the field it defaults no longer exists) (part of Req 1.7's cleanup).
@ -111,9 +111,9 @@ graph TB
### Verification Infrastructure (existing, no new files)
- `controller_unit_test` (meson target, existing) — runs the full existing suite including the two updated test files above (Req 5.2).
- A small throwaway capnp-encoding tool (written and discarded during implementation, not part of the shipped codebase) — decodes `ha/data/cluster.bin`'s retained-field values under the pre-spec schema and re-encodes them under the post-spec (renumbered) schema, overwriting the fixture (Req 3.2, 5.3).
- `/xce-deploy-ha` skill (existing, `src/tools/deploy-ha.sh`) — deploys the 3-node live cluster for the config-push cycle, using the regenerated `ha/data/cluster.bin` (Req 5.4).
- Direct `xce_controller` invocation with no `--peers` flag (existing supported mode, `main.c:293-295`) — the single-node startup verification (Req 5.5).
- A small throwaway capnp-encoding tool (written and discarded during implementation, not part of the shipped codebase) — decodes `ha/data/cluster.bin`'s retained-field values under the pre-spec schema and re-encodes them under the post-spec (renumbered) schema, overwriting the fixture (Req 3.2, 5.3). (Note: the fixture this tool targeted no longer exists — see the `ha/data/cluster.bin` note above.)
- `/xce-deploy-ha` skill (existing, `src/tools/deploy-ha.sh`) — deploys the 3-node live cluster for the config-push cycle, using the regenerated `ha/data/cluster.bin` (Req 5.4). (Note: superseded by `admin-config-tool` — `deploy-ha.sh` now generates `cluster.bin` fresh via `xceadmin config build` instead of consuming this fixture. See that spec's design.md.)
- Direct `xce_controller` invocation with no `--peers` flag (existing supported mode, `main.c:441-443` — line numbers shifted from the original `main.c:293-295` after this spec's own Requirement 6 CLI flags were added earlier in the file) — the single-node startup verification (Req 5.5).
- Direct `xce_controller` invocation with a single self-referencing `--peers <name>=<host>:<port>` entry (existing supported mode) — verifies the custom-port replacement mechanism itself (Req 5.5).
### Optional (Requirement 6, nice-to-have)
@ -149,8 +149,8 @@ flowchart TD
```
**Key decisions** (corrected from the initial draft during `/kiro-validate-design`; the `--port` CLI flag was further revised away in favor of the mechanism below during `/kiro-impl` kickoff — see `research.md`'s "Design Revision" section):
- **Fallback trigger is port value, not just match presence.** A matched candidate whose port field is `0` (a malformed `--peers` entry — already WARNed about at `main.c:289-291`) falls through to the hardcoded-default tier exactly like today's `if (my_port == 0)` check, rather than binding to literal port 0. This preserves Requirement 1.3's "exactly as it does today" for the already-tolerated malformed-entry case.
- **Error path is preserved, but only for one specific case.** `candidates_count == 0` can only happen when `--peers` was never passed at all — a passed-but-empty or malformed `--peers` string is rejected earlier by `xce_parse_peers` itself (`src/lib/cluster.c:58` returns `-1` unless `candidates_count > 0`), which `main.c` treats as a startup error before `cluster_open_listener` is ever reached (corrected citation, 2026-07-20: this guarantee comes from `xce_parse_peers`'s contract, not `validate_peer_count`, which only rejects even non-zero counts) — that's genuine single-node mode, and gets the hardcoded-default fallback per Requirement 1.7. An operator who wants a non-default port in this mode passes a single self-referencing `--peers <name>=<host>:<port>` entry instead — an already-existing, already-supported invocation (this resolves via the unchanged `NameFound`/`PortValid` branches above, not a new mechanism). But when `--peers` WAS passed and the local name simply isn't in it, that's a likely operator misconfiguration (own name typo'd or omitted) — this spec keeps today's `-EINVAL`/refuse-to-start behavior for that case rather than silently defaulting, per the project's fail-loud convention (`src/controller/CLAUDE.md` process-lifecycle invariant). Decided interactively during design review rather than defaulting to "simplest single code path."
- **Fallback trigger is port value, not just match presence.** A matched candidate whose port field is `0` (a malformed `--peers` entry — already WARNed about at `main.c:434-439`, was `main.c:289-291`; line numbers shifted after this spec's own Requirement 6 CLI flags were added earlier in the file) falls through to the hardcoded-default tier exactly like today's `if (my_port == 0)` check, rather than binding to literal port 0. This preserves Requirement 1.3's "exactly as it does today" for the already-tolerated malformed-entry case.
- **Error path is preserved, but only for one specific case.** `candidates_count == 0` can only happen when `--peers` was never passed at all — a passed-but-empty or malformed `--peers` string is rejected earlier by `xce_parse_peers` itself (`src/lib/cluster.c:59` returns `-1` unless `candidates_count > 0`; was cited as `:58`), which `main.c` treats as a startup error before `cluster_open_listener` is ever reached (corrected citation, 2026-07-20: this guarantee comes from `xce_parse_peers`'s contract, not `validate_peer_count`, which only rejects even non-zero counts) — that's genuine single-node mode, and gets the hardcoded-default fallback per Requirement 1.7. An operator who wants a non-default port in this mode passes a single self-referencing `--peers <name>=<host>:<port>` entry instead — an already-existing, already-supported invocation (this resolves via the unchanged `NameFound`/`PortValid` branches above, not a new mechanism). But when `--peers` WAS passed and the local name simply isn't in it, that's a likely operator misconfiguration (own name typo'd or omitted) — this spec keeps today's `-EINVAL`/refuse-to-start behavior for that case rather than silently defaulting, per the project's fail-loud convention (`src/controller/CLAUDE.md` process-lifecycle invariant). Decided interactively during design review rather than defaulting to "simplest single code path."
## Requirements Traceability
@ -165,7 +165,7 @@ flowchart TD
| 1.7 | Replacement port-sourcing mechanism (existing `--peers` self-reference + hardcoded default) | Listener/Port-Resolution Component | `cluster.c`, `myconf.c`, `config_repl.c` |
| 2.1 | Remove 6 perf-tuning fields from schema | Schema Component | `controller.capnp` |
| 2.5 | Remove orphaned `HashZArrayConfig`/`ResmgrConfig` structs | Schema Component | `controller.capnp` |
| 2.2 | Hardcoded tunables unchanged | Tunables Component | `cluster.c` (no change) |
| 2.2 | Hardcoded tunables unchanged (as defaults) | Tunables Component | `cluster.c` (values unchanged; function itself later extended by the now-implemented Req 6.2 CLI-sourcing wrapper) |
| 2.3 | Controller stops attempting to read removed tunable fields | Tunables Component | `cluster.c` (implicit — field no longer exists) |
| 2.4 | `cluster_config_t` drops tunable members | C Struct Component | `clusterconfig.h` |
| 3.1 | Retained fields unchanged in type/number | Schema Component | `controller.capnp` (no change to retained fields) |
@ -175,7 +175,7 @@ flowchart TD
| 4.2 | No deprecate-in-place transition | Schema Component | `controller.capnp` |
| 5.1 | Build + test-fixture compile cleanly | Test Fixture Updates | `test_commit_barrier_rollout.c`, `test_config_repl.c` |
| 5.2 | No new unit test failures | Verification | controller unit test suite |
| 5.3 | Regenerate `ha/data/cluster.bin` fixture against the new schema | Verification | `ha/data/cluster.bin` |
| 5.3 | Regenerate `ha/data/cluster.bin` fixture against the new schema | Verification | `ha/data/cluster.bin` (Note: `ha/data/` was later deleted entirely by `admin-config-tool`; see the "Note" on this file in File Structure Plan above) |
| 5.4 | 3-node live soak (config-push, peer/listener/worker-liveness) | Verification | live HA cluster |
| 5.5 | Single-node + bare-peers port-override + `-EINVAL` live verification | Verification | live single-node controller |
| 6.1 | (Nice-to-have) CLI tunable flags | CLI Tunables Component (optional) | `main.c`, `myconf.h` |
@ -251,20 +251,20 @@ int cluster_open_listener(cluster_t *c);
No new error categories are introduced, and the existing `-EINVAL` error path (`cluster_open_listener`'s "no port could be determined," `cluster.c:1226` today) is **preserved**, not removed — corrected during design review, since single-code-path simplicity would have traded a clear startup error for a silent wrong-default in a likely-misconfiguration scenario (`--peers` given, local name absent). It now applies to exactly one case: `--peers` was passed and the local name isn't in it. The genuinely-new case (`--peers` never passed at all, i.e. single-node) and the pre-existing-tolerated case (name matched, that entry's port is `0`) both resolve via the hardcoded-default fallback and never error — unchanged from today's tolerance for the latter, newly-defined for the former. All other error handling (schema decode failures, capnp field-absence for legacy blobs) is capnp's existing default behavior: an old `cluster.bin` with data in now-unused field slots decodes without error, and those values are simply never read (Requirement 4.1) — no new error branch needed for that case either.
### Monitoring
No new logging beyond what already exists for `--peers` parsing (`main.c:284-291`) and controller startup (`INFO`/`WARN`/`ERROR` via the existing `mylog` macros). No new metrics.
No new logging beyond what already exists for `--peers` parsing (`main.c:411-443`, was `main.c:284-291` — shifted after this spec's own Requirement 6 CLI flags were added earlier in the file) and controller startup (`INFO`/`WARN`/`ERROR` via the existing `mylog` macros). No new metrics.
## Testing Strategy
- **Unit**: build-time enforcement is the primary "test" for Requirements 1.1-1.2, 1.5-1.6, 2.1, 2.4 — a removed field/struct member that's still referenced anywhere is a compile error, not a silent gap (this is why Requirement 5.1's test-fixture fix is explicit rather than assumed).
- **Unit**: `controller_unit_test` full suite run (Requirement 5.2) — existing coverage for `cluster_build_config`, `init_controller_tunables`, and the two touched test files, now compiling against the shrunk struct.
- **Integration**: regenerate the `ha/data/cluster.bin` fixture (Requirement 3.2, 5.3) — since retained-field renumbering breaks binary compatibility with any pre-spec blob, decode the current fixture's semantic field values under the pre-spec schema and re-encode them under the new schema with a small throwaway tool; confirm the regenerated blob decodes correctly under the post-spec schema and `deploy-ha.sh`'s verbatim-copy usage keeps working unmodified.
- **Integration**: regenerate the `ha/data/cluster.bin` fixture (Requirement 3.2, 5.3) — since retained-field renumbering breaks binary compatibility with any pre-spec blob, decode the current fixture's semantic field values under the pre-spec schema and re-encode them under the new schema with a small throwaway tool; confirm the regenerated blob decodes correctly under the post-spec schema and `deploy-ha.sh`'s verbatim-copy usage keeps working unmodified. (Note: superseded by `admin-config-tool` — `deploy-ha.sh` no longer does verbatim-copy of a fixture at all; it builds `cluster.bin` fresh via `xceadmin config build` from YAML on every run. See that spec's design.md.)
- **Live/E2E**: 3-node HA cluster deploy + config-push cycle (Requirement 5.4) — using the regenerated fixture; peer discovery via `--peers`, listener binding, worker-liveness tracking all behave identically to pre-spec.
- **Live/E2E**: single-node controller startup with no `--peers` flag, AND with a single self-referencing `--peers` entry for a custom port (Requirement 5.5) — the actual regression risk this spec's design resolves; must be verified live, not inferred from the 3-node soak (single-node never exercises the `--peers`-match branch at all).
- **Live/E2E (optional, Requirement 6)**: if the nice-to-have CLI tunable flags are implemented, verify each flag overrides its corresponding hardcoded default and that omitting all of them reproduces today's exact hardcoded values.
## Migration Strategy
No data migration is required for the fields this spec targets — Requirement 4.2 explicitly rules out a deprecate-in-place transition, and Requirement 4.1 confirms retained-field renumbering is an accepted, unguarded compatibility break for this pre-1.0 product: a `cluster.bin` built before this spec is not expected to be usable as-is against a controller built after it, and no backward-read path is supported or verified for such an artifact. The one thing this spec DOES migrate is the checked-in `ha/data/cluster.bin` test fixture (Requirement 3.2, 5.3): its retained-field values are decoded under the pre-spec schema and re-encoded under the new schema with a small throwaway tool, so `/xce-deploy-ha` and dependent live-verification tooling keep working. There is no forward path either (a post-spec `cluster.bin` is not expected to be readable by a pre-spec controller, but that direction isn't a supported downgrade path in this project regardless of this spec).
No data migration is required for the fields this spec targets — Requirement 4.2 explicitly rules out a deprecate-in-place transition, and Requirement 4.1 confirms retained-field renumbering is an accepted, unguarded compatibility break for this pre-1.0 product: a `cluster.bin` built before this spec is not expected to be usable as-is against a controller built after it, and no backward-read path is supported or verified for such an artifact. The one thing this spec DOES migrate is the checked-in `ha/data/cluster.bin` test fixture (Requirement 3.2, 5.3): its retained-field values are decoded under the pre-spec schema and re-encoded under the new schema with a small throwaway tool, so `/xce-deploy-ha` and dependent live-verification tooling keep working. (Note: superseded by `admin-config-tool`, landed 2026-07-22 — `ha/data/cluster.bin` was deleted entirely and `/xce-deploy-ha` now generates `cluster.bin` fresh via `xceadmin config build` from YAML, so this migrated fixture no longer exists or is consumed. See that spec's design.md.) There is no forward path either (a post-spec `cluster.bin` is not expected to be readable by a pre-spec controller, but that direction isn't a supported downgrade path in this project regardless of this spec).
## Supporting References
- `src/docs/cluster-config-split-analysis.md` — prior team analysis; primary design input for the dead-field classification.