docs(specs): gap-analysis validation pass across all 9 HA-family specs
Appended "Gap Analysis — Validation Pass (2026-06-09)" to each spec's research.md with requirement-to-asset maps, status tags, and effort/risk. Key outcomes: - ha-repl-hardening: REOPENED with task series 7.1-7.7 — replication payload data plane is missing (snapshot chunks and WAL catch-up ship header-only; standbys converge on LSN without holding entity state) - worker-join-refactor, resval-id-decoupling: marked implemented (all criteria re-verified, full test suite green) - buglog: 335/336 (scheduler dequeue, open), 337/338/339 (fixed in previous commit); cerebrum Decision Log corrected (term-only vote deny was recorded as intentional but was a regression)
This commit is contained in:
parent
9821ec4f3c
commit
cdd446fc17
21 changed files with 3955 additions and 288 deletions
|
|
@ -166,3 +166,77 @@ Ruled out: does not fix the design — just moves the source of truth without el
|
|||
- **Task approval gap**: spec.json shows `tasks.approved: false`. All tasks are implemented but the tasks document was never formally approved. This is a process artifact — the implementation matches the approved design.
|
||||
- **Cross-spec impact**: `scheduler-controller-msg-gap` extends the same scheduler handlers (`sched_init_sharedInst`, `make_sched_worker`) that were modified by this spec. The two specs are consistent — both use name-based resource resolution.
|
||||
- **Recommendation**: Mark tasks as approved in spec.json, or run `/kiro-spec-status resval-id-decoupling` to sync.
|
||||
|
||||
---
|
||||
|
||||
# Gap Analysis — Validation Pass (2026-06-09)
|
||||
|
||||
## Method
|
||||
|
||||
Independent re-verification of every requirement (requirements.md R1–R6) against the current code on branch `persistent_ha`, via grep + direct read of the controller, scheduler, libxce, capnp schema, and Go API-server sources. Prior audit (2026-06-08) claimed ~100% completion; this pass confirms that claim and adds two residual findings it missed.
|
||||
|
||||
## Current State Summary
|
||||
|
||||
- The feature is **implemented end-to-end**. `collect_resval_with_idx` and `free_resval_with_idx_list` no longer exist anywhere in `src/` (zero grep matches). The replacement `collect_resval(var resmgr, zarray_t*, xlist_t*)` lives at `src/controller/dataex.c:23` and iterates the resmgr (names) rather than the zarray (slots), exactly as designed.
|
||||
- Wire format: `src/schema/controller.capnp` — `Worker.resvals @5` (line 138) and `Conf4Sched.resvals @6` (line 239) are both `List(NameValue)`. Field ordinals preserved.
|
||||
- All tasks in tasks.md are `[x]` (1.1–8.3), but `spec.json` still shows `tasks.approved: false` (phase: `tasks-generated`). Implementation landed despite the unapproved tasks document — a process artifact, not a code gap.
|
||||
|
||||
## Requirement-to-Asset Map
|
||||
|
||||
| Req | EARS | Asset(s) | Status |
|
||||
|-----|------|----------|--------|
|
||||
| R1 — name-keyed shared resvals in initial config | R1-1 | `controller.capnp:239` `List(NameValue)`; `src/controller/dataex.c:381` `collect_resval(c->resmgr, &c->sharedinst, l)`; `src/controller/sched.c:120` casts to `xce_name_val_t**` | **Implemented** |
|
||||
| | R1-2 | `src/scheduler/src/scheduler.c:574-599` `sched_init_sharedInst`: `key_find(sched->resmgr, nv->name)` → `zarray_put(&sched->sharedInst, r->idx, val)` | **Implemented** |
|
||||
| | R1-3 | `scheduler.c:587-592`: WARN + `continue` on unknown name; NULL name/entry also skipped | **Implemented** |
|
||||
| | R1-4 | Same name→`r->idx` resolution as Path C handler (`scheduler.c:782-797`) | **Implemented** (by inspection; no equivalence test exists — see R6) |
|
||||
| R2 — name-keyed per-worker resvals to scheduler | R2-1 | `controller.capnp:138` `List(NameValue)`; `dataex.c:151-166` `collect_resvals_workers(resmgr,...)`; `sched.c:442` | **Implemented** |
|
||||
| | R2-2 | `src/scheduler/src/schedobj.c:274-292` `make_sched_worker`: name lookup → `zarray_put(&worker->resvals, r->idx, val)` | **Implemented** |
|
||||
| | R2-3 | `schedobj.c:283-288`: WARN + skip, worker construction continues | **Implemented** |
|
||||
| | R2-4 | Resolution happens at each receipt against the live `sched->resmgr`; no index is cached across messages | **Implemented** (by design; no automated test for resource add/remove mid-run) |
|
||||
| R3 — name-keyed worker resvals to API server | R3-1 | `src/controller/apiserver.c:367-369`: same `collect_resval` + `xce_name_val_t**` path as the scheduler push | **Implemented** |
|
||||
| | R3-2 | `src/frontend/apiserver/backend/convert.go:100-105` iterates `resvals_to_sched`, `convert.go:283` `cNameValToWorkerResval` (name-keyed, nil-checked); `convert_pure.go:210`; `models/hosts.go:94` `ResourceValues` | **Implemented** |
|
||||
| R4 — Path C unchanged | R4-1 | `src/scheduler/src/handler.c:82-103` (`Data_data_resvals` → `XCE_SCHED_OP_UPDATE_RESVALS_FULL`) and `scheduler.c:782-800` untouched, still `xce_resval_t` name-keyed | **Implemented** |
|
||||
| | R4-2 | `xce_conf4sched_t.resvals` (`typemap.h:62`) and `xce_worker_t.resvals_to_sched` (`types.h:219`) are `xce_name_val_t**`; codec auto-generated; `free_name_val_list` in `src/lib/utils.c:388` / `utils.h:130`; all controller free sites converted | **Implemented** |
|
||||
| R5 — remove index-keyed call sites | R5-1 | Zero cross-subsystem index-keyed transfers remain (grep-verified); both receivers resolve by name | **Implemented** |
|
||||
| | R5-2 | `xce_val_with_idx_t` typedef (`src/include/xce/types.h:38-41`) and capnp `struct ValueWithIdx` (`controller.capnp:176`) still exist with **zero users and no scoping comment** | **Partial** (SHOULD-level: dead type lingers, contrary to "removed or restricted ... with a comment") |
|
||||
| R6 — no regression / correctness under divergent indices | R6-1 | Correct by code inspection (name resolution is order-independent); **no automated test** registers resources in different orders and asserts `sharedInst` contents | **Partial** (code Implemented, test Missing) |
|
||||
| | R6-2 | Existing controller/scheduler suites passed at task 8.2; same-order behavior preserved | **Implemented** |
|
||||
| | R6-3 | `calc_wanted_*` paths read `sharedInst`/`worker->resvals` via scheduler-side `r->idx` (scheduler testsuite exercises these at zarray level) | **Implemented** |
|
||||
|
||||
### Adjacent finding (Constraint — out of spec scope, same defect class)
|
||||
|
||||
`XCE_SCHED_OP_UPDATE_RESVALS` (per-host running update, `handler.c:231-251` → `scheduler.c:801-816`) receives name-keyed `xce_resval_t` records but stores them with a **hardcoded slot**: `zarray_put(&w->resvals, 0, rv->val)` — it finds the worker by `rv->host` but never resolves `rv->name` to `r->idx`. Any per-host update for a resource whose scheduler idx ≠ 0 lands in the wrong slot (or clobbers slot 0). This path was not enumerated in this spec (Paths A/B/C only) and belongs to the `scheduler-controller-msg-gap` surface, but it violates the spirit of EARS-R5-1. Flagged for follow-up, not a regression of this feature.
|
||||
|
||||
## Implementation Approach Options (for residual gaps)
|
||||
|
||||
### Option A — Minimal cleanup within this spec (Recommended)
|
||||
Delete the dead `xce_val_with_idx_t` typedef and capnp `ValueWithIdx` struct (or add the scoping comment EARS-R5-2 asks for); add one scheduler testsuite case that feeds `sched_init_sharedInst`/`make_sched_worker` a name-val list against a resmgr populated in a different order and asserts slot contents (closes R6-1 test gap).
|
||||
- ✅ Closes both Partial items with ~2 files + 1 test; no wire change (struct ordinals of NameValue untouched)
|
||||
- ❌ Touches a shared header (`types.h`) — full rebuild needed to confirm nothing else references the type
|
||||
|
||||
### Option B — Fold residuals into `scheduler-controller-msg-gap`
|
||||
Fix the `zarray_put(..., 0, ...)` slot bug together with the R5-2/R6-1 residuals in the adjacent spec, since it already owns the per-host update handlers.
|
||||
- ✅ Single coherent change set for all per-host resval handling; this spec can be closed as-is
|
||||
- ❌ R5-2/R6-1 residue stays attached to the wrong spec's history; risk of being dropped if that spec stalls
|
||||
|
||||
### Option C — Accept and close
|
||||
Mark tasks approved, record R5-2 (SHOULD) and the missing R6-1 test as known accepted debt.
|
||||
- ✅ Zero effort; all MUST-level acceptance criteria are demonstrably met
|
||||
- ❌ Dead type invites accidental reuse; the divergent-order scenario — the entire point of the feature — remains untested by automation
|
||||
|
||||
## Effort & Risk
|
||||
|
||||
- **Effort: S** — residual work is a dead-type deletion plus one table-driven C test; the feature itself requires no further code.
|
||||
- **Risk: Low** — all changes verified in place and grep-confirmed; the only Medium-risk item is the adjacent slot-0 bug, which is pre-existing and outside this spec.
|
||||
|
||||
## Recommendations
|
||||
|
||||
1. Approve tasks in `spec.json` (`tasks.approved: true`) to close the process gap — implementation matches the approved design.
|
||||
2. Take Option A: remove (or comment-scope) `xce_val_with_idx_t` / `ValueWithIdx`, and add the divergent-registration-order test for `sched_init_sharedInst` / `make_sched_worker` (TDD: the test also documents EARS-R6-1 permanently).
|
||||
3. File the `XCE_SCHED_OP_UPDATE_RESVALS` slot-0 defect against `scheduler-controller-msg-gap` (or a standalone fix) — it is the last index-style hazard in the resval surface.
|
||||
|
||||
## Research Needed
|
||||
|
||||
- **RN-1**: Confirm `XCE_SCHED_OP_UPDATE_RESVALS` (`scheduler.c:814`) is a live path (who sends `schedUpdateResvals` capnp field @8, and with which resources) before fixing the hardcoded slot 0 — it may be stub/dead code.
|
||||
- **RN-2**: Verify no out-of-tree or generated consumer (e.g., `src/build/lib/controller.capnp.c`) still references `ValueWithIdx` codecs before deleting the schema struct; a clean `ninja -C src/build` after removal settles this.
|
||||
- **RN-3**: EARS-R2-4 (resource add/remove mid-run) has no test harness today; determine whether the scheduler testsuite can drive `sched_handle_resources` followed by a worker push without a live controller.
|
||||
|
|
|
|||
|
|
@ -1,9 +1,9 @@
|
|||
{
|
||||
"feature_name": "resval-id-decoupling",
|
||||
"description": "investigate a better solution to pass resource val from controller to scheduler/api server, the current solution has a very strong assumption that the idx/id of resource in controller and scheduler should keep the same, but this assumption may be too strong, the key function is collect_resval_with_idx() in src/controller/dataex.c",
|
||||
"phase": "tasks-generated",
|
||||
"phase": "implemented",
|
||||
"created_at": "2026-05-12T06:52:00-07:00",
|
||||
"updated_at": "2026-05-12T07:45:00-07:00",
|
||||
"updated_at": "2026-06-09T17:45:00Z",
|
||||
"language": "en",
|
||||
"status": "active",
|
||||
"approvals": {
|
||||
|
|
@ -17,7 +17,7 @@
|
|||
},
|
||||
"tasks": {
|
||||
"generated": true,
|
||||
"approved": false
|
||||
"approved": true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue