docs(scheduler-sim-decision-parity): finalize design and generate tasks
Design went through 3 review rounds (fixed 6 real issues: verify-loop positional alignment, snapshot-restored-job ambiguity, a leak, a double-free/UAF from move-semantics aliasing, a const-qualified interface contradicting its own mutation requirement, and a file-location error in the accounting-function relocation plan). Tasks generated and passed an independent sanity review after one repair round (missing call-site update, missing (P) boundary tag). All phases approved, ready for implementation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
230083ae5d
commit
0b2c856d8b
5 changed files with 125 additions and 22 deletions
|
|
@ -28,7 +28,7 @@ Two components, one spec:
|
|||
## Out of Boundary
|
||||
- `scheduler-sim-repl-verbosity`'s REPL output/formatting work.
|
||||
- `scheduler-sim-diagnostics`'s future new inspection commands (limit usage, pend-reason) — this spec restores existing data's correctness, it does not add new inspectable data.
|
||||
- Whether `sched_check_host_satisfy`/`sched_meet_host_selection` read `resInUse` for live capacity decisions — worth confirming during design (it affects severity/priority framing) but is existing scheduler-core behavior, not something this spec changes.
|
||||
- Whether `sched_check_host_satisfy`/`sched_meet_host_selection` read `resInUse` for live capacity decisions — CONFIRMED during design (see `research.md`'s Research Log): they do, unconditionally. This is existing `scheduler-core` behavior this spec does not change, only relies on.
|
||||
|
||||
## Upstream / Downstream
|
||||
- **Upstream**: `scheduler-core`'s core/driver split and its now-documented equivalence invariant (`.kiro/steering/scheduler-driver-equivalence.md`).
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@
|
|||
|
||||
**Users**: XCE scheduler engineers and operators debugging via `xce_scheduler -i -s <replay>` or batch `--sim`, and any future scheduler spec (`scheduler-allocation`, `scheduler-preemption`) relying on simulation mode reflecting real scheduling behavior.
|
||||
|
||||
**Impact**: Changes `sim_loop.c`'s round-boundary handling (replacing the monolithic `xce_core_go()` with the split calls `driver.c` already uses, plus a new reconciliation step), the signature of five accounting functions currently in `resource.c` (relocated to operate on `xce_sched_core_t*`), and possibly `sim_verify.c`'s comparison driver (pending confirmation — see Risks). No change to `core.c`/`core_events.c`'s event-application logic, to scheduling/allocation policy, or to REPL output formatting.
|
||||
**Impact**: Changes `sim_loop.c`'s round-boundary handling (replacing the monolithic `xce_core_go()` with the split calls `driver.c` already uses, plus a new reconciliation step that also compacts `decoded_session->decisions` as it consumes entries — see Recorded-Decision Reconciliation's Implementation Notes), and the signature of five accounting functions currently in `resource.c` (relocated to operate on `xce_sched_core_t*`). `sim_verify.c`'s comparison predicates (`sim_decision_eq()` and its helpers) need no change — confirmed during design validation that `sim_loop.c`'s own array bookkeeping is sufficient to keep the existing positional comparison correct. No change to `core.c`/`core_events.c`'s event-application logic, to scheduling/allocation policy, or to REPL output formatting.
|
||||
|
||||
### Goals
|
||||
- Resource-in-use accounting (`resInUse`/`sharedInUse`) reflects every currently-running job in simulation mode, matching normal mode (Requirement 1).
|
||||
|
|
@ -42,13 +42,14 @@
|
|||
- Any change to `xce_job_alloc_t`/`xce_host_alloc_t`/`xce_alloc_t`'s wire shape (`controller.capnp`) must be re-checked against the reconstruction logic here.
|
||||
- Any change to how `--sim-verify` compares decisions must be re-checked against this spec's reconciliation step, and vice versa.
|
||||
- If `scheduler-sim-diagnostics` later adds a "why did this job dispatch" trace, it can build on the now-correct `job->accounts` this spec establishes rather than re-deriving it.
|
||||
- **When `scheduler-preemption` lands** (currently requirements-generated, not yet designed — `driver.h:96` presently states "no live allocator path produces this decision type"): Requirement 3.1 here covers "dispatched, pended, or preempted" jobs, but Recorded-Decision Reconciliation's trigger condition (RUN-state + `job->accounts == NULL`) only ever matches a recorded DISPATCH entry — a preempted job leaves RUN-state at the moment of preemption, so it cannot trigger this same mechanism, and PEND doesn't need reconciliation because the live picker already re-derives it fresh every round. This is not a gap today (no code path emits `XCE_DECISION_PREEMPT` yet, so no recorded replay can contain one), but once `scheduler-preemption` starts producing real PREEMPT decisions, revisit whether this spec's mechanism needs a second trigger condition for "job left RUN-state via preemption within this round, no reconciled visibility yet" or whether `scheduler-preemption`'s own design should own that instead.
|
||||
|
||||
## Architecture
|
||||
|
||||
### Existing Architecture Analysis
|
||||
- `driver.c`'s `xce_sched_driver_run_normal_mode_round()` is the reference implementation for the exact sequence this spec ports to `sim_loop.c`: `xce_core_begin_session()` → driver-specific interleaved bookkeeping (`plugin_begin_session()`, `sched_account_jobs()`) → `xce_core_signal_go()` → decision-drain loop (`cbuf_get(&core->decisions)`, recording each via `sched_record_session_add_decision()` when recording, then `core->on_decision()`).
|
||||
- `sim_loop.c` currently calls the older monolithic `xce_core_go()` (itself now just `xce_core_begin_session()`+`xce_core_signal_go()` back-to-back, per `core_session.c`) with no interleaving room, and decodes `decoded_session->decisions` every round but only consumes it under `sim->verify`.
|
||||
- `resource.c`'s accounting functions (`sched_account_jobs()` and its callees) already only touch `sched->core` in their bodies despite their `xce_scheduler_t*` signature — confirmed by reading every function in this call chain this session.
|
||||
- The accounting functions being relocated — `sched_account_jobs()` (`scheduler.c`) and its callees `sched_charge_account()`/`sched_uncharge_account()`/`sched_update_account()`/`sched_update_res_accounts()`/`sched_commit_job_alloc()` (`resource.c`) — already only touch `sched->core` in their bodies despite their `xce_scheduler_t*` signature. Confirmed by reading every function in this call chain, and independently re-confirmed with `grep "sched->" resource.c | grep -v "sched->core"` returning zero matches across that whole file (covers the five `resource.c` functions; `sched_account_jobs()` itself was separately read in full).
|
||||
|
||||
### Architecture Pattern & Boundary Map
|
||||
|
||||
|
|
@ -58,7 +59,8 @@ flowchart TD
|
|||
Reconcile --> Lookup{Recorded decision found in decoded_session}
|
||||
Lookup -->|Yes| Rebuild[Rebuild job allocs and accounts from recorded decision]
|
||||
Lookup -->|No| Unavailable[Report usage unavailable, continue]
|
||||
Rebuild --> Report[Feed decision through existing reporting channel]
|
||||
Rebuild --> Compact[Free consumed entry, compact decoded_session.decisions]
|
||||
Compact --> Report[Feed decision through existing reporting channel]
|
||||
Report --> AccountJobs[sched_account_jobs on core]
|
||||
Unavailable --> AccountJobs
|
||||
AccountJobs --> SignalGo[xce_core_signal_go]
|
||||
|
|
@ -82,13 +84,13 @@ flowchart TD
|
|||
## File Structure Plan
|
||||
|
||||
### Modified Files
|
||||
- `src/scheduler/src/resource.c` — relocate `sched_account_jobs()`, `sched_charge_account()`, `sched_uncharge_account()`, `sched_update_account()`, `sched_update_res_accounts()`, `sched_commit_job_alloc()` from `xce_scheduler_t*` to `xce_sched_core_t*` parameters (body unchanged — every reference is already `sched->core.*`).
|
||||
- `src/scheduler/src/resource.c` — relocate `sched_charge_account()`, `sched_uncharge_account()`, `sched_update_account()`, `sched_update_res_accounts()`, `sched_commit_job_alloc()` from `xce_scheduler_t*` to `xce_sched_core_t*` parameters (body unchanged — every reference is already `sched->core.*`).
|
||||
- `src/scheduler/include/scheduler.h` — remove/update the now-relocated declarations.
|
||||
- `src/scheduler/include/core.h` — add the relocated declarations, mirroring `sched_resolve_job_picker()`'s existing placement and doc-comment precedent.
|
||||
- `src/scheduler/src/scheduler.c` — update `sched_commit_decision()`'s call to `sched_commit_job_alloc()` for the new signature (pass `&(sched->core)`); update `sched_begin_session()`'s existing call to `sched_account_jobs()` likewise.
|
||||
- `src/scheduler/src/scheduler.c` — relocate `sched_account_jobs()` itself the same way (it is defined here, not in `resource.c`; body likewise unchanged — every reference is already `sched->core.*`); update this file's own two call sites for the new signatures: `sched_commit_decision()`'s call to `sched_commit_job_alloc()` (pass `&(sched->core)`) and `sched_begin_session()`'s call to `sched_account_jobs()` likewise.
|
||||
- `src/scheduler/src/driver.c` — update `xce_sched_driver_run_normal_mode_round()`'s `sched_account_jobs(sched)` call to `sched_account_jobs(&(sched->core))`.
|
||||
- `src/scheduler/src/sim_loop.c` — replace the `xce_core_go(&(sim->core))` call with `xce_core_begin_session()` → reconciliation step (new static function(s)) → `sched_account_jobs(&(sim->core))` → `xce_core_signal_go()`; reconciled decisions feed through the same decision-reporting call the existing drain loop uses.
|
||||
- `src/scheduler/src/sim_verify.c` — adjust the round-level comparison driver (exact change pending the implementation-time read flagged in Risks) so a reconciled decision is not reported as a false mismatch.
|
||||
- `src/scheduler/src/sim_loop.c` — replace the `xce_core_go(&(sim->core))` call with `xce_core_begin_session()` → reconciliation step (new static function(s)) → `sched_account_jobs(&(sim->core))` → `xce_core_signal_go()`; reconciled decisions feed through the same decision-reporting call the existing drain loop uses, and each consumed recorded decision is removed/compacted from `decoded_session->decisions` in the same pass so the existing `--sim-verify` positional walk (`di` against `cbuf_get(&(sim->core.decisions), ...)`, unchanged) stays correctly aligned.
|
||||
- `src/scheduler/src/sim_verify.c` — **no change** (confirmed during design validation; see `sim_loop.c` entry above and the Recorded-Decision Reconciliation component's Implementation Notes).
|
||||
|
||||
### New/Modified Test Files
|
||||
- `src/scheduler/testsuite/test_driver_session_ordering.c` (or a new `test_sim_session_ordering.c` mirroring it) — proves `sim_loop.c` now upholds the same "picker never observes zeroed-but-not-yet-resummed accounting" property `driver.c`'s own test already proves.
|
||||
|
|
@ -187,18 +189,29 @@ int sched_account_jobs(xce_sched_core_t* core);
|
|||
* no matching recorded decision is left with job->accounts == NULL
|
||||
* (Requirement 4 -- reported as unavailable elsewhere, not an error
|
||||
* here) and is safe to call again next round.
|
||||
*
|
||||
* MUTATES *decoded_session: each consumed entry is freed and removed
|
||||
* (shifted down, not swap-removed -- see Implementation Notes) from
|
||||
* decisions[]/n_decisions, so decoded_session cannot be `const` here --
|
||||
* this is deliberate, not an oversight: the caller's own subsequent
|
||||
* --sim-verify positional walk over the same decoded_session depends on
|
||||
* consumed entries already being gone by the time it runs.
|
||||
*/
|
||||
void sim_reconcile_unaccounted_jobs(xce_sched_sim_driver_t* sim,
|
||||
const replay_session_t* decoded_session);
|
||||
replay_session_t* decoded_session);
|
||||
```
|
||||
- Preconditions: `sim`/`decoded_session` non-NULL (NULL is a safe no-op, matching this codebase's established "never load-bearing" REPL-hook convention).
|
||||
- Postconditions: every RUN-state job either has `job->accounts` set (reconstructed) or is left NULL (genuinely unavailable this round).
|
||||
- Invariants: never double-applies (idempotent per job, per the `job->accounts` gate); never fabricates allocation data not present in the recording.
|
||||
- Postconditions: every RUN-state job either has `job->accounts` set (reconstructed) or is left NULL (genuinely unavailable this round). `decoded_session->decisions`/`n_decisions` reflect the removal of every consumed entry (freed, not merely uncounted -- see memory-ownership note below), leaving the remaining entries in their original relative order.
|
||||
- Invariants: never double-applies (idempotent per job, per the `job->accounts` gate); never fabricates allocation data not present in the recording; never leaves a freed pointer reachable through `job->allocs` or through the remaining `decoded_session->decisions` entries.
|
||||
|
||||
**Implementation Notes**
|
||||
- Integration: exact interaction with `--sim-verify`'s comparison driver is not yet confirmed (see Risks in `research.md`) — the first implementation task must read that driver before finalizing whether reconciliation happens before or after the verify comparison for a given round.
|
||||
- Integration with `--sim-verify` (CONFIRMED during design validation, resolving the prior "read the comparison driver at implementation time" placeholder): `sim_loop.c`'s verify branch (current code: the `while ((cbuf_get(&(sim->core.decisions), &d) == 0) ...)` loop) walks `decoded_session->decisions` *positionally* via an index `di`, comparing each live-drained decision `d` against `decoded_session->decisions[di]` and advancing `di` once per live decision. A recorded decision belonging to a job this round's reconciliation step consumes (i.e., a job whose allocation/decision visibility is being restored from the recording rather than freshly re-derived by the live picker) will **never** produce a corresponding live-drained `d` — the picker never re-decides an already-resolved job. Comparing positionally without accounting for this would misalign every subsequent entry and produce false mismatches (Requirement 5.2 violation), independent of any bug in the comparison logic itself.
|
||||
- **Resolution**: the reconciliation step removes each recorded decision it consumes from `decoded_session->decisions` *before* the verify drain loop runs for that round — it already has the exact jobid-matched entry in hand from its own lookup, so this is a natural side effect of the same pass, not a second scan. This keeps the remaining (unconsumed) recorded entries positionally aligned with the live-drained stream exactly as they are today, when no reconciliation was needed for a job. No change to `sim_decision_eq()`/`sim_verify_job_alloc_eq()`/`sim_verify_job_preason_eq()` (the comparison predicates in `sim_verify.c`) is required — only `sim_loop.c`'s bookkeeping of which array entries reach that comparison.
|
||||
- **Memory ownership (must not be overlooked)**: `decoded_session->decisions`/`n_decisions` (`replay_session_t`, `typemap.h`) are a plain heap-owned array — `free_ReplaySession_ptr()` frees it via `free_ReplayDecision_list(d->n_decisions, d->decisions)`, which walks and frees exactly the first `n_decisions` entries, then frees the array itself. Consequently, simply decrementing `n_decisions` to "drop" a consumed entry (without separately freeing that entry) is **not** safe compaction — it silently leaks that entry's payload every round a reconciliation happens, since the round-end free walk will never reach it again. The reconciliation step must instead: (1) extract the data it needs (see ownership sub-point immediately below — this is NOT a plain read for a DISPATCH entry), (2) call the existing `free_ReplayDecision(entry)` + `free(entry)` (the same two calls `free_ReplayDecision_list()` already makes per-element) on the one consumed entry, then (3) compact the array by shifting the remaining entries down over the removed slot (`memmove`), and decrement `n_decisions` by one. Shift-down, not swap-with-last: swap-with-last is the usual O(1) removal trick, but it would relocate the last entry into the removed slot and change its position relative to the other remaining entries — corrupting the very relative ordering the positional `di` comparison in the point above depends on. `decoded_session` is decoded fresh once per round regardless, so this compaction is confined to the current round with no cross-round effect.
|
||||
- **Ownership hazard in step (1) above — a DISPATCH entry's `alloc` must be moved, not aliased, before step (2) frees the entry**: the established live-path precedent (`scheduler.c:1440`, `sched_commit_decision()`) is `job->allocs = decision->u.alloc;` — a pointer *takeover*, not a deep copy; `job->accounts` (`make_job_account(job->allocs)`) is then built directly on top of that same, now job-owned, memory. If reconciliation reconstructs `job->allocs` the same way from a recorded DISPATCH entry — `job->allocs = entry->body.dispatch->alloc;` (`sched_dispatch_decision_t.alloc`, an `xce_job_alloc_t*`, `typemap.h`) — that pointer is now aliased by BOTH `job->allocs` and the entry about to be freed in step (2): `free_ReplayDecision()` → `free_DispatchDecision()` → `free_JobAlloc_ptr(&d->alloc)` would free that exact `xce_job_alloc_t`, leaving `job->allocs` dangling immediately (and a double-free whenever the job's own allocation is later released through its normal lifecycle). **Required fix**: immediately after the takeover assignment, null out the source field — `entry->body.dispatch->alloc = NULL;` — before calling `free_ReplayDecision(entry)`; every `_ptr`-style free helper in this codebase (including `free_JobAlloc_ptr`) already no-ops on `NULL`, so this makes the subsequent free of the rest of the entry safe without a special case. This is DISPATCH-specific and, by construction, the *only* case this component needs to handle: the reconciliation trigger is a RUN-state job, and a job can only reach RUN-state via a recorded DISPATCH decision — a recorded PEND (or PREEMPT) entry for the same job in the same round is structurally impossible to match here, since PEND/PREEMPT are exactly the decisions that do *not* put a job into RUN-state. (Independently confirmed no `job->preason`/equivalent field exists to alias in the first place — `job->accounts`/`job->allocs` are DISPATCH-only concepts, consistent with this.) No PEND/PREEMPT ownership case needs to be designed or implemented.
|
||||
- This also fixes a latent problem that exists independently of this feature: `--sim-verify` against any real replay containing already-resolved jobs likely already misaligns positionally today (the recorded stream has entries the live stream never independently reproduces), so this correction is a net improvement to verify-mode accuracy, not merely a regression guard.
|
||||
- Validation: must be exercised by a resource-contention fixture, not only a fixture where every job fits, per `scheduler-driver-equivalence.md`.
|
||||
- Risks: a job already running at the snapshot the replay starts from has no recorded decision in this replay's own window — falls through to Requirement 4's graceful degradation unless `xce_sched_core_load_snapshot()` already restores accounts for such jobs (unconfirmed, worth a quick check before assuming Requirement 4 is the only path).
|
||||
- Snapshot-restored jobs (CONFIRMED during design validation): `xce_sched_core_load_snapshot()` (`snapshot_load.c`) restores `job->state` from snapshot data but never touches `job->accounts`/`job->allocs`. A job already RUN-state at snapshot-load time therefore always has `job->accounts == NULL` with no in-window recorded decision to reconcile from — this is unambiguously Requirement 4's graceful-degradation case, not a separate path requiring its own handling.
|
||||
|
||||
## Data Models
|
||||
|
||||
|
|
@ -230,6 +243,7 @@ No new logging framework — existing `WARN`/`TRACE` conventions apply if reconc
|
|||
- Recorded-Decision Reconciliation reconstructs `job->allocs`/`job->accounts` correctly from a synthetic recorded DISPATCH decision (Requirement 3.1, 3.2).
|
||||
- Recorded-Decision Reconciliation is a no-op (does not double-apply) for a job whose `job->accounts` is already set (Requirement 3.3).
|
||||
- Recorded-Decision Reconciliation leaves a job unaccounted, without fabricating data or erroring, when no matching recorded decision exists (Requirement 4.1, 4.2).
|
||||
- Reconciling a DISPATCH entry, then later releasing the job's allocation through its normal end-of-life path, must be ASAN-clean (no double-free, no leak) — this directly exercises the move-then-null-then-free ownership handling in the Implementation Notes above, run under the project's standard ASAN build per `tech.md`/`build-deployment.md` rather than relying on a plain functional assertion to catch it.
|
||||
|
||||
### Integration Tests
|
||||
- **Resource-contention fixture** (new, required per `scheduler-driver-equivalence.md`): multiple jobs competing for insufficient capacity, replayed in simulation mode; assert `resInUse` matches what the equivalent normal-mode recording produced at every round, and that no job is dispatched in simulation mode that normal mode would have pended (Requirement 2.1, 2.2).
|
||||
|
|
|
|||
|
|
@ -51,9 +51,10 @@
|
|||
- **Follow-up**: none.
|
||||
|
||||
## Risks & Mitigations
|
||||
- **`--sim-verify` interaction is not fully understood** — the equality helpers (`sim_verify_*_eq`) were read, but the round-level comparison *driver* that decides what counts as a match/mismatch was not. It's possible today's verify mode already treats a recorded decision with no live-drained counterpart as a mismatch (since that's exactly what "already resolved, not re-decided live" looks like) — if so, this feature's reconstruction must either run before verify's comparison in a way that avoids introducing a false mismatch, or verify's comparison needs a small adjustment to recognize an already-resolved job as an expected non-mismatch. Mitigation: read the comparison driver as the first implementation task, before writing the reconstruction logic — this determines exact integration order.
|
||||
- **Snapshot-restored jobs already running at replay start** — a job dispatched *before* the snapshot the replay begins from has no recorded decision within this replay's own stream window (the decision happened in a round the replay never processes). Mitigation: this is exactly Requirement 4's graceful-degradation case (report unavailable, don't fail) unless `xce_sched_core_load_snapshot()` already restores `job->accounts` for such jobs directly from snapshot data — worth a quick check at implementation time; not assumed either way here.
|
||||
- **Relocating `sched_account_jobs()`'s signature touches a function with production callers** — mitigated by the fact the relocation is signature-only (body unchanged, per the `sched_resolve_job_picker()` precedent already proven safe for the identical pattern) and both call sites (`driver.c`, `sim_loop.c`) are updated in the same change.
|
||||
- **`--sim-verify` interaction — RESOLVED during design validation** (see `design.md`'s Recorded-Decision Reconciliation Implementation Notes for the full mechanism). The round-level comparison driver (`sim_loop.c`'s `while (cbuf_get(&(sim->core.decisions), &d) ...)` loop) walks `decoded_session->decisions` positionally via an index `di`. Confirmed: a job this feature reconciles never produces a live-drained counterpart, so the reconciliation step must remove (properly freed, not just index-dropped — see design.md's memory-ownership note) each consumed recorded decision from `decoded_session->decisions` before the verify loop runs, keeping the remaining entries positionally aligned by shifting down (not swap-with-last, which would reorder them). `sim_verify.c`'s comparison predicates need no change.
|
||||
- **Snapshot-restored jobs already running at replay start — RESOLVED**: confirmed by reading `snapshot_load.c` that `xce_sched_core_load_snapshot()` restores `job->state` but never `job->accounts`/`job->allocs`. Such a job is therefore always `job->accounts == NULL` with no in-window recorded decision — unambiguously Requirement 4's graceful-degradation case, no separate handling needed.
|
||||
- **Relocating `sched_account_jobs()`'s signature touches a function with production callers** — mitigated by the fact the relocation is signature-only (body unchanged, per the `sched_resolve_job_picker()` precedent already proven safe for the identical pattern) and both call sites (`driver.c`, `sim_loop.c`) are updated in the same change. Independently re-confirmed during design validation: `grep "sched->" resource.c | grep -v "sched->core"` returns zero matches across the entire file — every one of the six relocated functions' bodies (not just `sched_account_jobs()` itself) genuinely touches only `sched->core`.
|
||||
- **New risk surfaced while resolving the `--sim-verify` risk above**: naively freeing a consumed DISPATCH entry to avoid the leak would double-free, because the live path (`scheduler.c:1440`, `job->allocs = decision->u.alloc;`) takes ownership of `alloc` by pointer move, not copy — reconciliation reconstructing `job->allocs` the same way would alias memory about to be freed. Mitigation: null out the source entry's `alloc` field immediately after the takeover assignment, before freeing the rest of the entry (every `_ptr`-style free helper in this codebase already no-ops on `NULL`). See design.md for the full writeup.
|
||||
|
||||
## References
|
||||
- `.wolf/buglog.json` bug-1611, bug-1613 — the originating findings.
|
||||
|
|
|
|||
|
|
@ -1,9 +1,9 @@
|
|||
{
|
||||
"feature_name": "scheduler-sim-decision-parity",
|
||||
"created_at": "2026-08-21T08:30:00Z",
|
||||
"updated_at": "2026-08-21T09:05:00Z",
|
||||
"updated_at": "2026-08-22T04:41:00Z",
|
||||
"language": "en",
|
||||
"phase": "design-generated",
|
||||
"phase": "tasks-generated",
|
||||
"approvals": {
|
||||
"requirements": {
|
||||
"generated": true,
|
||||
|
|
@ -11,12 +11,12 @@
|
|||
},
|
||||
"design": {
|
||||
"generated": true,
|
||||
"approved": false
|
||||
"approved": true
|
||||
},
|
||||
"tasks": {
|
||||
"generated": false,
|
||||
"approved": false
|
||||
"generated": true,
|
||||
"approved": true
|
||||
}
|
||||
},
|
||||
"ready_for_implementation": false
|
||||
"ready_for_implementation": true
|
||||
}
|
||||
|
|
|
|||
88
.kiro/specs/scheduler-sim-decision-parity/tasks.md
Normal file
88
.kiro/specs/scheduler-sim-decision-parity/tasks.md
Normal file
|
|
@ -0,0 +1,88 @@
|
|||
# Implementation Plan
|
||||
|
||||
- [ ] 1. Foundation: shared accounting surface and contention test data
|
||||
- [ ] 1.1 Make resource-in-use accounting callable from either driver
|
||||
- Relocate the resource re-accounting service (currently tied to the normal-mode-only driver struct) so it operates on the mode-agnostic core state shared by both drivers, mirroring the same relocation already done for the job-picker resolution service
|
||||
- Update all three existing call sites to the new signature: the normal-mode round handler's accounting call, the scheduler's own session-begin path's accounting call, and the scheduler's decision-commit path's call into the job-allocation-commit function that is part of this same relocated group
|
||||
- Body of the accounting logic itself is unchanged — this is a pure capability-relocation step, not new behavior
|
||||
- Observable completion: the full scheduler test suite still builds and passes unchanged (zero behavior change), and the simulation driver's build now has visibility to call the same accounting service the normal-mode driver already uses
|
||||
- _Requirements: 1.1, 1.2, 1.3, 1.4, 2.1, 2.2, 5.3_
|
||||
- _Boundary: Core-Level Resource Accounting_
|
||||
|
||||
- [ ] 1.2 (P) Build a resource-contention replay fixture for testing
|
||||
- Create a recorded-session test fixture with multiple jobs competing for capacity insufficient to run all of them at once (existing fixtures only ever have enough capacity for every job, which cannot exercise or prove capacity-check correctness)
|
||||
- Fixture must be usable both as a normal-mode recording source and as a simulation-mode replay input, so the same scenario can be run through both drivers and compared
|
||||
- Observable completion: a scheduling round exists in the fixture where at least one job is denied capacity that a currently-running job is consuming
|
||||
- _Requirements: 2.1, 2.2_
|
||||
- _Boundary: Scheduler test suite (contention fixture)_
|
||||
|
||||
- [ ] 2. Core: round-boundary accounting and decision reconciliation
|
||||
- [ ] 2.1 Give simulation mode the same round-boundary accounting sequence normal mode uses
|
||||
- Replace simulation mode's single combined session-start call with the same two-step sequence (reset accounting, then unblock the picker) the normal-mode driver already uses, opening an interleaving point between them
|
||||
- Call the relocated resource re-accounting service (task 1.1) in that interleaving point, in the same relative position the normal-mode driver already calls it
|
||||
- Preserve the existing error-handling behavior for a failed session start exactly as it works today
|
||||
- Interactive (step-by-step) and batch simulation runs must go through this same call site with no divergence between them
|
||||
- Observable completion: a job dispatched fresh within a simulation run (not already resolved before the replay reached it) now shows correct worker and shared resource-in-use totals immediately after the round it was dispatched in, matching what the equivalent normal-mode recording shows at the same point
|
||||
- _Requirements: 1.1, 1.2, 1.3, 1.4, 5.1, 5.3_
|
||||
- _Depends: 1.1_
|
||||
- _Boundary: Sim Driver Round-Boundary Accounting_
|
||||
|
||||
- [ ] 2.2 Detect and locate already-resolved jobs needing reconciliation
|
||||
- Identify, once per round, every currently-running job that has no known resource allocation yet (this is the signature of a job that was already resolved earlier in the original recording, before this replay run's own live decision-making ever considered it)
|
||||
- For each such job, search this round's decoded recorded-decision data for the matching originating decision by job identity
|
||||
- When no matching recorded decision is found, leave the job's allocation unknown, report its usage as unavailable rather than fabricating a value, and continue processing the rest of the replay without treating this as an error
|
||||
- A job left unresolved this round must be safe to re-check again on a later round without any adverse effect
|
||||
- Observable completion: given a fixture where a job is already running with no recorded decision available in the replay's own window (e.g. a job dispatched before the snapshot the replay started from), the job is reported as having unavailable usage and the replay continues to completion without error
|
||||
- _Requirements: 4.1, 4.2_
|
||||
- _Boundary: Recorded-Decision Reconciliation_
|
||||
|
||||
- [ ] 2.3 Reconstruct an already-resolved job's allocation from its recorded decision, safely
|
||||
- When a matching recorded dispatch decision is found (task 2.2), take ownership of its resource-allocation data to become the job's own allocation and accounting record — the same ownership-transfer pattern the live dispatch path already uses, not a duplicate copy
|
||||
- Once that data has been taken over, remove the now-fully-consumed recorded decision entry from this round's decoded data, releasing only the parts of it that were not taken over, and close the resulting gap so the remaining recorded entries keep their original relative order
|
||||
- A job whose allocation is reconstructed this way must never be reconsidered by this same mechanism again on a later round
|
||||
- Observable completion: reconciling a job's allocation and then later releasing that job's allocation through its normal end-of-life path is clean under the project's standard memory-safety build (no double-release, no leaked allocation) — this is the task's primary correctness bar, not a peripheral check
|
||||
- _Requirements: 1.1, 1.2, 3.2_
|
||||
- _Depends: 2.2_
|
||||
- _Boundary: Recorded-Decision Reconciliation_
|
||||
|
||||
- [ ] 2.4 Make a reconciled job's decision visible the same way a fresh one is
|
||||
- Feed a reconstructed decision (task 2.3) through the exact same decision-reporting path a freshly-computed decision already uses, preserving its original decision type and resource-allocation detail
|
||||
- If simulation mode's own live decision-making independently produces a fresh decision for a job in the same round it would otherwise be reconciled, report only the fresh one — never both
|
||||
- Observable completion: replaying a fixture where a job was already dispatched before the replay's live decision-making reaches it now shows that job's dispatch decision in step/continue output, exactly once, with the same resource-allocation detail the original recording captured
|
||||
- _Requirements: 3.1, 3.3_
|
||||
- _Depends: 2.3_
|
||||
- _Boundary: Recorded-Decision Reconciliation_
|
||||
|
||||
- [ ] 3. Integrate reconciliation into the round boundary and confirm decision-verification stays correct
|
||||
- Call the reconciliation mechanism (tasks 2.2-2.4) inside the round-boundary sequence (task 2.1), positioned after the session-start reset and before the resource re-accounting call, so the same round's re-accounting picks up every newly-reconstructed job and the still-pending live decision-making sees correct capacity
|
||||
- Confirm the round's decision-verification comparison (which walks the same recorded-decision data positionally) is unaffected by reconciliation having already consumed and removed some of those entries — no special-casing should be needed in the comparison logic itself, only in what data reaches it
|
||||
- Observable completion: running decision verification against a recording containing already-resolved jobs reports no mismatch attributable to this feature, and a job whose allocation was reconciled this round is reflected in resource-in-use totals before the live picker/allocator evaluates capacity for any other job in that same round
|
||||
- _Requirements: 1.3, 1.4, 2.1, 2.2, 5.2_
|
||||
- _Depends: 2.1, 2.4_
|
||||
- _Boundary: Sim Driver Round-Boundary Accounting, Recorded-Decision Reconciliation_
|
||||
|
||||
- [ ] 4. Validation
|
||||
- [ ] 4.1 (P) Prove capacity-aware parity under contention
|
||||
- Using the resource-contention fixture (task 1.2), replay it in simulation mode and compare worker and shared resource-in-use totals against the equivalent normal-mode recording at every round
|
||||
- Assert simulation mode never makes a scheduling decision (dispatch where normal mode pended, or vice versa) that the normal-mode recording of the same state did not also make
|
||||
- Observable completion: the contention fixture replay produces byte-for-byte identical resource-in-use totals and scheduling outcomes to its normal-mode recording, at every round, not just the final one
|
||||
- _Requirements: 1.4, 2.1, 2.2_
|
||||
- _Depends: 3_
|
||||
- _Boundary: Scheduler test suite (contention parity)_
|
||||
|
||||
- [ ] 4.2 (P) Regression suite: existing guarantees still hold
|
||||
- Re-run the existing interactive-vs-batch decision-equivalence coverage unmodified and confirm it still passes
|
||||
- Extend decision-verification coverage with a recording containing already-resolved jobs and confirm no mismatch is reported as a result of this feature
|
||||
- Confirm a replay starting from a snapshot with jobs already running at snapshot time (no recorded decision available in-window) degrades gracefully rather than crashing or fabricating a value
|
||||
- Confirm that with simulation mode not enabled, resource-accounting and decision-reporting behavior is byte-for-byte unchanged from before this feature
|
||||
- Observable completion: all four checks above pass in the same test run, with the pre-existing equivalence test's pass/fail behavior unchanged from before this feature
|
||||
- _Requirements: 4.1, 4.2, 5.1, 5.2, 5.3_
|
||||
- _Depends: 3_
|
||||
- _Boundary: Scheduler test suite (regression/verify coverage)_
|
||||
|
||||
- [ ] 4.3 Live-verify against the original motivating recording
|
||||
- Replay the real recorded session that originally surfaced the resource-accounting and decision-visibility gaps this feature fixes, through to completion
|
||||
- Observable completion: inspecting a worker mid-replay now shows nonzero resource usage for jobs independently confirmed running via their ongoing usage updates, and their dispatch decisions are now visible in step/continue output where they previously never appeared
|
||||
- _Requirements: 1.1, 1.2, 1.3, 3.1, 3.2, 3.3_
|
||||
- _Depends: 4.1, 4.2_
|
||||
- _Boundary: Scheduler test suite_
|
||||
Loading…
Add table
Add a link
Reference in a new issue