docs(scheduler-core): reconcile stale doc references after today's fixes
design.md's own "Open Questions/Risks" list (items 5 and 6) still described the OPCode_schedAdmin admission gap and the tlog activation gap as open/ deferred, even though both were fixed earlier in this session -- marked FIXED with pointers to the actual change. research.md's schedAdmin decision record and its Risks & Mitigations bullet got the same treatment (SUPERSEDES note, historical record preserved rather than rewritten). tasks.md's 2026-08-07 GO/NO-GO follow-up list gained a status-update line: follow-up 1 (tlog) is now fixed, follow-up 2 (fcfs.c/builtin_fcfs.c traceability) is only partially closed (design.md text was already correct; tasks 2.1-2.6 still lack _Verified: lines -- flagged, not fixed here since I have no fresh verification evidence for those tasks). roadmap.md's scheduler-core summary line corrected to attribute each gap to the right source (2026-08-07 validate-impl vs today's separate design-review pass) instead of conflating them.
This commit is contained in:
parent
9f7e656b2f
commit
0519d24d64
4 changed files with 6 additions and 4 deletions
|
|
@ -789,6 +789,6 @@ _Feature-specific targets only; general practices are in steering docs._
|
|||
2. **`PreemptDecision`'s wire shape is pre-committed ahead of `scheduler-preemption`'s own design** (P1). Flagged as a revalidation trigger in Boundary Commitments; `scheduler-preemption`'s design should explicitly confirm or extend this shape early, not discover it mid-implementation.
|
||||
3. **`SnapshotData.internalState` opaque-blob scope** (P2). If a future spec needs `xceadmin scheduler dump` to render shared-resource-tracking or unsatisfiable-resreq-cache contents meaningfully (not just byte length), `internalState` will need to become a structured capnp field instead of opaque `Data` — deferred here because no current requirement asks for it (AC 20's dump description only requires human-readable output for what dump already needs to show, and internals were never asked to be inspectable).
|
||||
4. **`xce_sched_core_t`'s `config`/`plugins`/`jobs`/`queues`/`workers`/`users`/`projects` field block is left as `var ...` in requirements.md's data-structure summary** (not fully typed) — Phase 2 implementation must finalize these as concrete `kvmgr`/pointer types when `core.c` is written; this design does not re-type them since requirements.md already commits to "moved from `xce_scheduler_t`" as the authoritative source of truth for their shapes.
|
||||
5. **`OPCode_schedAdmin` blocks a vtask handler thread up to 5s per request with no admission reservation** (P2 — accepted boundary case, deferred). Project owner decision (2026-08-02): `xceadmin scheduler` commands are cluster-administrator-triggered and low-frequency, not expected to burst concurrently — no `COMMIT_WAIT_RESERVED`-style admission gate added. Revisit only if usage patterns become automated/high-frequency. See `SchedAdminOpcode` component and `research.md`.
|
||||
6. **Req 5 AC 3's tlog virtual-clock logging integration is built but never activated** (P2 — found by `/kiro-validate-impl`, 2026-08-07, not yet fixed). `core_clock.c`'s `xce_core_register_tlog_format()`/`xce_core_set_active_for_logging()` are unit-tested (`test_clock.c`) but have zero call sites in `main.c`/`sim.c`/`driver.c` — task 4.5 deliberately deferred wiring them to "a later task" (its own header comment), and no later task picked it up. The correct activation site is `sim.c`'s `sched_sim_driver_start()` (after `sim_set_virtual_clock()` seeds `virtual_clock_base` to a real epoch value from the `.replay` file) — NOT `main.c`'s normal-mode path, where `virtual_clock_base=0` would make `xce_core_now()` return raw `CLOCK_MONOTONIC` boot-relative nanoseconds, making log timestamps worse, not better. Left unfixed pending a paired deactivation: `g_active_core_for_logging` is one process-wide pointer, `sched_sim_driver_run()` has no fini function (its own comment: "no sim-driver stop/fini function exists") and multiple early-return paths, and several test binaries construct more than one stack-allocated `xce_sched_sim_driver_t` per process — activating without deactivating risks a real ASAN stack-use-after-scope hazard. See tasks.md's Implementation Notes for the full trace.
|
||||
5. ~~**`OPCode_schedAdmin` blocks a vtask handler thread up to 5s per request with no admission reservation**~~ **FIXED 2026-08-19** (design-review follow-up). Now gated by `vtaskmgr_blocking_acquire`/`_release` against the same `c->vtaskmgr` reservation `commit.c`'s commit-wait barrier shares; admission-exhausted requests fast-reject with `{ok=false, error="server busy, try again"}` instead of blocking. See `SchedAdminOpcode` component above and `research.md`'s "SUPERSEDES" note.
|
||||
6. ~~**Req 5 AC 3's tlog virtual-clock logging integration is built but never activated**~~ **FIXED 2026-08-19** (design-review follow-up). `sched_sim_driver_start()` (`sim.c`) now calls `xce_core_register_tlog_format()`/`xce_core_set_active_for_logging(&sim->core)` right after `sim_set_virtual_clock()`, exactly at the site this entry identified; `sched_sim_driver_run()` (`sim_loop.c`) deactivates (`xce_core_set_active_for_logging(NULL)`) on all 7 of its return paths to avoid the ASAN stack-use-after-scope hazard this entry flagged — confirmed by fixing a real instance of exactly that hazard, caught live when the new test was added (bug-1598, `test_sim_event_loop.c`'s fixtures switched to `static` storage). No sim-driver fini function was added; the per-return-path deactivation is the shipped mitigation instead.
|
||||
7. **`plugins/fcfs.c` vs `plugins/builtin_fcfs.c` split was undocumented in this design until 2026-08-07** (P3 — documentation gap only, now fixed here). Found by `/kiro-validate-impl`: the file-structure and BuiltinFcfsPicker sections above originally said `fcfs.c` would be rewritten; the actual implementation added a new `builtin_fcfs.c` and deliberately left `fcfs.c` untouched (task 1.2's compat-wrapper exercise target). Functionally correct and confirmed live (`builtin_fcfs` is the registered default), just wasn't reflected here until this update. Tasks 2.1-2.6 in tasks.md still carry no `_Verified:` annotations (unlike every other task group) — worth backfilling for traceability.
|
||||
|
|
|
|||
|
|
@ -84,10 +84,11 @@
|
|||
- **Rationale**: The `COMMIT_WAIT_RESERVED` precedent exists because commit-barrier waits are on the hot path of every job submission (high, uncontrolled frequency). Admin recording control is operator-initiated and rare by nature; the risk this pattern protects against in the commit-barrier case doesn't apply at comparable severity here.
|
||||
- **Trade-offs**: If usage patterns change (e.g. automated/scripted recording control invoked at scale, or a monitoring tool polling `record status` frequently), this becomes a live risk without warning — no test or admission gate would catch it.
|
||||
- **Follow-up**: If `scheduler record`/`dump` usage ever becomes automated or high-frequency, revisit with a `COMMIT_WAIT_RESERVED`-style admission reservation.
|
||||
- **SUPERSEDES (2026-08-19, design-review follow-up)**: Option 3 above is no longer the shipped behavior. A retrospective design review flagged that `c->sched.lock` already serializes concurrent `schedAdmin` requests one at a time — a burst of N concurrent requests could occupy N vtask threads for up to N×5s each even under Option 3's "accepted, human-paced" framing, which this Decision's own Trade-offs bullet didn't fully account for. `vtaskmgr_blocking_acquire`/`vtaskmgr_blocking_release` (Option 1's mechanism, reusing `commit.c`'s existing `c->vtaskmgr` reservation rather than a new one) now wrap the whole forward+wait region; admission-exhausted requests fast-reject with `{ok=false, error="server busy, try again"}` instead of queuing. See `design.md`'s `SchedAdminOpcode` component ("Blocking-wait admission (hardened 2026-08-19)") and `requirements.md` Req 5 AC 10's own Amendment for the current, authoritative description.
|
||||
|
||||
## Risks & Mitigations
|
||||
- **`OPCode_schedAdmin` ships without an RBAC/auth check beyond the primary/standby gate — accepted, not blocking.** Mitigation: documented explicitly in `design.md`; tracked as a future roadmap item (see Decision above), not a Phase 3 dependency.
|
||||
- **`OPCode_schedAdmin` blocks a vtask thread for up to 5s per request with no admission reservation — accepted boundary case, not mitigated.** Rationale: low-frequency, human-triggered request class (see Decision above). Revisit only if usage patterns change.
|
||||
- **`OPCode_schedAdmin` blocks a vtask thread for up to 5s per request — MITIGATED 2026-08-19.** Now gated by the same `vtaskmgr_blocking_acquire`/`_release` admission reservation `commit.c`'s commit-wait barrier shares (see the Decision's "SUPERSEDES" note above); admission-exhausted bursts fast-reject rather than occupying vtask threads.
|
||||
- **`PreemptDecision` wire shape is pre-committed before `scheduler-preemption`'s own design exists** — mitigation: documented as a revalidation trigger; `scheduler-preemption` must review this shape early rather than discovering it mid-implementation.
|
||||
- **`jbuf` capacity=1 determinism invariant depends on a `FIXME: make this configurable` comment never being resolved carelessly** — mitigation: already hard-pinned in requirements.md Req 5 AC 7 with an explicit Phase 2 task; design.md's File Structure Plan marks `xce_sched_core_init()` as the single place this is enforced.
|
||||
|
||||
|
|
|
|||
|
|
@ -428,3 +428,4 @@
|
|||
1. **Req 5 AC 3's tlog logging-integration bullet is built but never activated.** `core_clock.c`'s `xce_core_register_tlog_format()`/`xce_core_set_active_for_logging()` (task 4.5) are unit-tested but have zero call sites in `main.c`/`sim.c`/`driver.c` — task 4.5's own header comment explicitly deferred this ("this task does not wire it into mylog_init()/main.c; that remains a later task's job"), but no later task in this spec ever picked it up. Root-caused where it *should* wire in: NOT `main.c`'s normal-mode path — normal mode seeds `virtual_clock_base=0, real_clock_base=0` (`core.c:139-140`), so `xce_core_now()` there returns raw `CLOCK_MONOTONIC` nanoseconds-since-boot, not epoch time; activating it for normal-mode logging would make log timestamps *worse*, not better. The correct site is `sim.c`'s `sched_sim_driver_start()`, right after `sim_set_virtual_clock()` (line 394) — `virtual_clock_base` is seeded to `snapshot_ts` there (a real epoch-ns value from the `.replay` file), so `xce_core_now()` becomes a meaningful simulated timestamp. Left unfixed here rather than patched under validation-gate time pressure because activation needs a matching deactivation: `g_active_core_for_logging` is a single process-wide pointer, `sched_sim_driver_run()`'s own header comment already documents "no sim-driver stop/fini function exists" (deliberate, accepted gap) with multiple early-return paths, and several test binaries (e.g. `test_repl_equivalence.c`) construct more than one stack-allocated `xce_sched_sim_driver_t` per process — activating without a paired deactivation risks a real ASAN stack-use-after-scope hazard the very next time an unrelated log call fires in that process. Needs a small dedicated follow-up task: wire activation into `sched_sim_driver_start()` AND deactivation into every `sched_sim_driver_run()` return path (or add the fini function its own comment says doesn't exist yet).
|
||||
2. **`plugins/fcfs.c` vs `plugins/builtin_fcfs.c` — functionally correct, undocumented.** design.md (component description) says `fcfs.c` is "rewritten (Phase 1) to `xce_job_picker_iface_t`, drops `job_elect_fn`." The actual implementation instead added a new parallel file, `plugins/builtin_fcfs.c`, and left `fcfs.c` untouched still registering the legacy `job_elect_fn` (`fcfs.c:353`) — confirmed deliberate, not an oversight: `scheduler.c:466`'s own comment says the new picker is registered "alongside the untouched legacy `xce_sched_fcfs`", and task 1.2's compatibility-wrapper requirement needs exactly this kind of legacy `job_elect_fn`-only plugin to exist as its live exercise target. `builtin_fcfs` is confirmed the registered default (`schedobj.c:364`). Functionally sound; the only gap is that tasks 2.1-2.6 carry no `_Verified:` annotations (unlike every other task group) and design.md's literal text was never updated to record this file-layout decision — a documentation/traceability gap, not a defect. Worth a follow-up: add `_Verified:` notes to 2.1-2.6 and correct design.md's component description.
|
||||
- Both items are cosmetic/traceability, not correctness or determinism defects — the soak, equivalence, and integration evidence for this validation pass (15-round sequential + 15-round chaotic HA soak, both zero-loss/zero-divergence; full rebuild 41/41 meson + 15/15 manual scheduler + all Go/xceadmin tests green; independent requirements-coverage/design-alignment/cross-task-integration subagent passes) is unaffected by either.
|
||||
- **Status update (2026-08-19)**: Follow-up 1 (tlog activation) is now **FIXED** — see design.md's Open Questions/Risks #6. Follow-up 2 is **partially closed**: design.md's component description was corrected the same day (2026-08-07, see design.md's own Open Questions/Risks #7), but tasks 2.1-2.6 still carry no `_Verified:` annotations — that backfill remains open.
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ XCE is in the persistence + HA transformation (branch `persistent_ha`). Governin
|
|||
|
||||
## In Flight
|
||||
|
||||
- ~~**scheduler-framework** — NEW 2026-07-06 (`/kiro-discovery`). Iterator-driven allocation architecture: lazy plan iterator + host iterator, combinatorial parallel job allocation, configurable job picker, session-level unsatisfiable resreq cache. Brief: `.kiro/specs/scheduler-framework/brief.md`.~~ **DECOMPOSED 2026-07-27** into three child specs (`.kiro/specs/scheduler-framework/spec.json` `child_specs`): **scheduler-core** — core/driver separation, pluggable job picker interface, and simulation mode; **IMPLEMENTATION-COMPLETE**, GO/NO-GO validation 2026-08-07/08 (2 non-blocking follow-ups), 45/45 sub-tasks done. **scheduler-allocation** — lazy iterator-driven allocation architecture, parallel job support, session-level unsatisfiable resreq cache; requirements generated, `blocked_on: [scheduler-core]` — now unblocked. **scheduler-preemption** — SIGSTP-based victim selection + controller→worker pipeline; requirements generated, `blocked_on: [scheduler-core]` — now unblocked. The parent `scheduler-framework` spec is retained only for discovery-context provenance and is no longer in flight itself. Next: `/kiro-spec-design scheduler-allocation` and `/kiro-spec-design scheduler-preemption`.
|
||||
- ~~**scheduler-framework** — NEW 2026-07-06 (`/kiro-discovery`). Iterator-driven allocation architecture: lazy plan iterator + host iterator, combinatorial parallel job allocation, configurable job picker, session-level unsatisfiable resreq cache. Brief: `.kiro/specs/scheduler-framework/brief.md`.~~ **DECOMPOSED 2026-07-27** into three child specs (`.kiro/specs/scheduler-framework/spec.json` `child_specs`): **scheduler-core** — core/driver separation, pluggable job picker interface, and simulation mode; **IMPLEMENTATION-COMPLETE**, GO/NO-GO validation 2026-08-07/08 (2 non-blocking follow-ups: (a) tlog logging-integration built but never activated, (b) `fcfs.c`/`builtin_fcfs.c` doc+traceability gap — design.md text was corrected same-day, but tasks 2.1-2.6 in `tasks.md` still carry no `_Verified:` lines, **still open**), 45/45 sub-tasks done as of that validation. **Updated 2026-08-19**: task 12.5 (REPL live-operator-testing fixes, Req 5.22 amendment) was added after that validation from live-operator testing gaps (bug-1593/1594/1595/1596) and is now also done, independently re-verified with fresh evidence (`/kiro-verify-completion`) — see `tasks.md`'s own `_Verified:` line on that task; this closes follow-up (a) too (the tlog gap is now activated — see `design.md`'s `SchedulerCore` component and `research.md`). A separate retrospective design review (2026-08-19, unrelated to the original 2026-08-07 follow-ups) additionally found and fixed an `OPCode_schedAdmin` blocking-wait admission gap and extended `core.c`'s isolation-boundary test coverage to its `core_events.c`/`core_session.c`/`core_clock.c` siblings — see `research.md`'s "SUPERSEDES" notes. **scheduler-allocation** — lazy iterator-driven allocation architecture, parallel job support, session-level unsatisfiable resreq cache; requirements generated, `blocked_on: [scheduler-core]` — now unblocked. **scheduler-preemption** — SIGSTP-based victim selection + controller→worker pipeline; requirements generated, `blocked_on: [scheduler-core]` — now unblocked. The parent `scheduler-framework` spec is retained only for discovery-context provenance and is no longer in flight itself. Next: `/kiro-spec-design scheduler-allocation` and `/kiro-spec-design scheduler-preemption`.
|
||||
- ~~**ha-consistency-invariants** — UMBRELLA CONTRACT (requirements + design + research approved/complete). Implementation decomposed 2026-06-11 into the four sub-specs below; the umbrella generates NO tasks itself. Each sub-spec brief references the umbrella docs as authoritative.~~ **COMPLETE 2026-07-13** — all four sub-specs (ha-commit-engine, ha-commit-barrier-rollout, ha-single-leader-role, ha-convergence-recovery) implemented; Criterion 2.9 (Facts-Apply invariant) satisfied via raft-apply-decoupling. No longer in flight.
|
||||
- ~~ha-wal-replication — implemented except task 5.4 (live 3-node integration test; needs `/xce-deploy-ha` cluster); sub-spec 4's E2E extends it~~ **task 5.4 CLOSED 2026-07-18** via `/kiro-validate-impl`'s live 10-round sequential soak (972/972 tests, zero loss). Spec is GO, no longer in flight.
|
||||
- ~~**libxce-failover** — CLIENT-SIDE transparent controller failover (requirements generated 2026-06-16). Wires all libxce production request paths through the (already-built-but-orphaned) `xce_request_with_retry()` primitive + adds multi-master rotation; classifies on the real controller status taxonomy `E_XCE_REDIRECT` / `E_XCE_NOMASTER` / `E_XCE_NOT_READY` / `E_XCE_STANDBY` (`errnox.h:22-25`); bounded retry → distinct cluster-not-ready (no more opaque EINVAL); includes one small controller change so redirect replies carry the primary host/port (schema fields already exist, `controller.capnp:598-599`, just unpopulated in `join.c`). Verified E2E via `xce_test` (the original reproduction). Closes the integration gap left by `ha-config-bootstrap` Req 7 (tasks 10/10.1/10.2 shipped only the standalone primitive). See bug-608. Dependencies: ha-config-bootstrap (primitive + error codes), ha-commit-barrier-rollout (write idempotency makes write-retry safe).~~ **IMPLEMENTED, validate-impl GO 2026-07-18** (all 19 tasks were done since 2026-06-18; the `ready_for_implementation` flag was just never flipped — administrative staleness, not a real block). No longer in flight. `component-failover-adoption` (backlog, below) is the next layer on top of this.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue