From 25e35bacaf9724e9fd859631a972aa5f550f08f0 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sat, 22 Aug 2026 02:52:00 +0000 Subject: [PATCH] docs(scheduler): document driver-equivalence invariant, scope decision-parity fix spec Elevates the normal-mode/simulation-mode driver equivalence guarantee from a one-line architectural description in CLAUDE.md to an enforced steering constraint (scheduler-driver-equivalence.md), backed by bug-1611/bug-1613 as concrete evidence of violation. Scopes the fix as its own spec (scheduler-sim-decision-parity, requirements + design) rather than reopening the closed scheduler-core spec or folding it into scheduler-sim-repl-verbosity's explicitly narrower boundary. Co-Authored-By: Claude Sonnet 5 --- .../scheduler-sim-decision-parity/brief.md | 44 ++++ .../scheduler-sim-decision-parity/design.md | 247 ++++++++++++++++++ .../requirements.md | 50 ++++ .../scheduler-sim-decision-parity/research.md | 61 +++++ .../scheduler-sim-decision-parity/spec.json | 22 ++ .../steering/scheduler-driver-equivalence.md | 36 +++ .wolf/memory.md | 1 + src/scheduler/CLAUDE.md | 2 +- 8 files changed, 462 insertions(+), 1 deletion(-) create mode 100644 .kiro/specs/scheduler-sim-decision-parity/brief.md create mode 100644 .kiro/specs/scheduler-sim-decision-parity/design.md create mode 100644 .kiro/specs/scheduler-sim-decision-parity/requirements.md create mode 100644 .kiro/specs/scheduler-sim-decision-parity/research.md create mode 100644 .kiro/specs/scheduler-sim-decision-parity/spec.json create mode 100644 .kiro/steering/scheduler-driver-equivalence.md diff --git a/.kiro/specs/scheduler-sim-decision-parity/brief.md b/.kiro/specs/scheduler-sim-decision-parity/brief.md new file mode 100644 index 00000000..d185a6ee --- /dev/null +++ b/.kiro/specs/scheduler-sim-decision-parity/brief.md @@ -0,0 +1,44 @@ +# Brief: scheduler-sim-decision-parity + +## Problem +Operators debugging scheduling behavior via `xce_scheduler -i -s ` (or batch `--sim`) cannot trust two things about jobs that were already resolved (dispatched) in the *original* recorded run: (1) their resource usage is never reflected in `w->resInUse`/`core->sharedInUse` (`info worker` always shows `used=0.00` regardless of actual load — bug-1613), and (2) their `DISPATCH` decision is never visible in decision output, even under `step` (bug-1611). Both undermine the entire premise of simulation mode: that replaying a recording reproduces what actually happened, closely enough to debug and tune against. + +## Current State +- `scheduler-core` (implementation-complete, closed) built the core/driver split: `xce_sched_core_t` is mode-agnostic; `driver.c` (normal mode) and `sim_loop.c` (simulation mode) are separate driver implementations of the same session/decision-callback protocol. +- **Confirmed (bug-1613)**: `sched_account_jobs()` — re-derives `resInUse`/`sharedInUse` from currently-RUN-state jobs' already-built `job->accounts`, required after every session's resval reset — is called by `driver.c`'s `xce_sched_driver_run_normal_mode_round()` but never by `sim_loop.c`, which still uses the monolithic `xce_core_go()` (no interleaving point for it). The function is also currently signatured on `xce_scheduler_t*` (normal-mode-only struct), which `xce_sched_sim_driver_t` deliberately does not have (Req 5 AC 17 simulation-isolation boundary) — it cannot be called from `sim_loop.c` today without a signature change. +- **Confirmed (this session, new finding)**: `xce_sched_driver_run_normal_mode_round()`'s decision-drain loop shows every decision the normal-mode scheduler makes — including `DISPATCH`, with its full `xce_job_alloc_t` allocation payload — is written into the `.replay` file via `sched_record_session_add_decision()`. `sim_loop.c` already decodes this full `decisions` list every round (`decoded_session->decisions`, via `decode_ReplaySession_ptr()`), but only ever reads it when `sim->verify` is set, purely to compare against freshly-drained live decisions. It is never applied to reconstruct `job->allocs`/`job->accounts` for a job the live picker in *this* replay run doesn't independently re-decide — which is the normal case for any job already resolved before the replay began. This is very likely the single root cause behind both bug-1611 and bug-1613: the data needed already exists in every replay stream and is already decoded every round; it is simply never applied outside verify-mode comparison. +- `.kiro/steering/scheduler-driver-equivalence.md` (created this session) documents the constraint this spec restores: replaying a normal-mode recording must reproduce identical decisions AND identical resource-accounting state, not just matching final decisions on fixtures with no contention. + +## Desired Outcome +Replaying the exact recording of a normal-mode run reproduces, at every point in the replay: the same resource-accounting state (`resInUse`/`sharedInUse`/`job->accounts`) and the same decision visibility (a `DISPATCH`/`PEND`/`PREEMPT` line is observable for every job, whether the live picker re-derived it fresh this run or it was already resolved in the original recording) that the normal-mode run itself produced. + +## Approach +Two components, one spec: +1. **Accounting re-derivation parity**: relocate `sched_account_jobs()` and its callees (`sched_charge_account`/`sched_uncharge_account`/`sched_update_account`/`sched_update_res_accounts`) from `xce_scheduler_t*` to `xce_sched_core_t*` — every one of their bodies already only touches `sched->core`, so this is a signature change, not a logic change, following the exact precedent `sched_resolve_job_picker()` already established (task 4.3) for the identical reason. Then call it in `sim_loop.c` at the same relative position `driver.c` does: replace the monolithic `xce_core_go()` with the split `xce_core_begin_session()` → `sched_account_jobs(&(sim->core))` → `xce_core_signal_go()`. +2. **Recorded-decision application**: for a round where the live picker/allocator does not independently produce a decision for a job that the recording shows as already resolved, apply the corresponding recorded decision from `decoded_session->decisions` directly — reconstructing `job->allocs`/`job->accounts` (mirroring the field data `sim_verify_job_alloc_eq()` already knows how to compare) and feeding it through the same decision pipeline `sim_print_decision()`/`xce_repl_on_decision()` consume, so it is both charged and visible. + +## Scope +- **In**: Accounting-state re-derivation parity between the two drivers; applying recorded decisions for already-resolved jobs during replay (both interactive and batch `--sim`); avoiding double-application when a job is both recorded-resolved and somehow also independently re-decided; resource-contention test fixtures proving the fix (per `scheduler-driver-equivalence.md`'s testing discipline) since no-contention fixtures cannot distinguish correct from broken accounting. +- **Out**: Any change to REPL console output/formatting (that's `scheduler-sim-repl-verbosity`, a sibling spec, explicitly excluded from touching simulation state by its own Non-Goals). Any change to `--sim-verify`'s actual comparison semantics beyond ensuring recorded-decision application doesn't produce a spurious mismatch or double-count against it. Reopening or modifying `scheduler-core` itself. New scheduling/allocation policy. + +## Boundary Candidates +- Accounting relocation (`resource.c`/`scheduler.h`/`core.h`, signature-only change) — small, mechanical, low-risk, could land independently of the second piece. +- Recorded-decision application (`sim_loop.c`, and `sim_verify.c` for the double-count/verify-interaction question) — larger, the real design surface: what "already resolved, not independently re-decided" means precisely, and how it composes with `--sim-verify`'s existing comparison. + +## 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. + +## Upstream / Downstream +- **Upstream**: `scheduler-core`'s core/driver split and its now-documented equivalence invariant (`.kiro/steering/scheduler-driver-equivalence.md`). +- **Downstream**: `scheduler-sim-repl-verbosity`'s round-rollup dispatch counts (currently designed to read from the existing decision tally) become accurate once this lands, rather than permanently under-reporting dispatches for already-resolved jobs. `scheduler-allocation`/`scheduler-preemption` (both requirements-generated, not yet designed) inherit a scheduler core with a restored, tested equivalence guarantee to build on. + +## Existing Spec Touchpoints +- **Extends**: none formally — `scheduler-core` is closed; this spec restores its own documented invariant rather than reopening it. +- **Adjacent**: `scheduler-sim-repl-verbosity` (sibling, not yet implemented — benefits from this fix but does not depend on it to ship), `scheduler-sim-diagnostics` (sibling, brief only). + +## Constraints +- Must not regress REPL-vs-batch-sim decision equivalence (`test_repl_equivalence.c`) — that invariant is solid today and must stay solid. +- Must be validated against a resource-contention fixture (multiple jobs competing for insufficient capacity), per `scheduler-driver-equivalence.md`'s testing discipline — a no-contention fixture cannot prove this fix works. +- `sched_account_jobs()`'s relocation must be a pure signature change (no behavior change) to keep the accounting-parity piece low-risk and independently reviewable from the larger recorded-decision-application piece. diff --git a/.kiro/specs/scheduler-sim-decision-parity/design.md b/.kiro/specs/scheduler-sim-decision-parity/design.md new file mode 100644 index 00000000..8addd097 --- /dev/null +++ b/.kiro/specs/scheduler-sim-decision-parity/design.md @@ -0,0 +1,247 @@ +# Design Document + +## Overview +**Purpose**: Restore the scheduler's documented normal-mode/simulation-mode driver equivalence invariant (`.kiro/steering/scheduler-driver-equivalence.md`) for jobs already resolved before a given point in a replay — both their resource-accounting state and their decision visibility. + +**Users**: XCE scheduler engineers and operators debugging via `xce_scheduler -i -s ` 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. + +### Goals +- Resource-in-use accounting (`resInUse`/`sharedInUse`) reflects every currently-running job in simulation mode, matching normal mode (Requirement 1). +- Simulation-mode capacity checks under contention produce the same outcome normal mode would (Requirement 2) — a direct consequence of Goal 1, since the capacity-check code path is unchanged and already reads this data correctly. +- Every job's decision (including ones resolved before this replay run started tracking them live) is visible through the existing decision-reporting channel (Requirement 3). +- Missing recorded data degrades gracefully, never fabricates or crashes (Requirement 4). +- Zero regression to REPL-vs-batch-sim equivalence, `--sim-verify`, or normal-mode behavior (Requirement 5). + +### Non-Goals +- Any change to REPL console output formatting (`scheduler-sim-repl-verbosity`'s boundary). +- Any new inspectable data or command (`scheduler-sim-diagnostics`'s boundary). +- Any change to scheduling/allocation policy or picker plugin behavior. +- Reopening `scheduler-core`. + +## Boundary Commitments + +### This Spec Owns +- The signatures of `sched_account_jobs()`/`sched_charge_account()`/`sched_uncharge_account()`/`sched_update_account()`/`sched_update_res_accounts()`/`sched_commit_job_alloc()` (currently `resource.c`, `xce_scheduler_t*`-coupled) and their relocation to `xce_sched_core_t*`. +- `sim_loop.c`'s round-boundary sequence (the `xce_core_go()` replacement and the new reconciliation step). +- The logic that reconstructs `job->allocs`/`job->accounts` from a recorded decision for a RUN-state job that has neither, and feeds that reconstructed decision through the existing decision-reporting channel. + +### Out of Boundary +- `driver.c`'s normal-mode round logic, beyond updating its one call site to the relocated accounting function signature. +- `core.c`/`core_events.c`'s event-application logic (JOB_PUSH/JOB_UPDATE/JOB_STATE_CHANGE handling) — unchanged. +- `repl_break.c`/`repl.c`/`repl_info.c` — unchanged; they consume whatever the decision-reporting channel produces, without needing to know whether a given decision was freshly computed or reconstructed. +- `--sim-verify`'s actual pass/fail semantics — this spec must not introduce a false mismatch, but does not redesign verification. + +### Allowed Dependencies +- `sim_verify.c`'s existing `sim_verify_job_alloc_eq()`/`sim_verify_host_alloc_eq()`/`sim_verify_alloc_eq()` field shapes as the reference for what a reconstructed `xce_job_alloc_t` must contain — not necessarily calling these functions, but matching the payload shape they already validate. +- `decoded_session->decisions`, already decoded every round by existing `sim_loop.c` code — no new decode path. +- The `sched_resolve_job_picker()` relocation (task 4.3) as the precedent for the accounting-function signature change. + +### Revalidation Triggers +- 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. + +## 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. + +### Architecture Pattern & Boundary Map + +```mermaid +flowchart TD + BeginSession[xce_core_begin_session] --> Reconcile[Reconcile RUN jobs with no accounts] + 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] + Report --> AccountJobs[sched_account_jobs on core] + Unavailable --> AccountJobs + AccountJobs --> SignalGo[xce_core_signal_go] + SignalGo --> Picker[Live picker and allocator run for still-pending jobs] + Picker --> DrainLoop[Decision drain loop -- same reporting channel for fresh decisions] +``` + +**Architecture Integration**: +- Selected pattern: extend the existing round-boundary interleaving slot (the same one `driver.c` already uses for its own bookkeeping) with one additional step, rather than a new hook or a second round-processing pass. +- Domain/feature boundaries: reconciliation logic lives in `sim_loop.c` only; the relocated accounting functions live in `resource.c`/`core.h` and are shared by both drivers; `driver.c` gets a one-line call-site update. +- Existing patterns preserved: the `job->accounts == NULL` check `sched_account_jobs()` already performs is reused as the reconciliation trigger and the double-application guard — no new state. +- New components rationale: reconciliation must run before `sched_account_jobs()` (so the same round's re-charge picks up newly-reconstructed jobs) and before `xce_core_signal_go()` (so the live picker/allocator's capacity checks for still-pending jobs see correct usage this round, satisfying Requirement 2). +- Steering compliance: `sim_loop.c` remains at its documented interleaving-slot discipline (no new call sites added to the core's own API surface); `resource.c`'s functions gain no new behavior, only a new valid signature. + +### Technology Stack + +| Layer | Choice / Version | Role in Feature | Notes | +|-------|------------------|------------------|-------| +| Scheduler daemon (C, gnu17) | existing `src/scheduler/` toolchain | All logic in existing driver/core/verify translation units | No new dependency; matches `tech.md` | + +## 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/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/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. + +### 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. +- A new resource-contention fixture (multiple jobs competing for insufficient capacity) added to the scheduler's simulation testsuite, per `scheduler-driver-equivalence.md`'s testing discipline — required because existing fixtures evidently don't exercise this path. +- `src/scheduler/testsuite/test_sim_equivalence.c` (existing, decision-identity coverage) — extended with a case where a job is already resolved before the replay's live picker runs, verifying decision AND accounting-state identity, not just decision identity. + +## System Flows +See the Architecture Pattern & Boundary Map diagram above — it is the single non-trivial flow this feature introduces (the round-boundary reconciliation decision tree). No sequence diagram is needed beyond it; the decision-drain loop's own flow is unchanged. + +## Requirements Traceability + +| Requirement | Summary | Components | Interfaces | Flows | +|-------------|---------|-------------|-----------|-------| +| 1.1, 1.2, 1.3, 1.4 | Accurate resource-usage reporting | Core-Level Resource Accounting, Sim Driver Round-Boundary Accounting | relocated `sched_account_jobs` | Round-boundary flow | +| 2.1, 2.2 | Correct capacity awareness under contention | Core-Level Resource Accounting (consequence of 1.x being correct) | `sched_check_host_satisfy_`/`sched_check_shared_satisfy_` (unchanged, already reads this data) | Round-boundary flow | +| 3.1, 3.2, 3.3 | Decision visibility for pre-resolved jobs | Recorded-Decision Reconciliation | decision-reporting channel | Round-boundary flow | +| 4.1, 4.2 | Graceful degradation | Recorded-Decision Reconciliation | n/a (error path) | Round-boundary flow | +| 5.1, 5.2, 5.3 | No regression | all components | n/a | n/a | + +## Components and Interfaces + +| Component | Domain/Layer | Intent | Req Coverage | Key Dependencies (P0/P1) | Contracts | +|-----------|---------------|--------|---------------|---------------------------|-----------| +| Core-Level Resource Accounting | Scheduler core / resource.c, core.h | Re-derive resource-in-use state from currently-running jobs' known allocations, callable by either driver | 1.1-1.4, 2.1-2.2 | `xce_sched_core_t` (P0) | Service | +| Sim Driver Round-Boundary Accounting | Sim driver / sim_loop.c | Call the core-level accounting function at the correct point in the round boundary, mirroring the normal-mode driver | 1.3, 1.4, 5.3 | Core-Level Resource Accounting (P0) | State | +| Recorded-Decision Reconciliation | Sim driver / sim_loop.c | Reconstruct allocation/accounting and decision visibility for a RUN-state job with no accounts, from this round's recorded decisions | 3.1-3.3, 4.1-4.2 | `decoded_session->decisions` (P0), decision-reporting channel (P0) | Service, State | + +### Scheduler Core (resource.c / core.h) + +#### Core-Level Resource Accounting +| Field | Detail | +|-------|--------| +| Intent | Re-derive `resInUse`/`sharedInUse` from every currently-running job's known resource allocation, usable identically by both drivers | +| Requirements | 1.1, 1.2, 1.3, 1.4, 2.1, 2.2 | + +**Responsibilities & Constraints** +- Body unchanged from today's `sched_account_jobs()`/callees — this is a signature relocation (`xce_scheduler_t*` → `xce_sched_core_t*`), not a logic change, since every reference in the existing bodies is already `sched->core.*`. +- Both drivers call it at the identical relative position: after `xce_core_begin_session()`'s resval reset, before `xce_core_signal_go()` unblocks the picker/allocator. +- Requirement 2 (capacity awareness) is satisfied as a direct consequence — `sched_check_host_satisfy_()`/`sched_check_shared_satisfy_()` already read `resInUse`/`sharedInUse` unconditionally; once those fields are correctly populated in simulation mode, the existing (unmodified) capacity-check code path produces the same outcome normal mode would. + +**Contracts**: Service [x] / State [ ] + +##### Service Interface +```c +/* + * Re-charges every currently-running job's known resource allocation + * into core->sharedInUse and each worker's resInUse. Skips a job whose + * allocation is not yet known (job->accounts == NULL) -- reconciling + * that is the Recorded-Decision Reconciliation component's job, which + * must run first in the same round-boundary window for this call to + * see a fully up-to-date job set. + */ +int sched_account_jobs(xce_sched_core_t* core); +``` +- Preconditions: `core` non-NULL; called strictly after `xce_core_begin_session()`'s resval reset for the current round. +- Postconditions: every RUN-state job with known accounts is reflected in `resInUse`/`sharedInUse`; a RUN-state job with unknown accounts is silently skipped (not an error — see Recorded-Decision Reconciliation for why that's expected before this call runs, and Requirement 4 for the case where it stays unknown). +- Invariants: identical result given identical core state, regardless of which driver calls it. + +### Sim Driver (sim_loop.c) + +#### Sim Driver Round-Boundary Accounting +| Field | Detail | +|-------|--------| +| Intent | Call Core-Level Resource Accounting at the same relative round-boundary position `driver.c` already uses | +| Requirements | 1.3, 1.4, 5.3 | + +**Responsibilities & Constraints** +- Replaces the current single `xce_core_go(&(sim->core))` call with the split sequence, matching `xce_sched_driver_run_normal_mode_round()`'s documented ordering exactly. +- Must not change `xce_core_go()`'s existing error-handling contract (a failed `xce_core_begin_session()` still skips the round's decision drain, per today's `sim_loop.c` error path) — the split must preserve this, not simplify it away. +- Headless (non-interactive) and interactive (`-i`) simulation runs both go through this same call site — no divergence between them (Requirement 5.1 depends on this). + +**Contracts**: State [x] + +#### Recorded-Decision Reconciliation +| Field | Detail | +|-------|--------| +| Intent | For a RUN-state job with no known allocation, reconstruct it from this round's recorded decision and report it through the existing decision channel | +| Requirements | 3.1, 3.2, 3.3, 4.1, 4.2 | + +**Responsibilities & Constraints** +- Runs once per round, after `xce_core_begin_session()`, before `sched_account_jobs()`. +- Trigger condition: a job in `sched->core.jobs` (via the existing core.h iteration idiom) that is RUN-state and has `job->accounts == NULL`. +- On trigger, search this round's `decoded_session->decisions` for a matching jobid. If found, reconstruct `job->allocs`/`job->accounts` from the recorded payload (same field shape `sim_verify_job_alloc_eq()` already validates) and feed the reconstructed decision through the same reporting call the existing decision-drain loop uses for a freshly-drained decision (Requirement 3.1). +- If not found (Requirement 4): report the job's usage as unavailable — do not fabricate a value, do not treat this as fatal, continue processing the rest of the replay. This is the expected case for a job already running at the moment the replay's starting snapshot was taken (see Risks). +- Because `job->accounts` is set as a side effect, a job reconciled in round N is never reconsidered in round N+1 — the same NULL-check that identifies the trigger also prevents double-application (Requirement 3.3), including against a job that separately receives a fresh live decision in the same round (that path sets `job->accounts` too, via the unchanged `sched_commit_decision()`, so this component simply finds nothing to do for it). + +**Contracts**: Service [x] / State [x] + +##### Service Interface +```c +/* + * For every RUN-state job in `sim->core.jobs` with job->accounts == + * NULL, attempts to reconstruct its allocation from a matching + * decision in this round's decoded_session->decisions and reports it + * through the same channel a freshly-drained decision uses. A job with + * 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. + */ +void sim_reconcile_unaccounted_jobs(xce_sched_sim_driver_t* sim, + const 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. + +**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. +- 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). + +## Data Models + +### Domain Model +No new persistent entity. `job->accounts`/`job->allocs` (existing fields on `xce_sched_job_t`, `xce/types.h`) become the single source of truth for "has this job's allocation been established, by either the live path or reconciliation" — reused, not extended. + +### Logical Data Model +No new fields. The reconciliation step reads `decoded_session->decisions` (already-decoded, existing structure) and writes `job->allocs`/`job->accounts` (existing fields) exactly as the live `sched_commit_decision()` path already does for a fresh dispatch — same target shape, different source. + +No Physical Data Model, API Data Transfer, or Event Schema sections apply — no persistent storage, no new wire format, no new wire-level contract. This stays inside the existing `.replay` stream shape. + +## Error Handling + +### Error Strategy +Consistent with this codebase's established "never load-bearing, degrade to unattended" discipline (`repl_state_ensure()`'s own precedent): a reconciliation failure for one job never blocks the rest of the replay. + +### Error Categories and Responses +- **No matching recorded decision for a RUN-state, unaccounted job** (Requirement 4.1, 4.2): report usage as unavailable, continue. Not an error condition — expected for jobs already running at snapshot time. +- **Allocation failure while reconstructing `job->accounts` from a recorded decision**: fall back to leaving the job unaccounted (same as "not found") rather than a partial/corrupt reconstruction — preserves Requirement 4's "never fabricate" guarantee. +- **`xce_core_begin_session()` failure**: unchanged from today's `sim_loop.c` behavior — the round's decision drain is skipped, matching the existing error path this feature's split-call migration must preserve exactly. + +### Monitoring +No new logging framework — existing `WARN`/`TRACE` conventions apply if reconciliation needs to report a skip; no daemon-level behavior change. + +## Testing Strategy + +### Unit Tests +- Core-Level Resource Accounting produces identical `resInUse`/`sharedInUse` output whether called with a `core` obtained from `xce_scheduler_t` or from `xce_sched_sim_driver_t` (Requirement 1.4). +- 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). + +### 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). +- `test_repl_equivalence.c` (existing) re-run unmodified to confirm no regression to REPL-vs-batch-sim equivalence (Requirement 5.1). +- `--sim-verify` run against a recording containing already-resolved jobs, confirming no false mismatch is reported as a result of this feature (Requirement 5.2). +- A replay starting from a snapshot with jobs already running at snapshot time, confirming graceful degradation (Requirement 4) rather than a crash or fabricated value. + +### E2E +Live-verify against the real 108-round replay capture that originally motivated bug-1611/bug-1613: after the fix, `info worker` shows nonzero `used=` for jobs confirmed running via `JOB_RUSAGE` ticks, and their `DISPATCH` decisions become visible in `step`/`continue` output. + +## Optional Sections +Security Considerations, Performance & Scalability, and Migration Strategy are omitted: this is an internal scheduler-daemon correctness fix with no new attack surface, no new persistent data, and no schema/data migration. Performance is bounded by existing per-round job iteration this codebase already tolerates (`sched_account_jobs()`'s own existing full-job-table walk). + +## Supporting References +None beyond `research.md`. diff --git a/.kiro/specs/scheduler-sim-decision-parity/requirements.md b/.kiro/specs/scheduler-sim-decision-parity/requirements.md new file mode 100644 index 00000000..2eb28dc4 --- /dev/null +++ b/.kiro/specs/scheduler-sim-decision-parity/requirements.md @@ -0,0 +1,50 @@ +# Requirements Document + +## Introduction +The scheduler's `.replay`-driven simulation mode (`--sim`/`--interactive`) exists so operators can debug and tune scheduling behavior against recorded production data, trusting that a replay reproduces what actually happened. That trust currently does not hold for any job already resolved (dispatched, pended, or preempted) *before* a given point in the replay: its resource usage is never reflected in worker/shared resource-in-use totals, and its decision is never visible in decision output — because the recorded decision stream, though fully decoded every round, is only ever used for optional `--sim-verify` comparison, never applied to reconstruct state or report the decision otherwise. This feature restores decision *and* resource-accounting state parity between the normal-mode and simulation-mode drivers, per the constraint documented in `.kiro/steering/scheduler-driver-equivalence.md`. + +## Boundary Context +- **In scope**: resource-in-use accounting correctness during simulation-mode execution (interactive and batch); capacity-awareness correctness of simulation-mode scheduling decisions under resource contention; decision visibility (through existing decision-reporting channels) for jobs already resolved in the original recording; graceful behavior when recorded decision data for a job is unavailable. +- **Out of scope**: any change to REPL console output formatting (owned by the sibling `scheduler-sim-repl-verbosity` spec); any new inspectable data or REPL command (owned by the sibling `scheduler-sim-diagnostics` brief); any change to `--sim-verify`'s comparison logic itself, beyond not producing a false mismatch as a side effect of this feature; any change to scheduling/allocation policy; any change to normal-mode (non-simulation) behavior. +- **Adjacent expectations**: this feature restores an invariant `scheduler-core` (implementation-complete) was already expected to uphold; it does not reopen or modify that spec. `scheduler-sim-repl-verbosity`'s round-summary decision counts depend on decisions being reported through the same channel this feature restores, but that spec does not depend on this one to ship. + +## Requirements + +### Requirement 1: Accurate resource-usage reporting in simulation mode +**Objective:** As an operator inspecting simulation-mode resource state, I want per-worker and shared resource usage to reflect every currently-running job, so that what I inspect during a replay is trustworthy. + +#### Acceptance Criteria +1. While a job is in a running state during simulation-mode execution, the scheduler shall include that job's resource consumption in its worker's resource-in-use total. +2. While a job is in a running state during simulation-mode execution, the scheduler shall include that job's resource consumption in any shared resource's resource-in-use total it consumes from. +3. When a scheduling round completes, the scheduler shall refresh resource-in-use totals to reflect every currently-running job, not only jobs whose resource consumption was already reflected in an earlier round. +4. When replaying a recorded run in simulation mode, the scheduler shall report the same resource-in-use totals as normal-mode execution reported at the equivalent point in the original run. + +### Requirement 2: Correct capacity awareness under resource contention in simulation mode +**Objective:** As an operator relying on simulation mode to reflect realistic scheduling outcomes, I want the scheduler's capacity checks during simulation-mode execution to account for resources already consumed by running jobs, so that simulated scheduling behavior matches live behavior under contention. + +#### Acceptance Criteria +1. While resource capacity is already consumed by running jobs, when the scheduler evaluates whether a job fits on a worker during simulation-mode execution, the scheduler shall treat that already-consumed capacity as unavailable, the same way normal-mode execution does. +2. The scheduler shall not make a simulation-mode scheduling decision that normal-mode execution of the same recorded state would not also make. + +### Requirement 3: Decision visibility for jobs resolved before replay began +**Objective:** As an operator watching simulation-mode decision output, I want to see a decision for every job the replay reaches, including jobs already resolved in the original run, so I have a complete picture of scheduling activity. + +#### Acceptance Criteria +1. When a job was dispatched, pended, or preempted in the original recorded run and simulation-mode replay reaches the point that event occurred, the scheduler shall make that decision observable through the same decision-reporting channel used for a freshly-computed decision. +2. The scheduler shall preserve the original decision's type and resource-allocation detail when reporting it during replay. +3. When simulation-mode independently computes a fresh decision for a job in a given round, the scheduler shall report only that one decision for the job in that round, not also a duplicate from the recorded stream. + +### Requirement 4: Graceful degradation when recorded decision data is unavailable +**Objective:** As an operator running simulation mode against an incomplete or older recording, I want the scheduler to degrade gracefully rather than fail, so a data gap in one job's history doesn't block inspection of the rest of the replay. + +#### Acceptance Criteria +1. If a job's originating decision cannot be found in the recorded data when simulation-mode needs it to report or account for that job, the scheduler shall report that job's resource usage as unavailable rather than fabricating a value. +2. If a job's originating decision cannot be found in the recorded data, the scheduler shall continue processing the remainder of the replay without terminating. + +### Requirement 5: No regression to existing driver-equivalence and verification guarantees +**Objective:** As a maintainer relying on existing scheduler guarantees, I want this fix to preserve them, so existing verification workflows and tests continue to hold. + +#### Acceptance Criteria +1. The scheduler shall continue to produce identical decisions between interactive (step-by-step) and batch simulation execution modes. +2. The scheduler shall continue to support decision verification mode (comparing freshly-computed decisions against recorded ones) without reporting a mismatch caused by this feature's own behavior. +3. While simulation mode is not enabled, the scheduler shall not change its existing resource-accounting or decision-reporting behavior. diff --git a/.kiro/specs/scheduler-sim-decision-parity/research.md b/.kiro/specs/scheduler-sim-decision-parity/research.md new file mode 100644 index 00000000..f81836ea --- /dev/null +++ b/.kiro/specs/scheduler-sim-decision-parity/research.md @@ -0,0 +1,61 @@ +# Research & Design Decisions + +## Summary +- **Feature**: `scheduler-sim-decision-parity` +- **Discovery Scope**: Extension (existing system) — light discovery +- **Key Findings**: + - `sched_check_host_satisfy_()`/`sched_check_shared_satisfy_()` (resource.c) read `w->resInUse`/`core->sharedInUse` directly as inputs to the live allocator's own capacity check — bug-1613's accounting gap is **confirmed** to affect simulation-mode scheduling decisions under contention, not just displayed data. This resolves the open question from the brief and raises this spec's priority. + - Every decision the normal-mode scheduler makes (including `DISPATCH` with its full `xce_job_alloc_t` payload) is written into the `.replay` stream via `sched_record_session_add_decision()` (`xce_sched_driver_run_normal_mode_round()`, driver.c). `sim_loop.c` already decodes this per round (`decoded_session->decisions`) but only reads it under `sim->verify`, purely for comparison. + - `job->accounts == NULL` is already the exact signal `sched_account_jobs()` uses to skip a job it can't re-charge — the same signal cleanly identifies "this RUN-state job has never been reconstructed" and, once set, naturally prevents double-application from a later pass (idempotent by construction, no new bookkeeping field needed). + - `sim_verify_job_alloc_eq()`/`sim_verify_host_alloc_eq()`/`sim_verify_alloc_eq()` (sim_verify.c) already implement field-by-field equality for exactly the `xce_job_alloc_t`/`xce_host_alloc_t`/`xce_alloc_t` payload shapes this feature needs to reconstruct `job->allocs` from — reusable, not something to reinvent. + - The actual round-level comparison driver in sim_verify.c (as opposed to the eq-helpers read this session) was not read — how `--sim-verify` currently handles a recorded decision with no live-drained counterpart (which is exactly the case this feature changes the meaning of) needs to be confirmed at implementation time, flagged as a risk below. + +## Research Log + +### Does resInUse feed actual scheduling decisions, or only display data? +- **Context**: The brief flagged this as unconfirmed and consequential — determines whether bug-1613 is a display bug or a decision-correctness bug. +- **Sources Consulted**: `resource.c` (`sched_check_host_satisfy_`, `sched_check_shared_satisfy_`, `sched_try_alloc_on_host_`). +- **Findings**: `sched_check_host_satisfy_()` reads `w->resvals` (total) and `w->resInUse` (in-use) and passes both to `unsatisfy_()`, whose result determines whether `sched_try_alloc_on_host_()` (the live allocator's per-host evaluation, called from the picker/allocator's job-election path) treats a host as viable. `sched_check_shared_satisfy_()` does the identical thing against `core->sharedInUse` for shared resources. +- **Implications**: Confirmed, not hypothetical. Requirement 2 exists because this mechanism is real: with `resInUse` permanently zero, simulation-mode capacity checks can systematically pass where normal mode would fail, meaning simulated scheduling under contention can diverge from what actually happened. + +### Where does the recorded decision stream carry allocation data, and what does the normal-mode driver already do with decisions? +- **Context**: Confirming decoded_session->decisions is a complete, reusable data source rather than a partial one. +- **Sources Consulted**: `driver.c` (`xce_sched_driver_run_normal_mode_round`'s decision-drain loop), `sim_verify.c` (equality helpers). +- **Findings**: Every decision drained from `sched->core.decisions` in a normal-mode round — before being handed to `sched->core.on_decision` — is recorded via `sched_record_session_add_decision()` if recording is active, with its full payload (`xce_job_alloc_t` for DISPATCH, including per-host and shared-resource allocation lists). `sim_verify.c` already has mature, tested field-by-field equality functions for this exact payload shape. +- **Implications**: `decoded_session->decisions` is a complete historical record of every decision (not just PEND, which is the only op the live sim-mode picker currently re-derives for already-resolved jobs). Reconstructing `job->allocs` from a recorded `DispatchDecision` is a straightforward field copy, not a reverse-engineering exercise — and the equality-checking precedent in `sim_verify.c` shows the exact shape to build against. + +## Architecture Pattern Evaluation + +| Option | Description | Strengths | Risks / Limitations | Notes | +|--------|-------------|-----------|---------------------|-------| +| Reconstruct from recorded decisions, keyed on `job->accounts == NULL` (chosen) | At the round-boundary accounting point, for every RUN-state job with no accounts, look up and apply its recorded decision from this round's `decoded_session->decisions` | Reuses data already decoded every round; reuses `sim_verify.c`'s existing payload-equality precedent for the reconstruction shape; the NULL-accounts check is naturally idempotent (no new dedup bookkeeping) | Requires care around `--sim-verify`'s existing comparison semantics (see Risks) | Directly extends the point Requirement 1's accounting fix already establishes — one round-boundary pass, not a second parallel mechanism | +| Reconstruct from `JOB_UPDATE`'s wire `allocs` field | Extend `core_events.c`'s `XCE_CORE_EVENT_JOB_UPDATE` handler to check `updated->allocs != NULL`, mirroring `core_build_job()`'s existing JOBS_PUSH pattern | Simpler, localized to one file | **Rejected as primary mechanism** — confirmed this session that allocation data does not travel via `JOB_UPDATE` events at all; it travels via the recorded decision stream exclusively (see Research Log). This approach has no data to act on for the dominant case. | Not viable as the primary fix; not pursued further | +| Re-run the live picker/allocator against every job every round, discard non-matching results | Force full re-derivation regardless of recording | Would not depend on recorded-decision structure at all | Breaks the entire premise of replay (a deterministic capture-and-replay, not a live re-simulation) and risks non-determinism/performance cost re-running allocation logic that already ran once | Rejected — contradicts `scheduler-core`'s own design intent for what "replay" means | + +## Design Decisions + +### Decision: Key reconstruction and dedup on `job->accounts == NULL`, not a separate tracking set +- **Context**: Need to (a) know which RUN-state jobs need reconstruction from a recorded decision, and (b) avoid double-applying when a job also gets a fresh live decision in the same round. +- **Alternatives Considered**: (1) a separate per-job "already reconciled" flag/set; (2) reuse the existing `job->accounts == NULL` check `sched_account_jobs()` already performs. +- **Selected Approach**: (2). +- **Rationale**: `job->accounts` is set exactly once, at the moment a job's allocation becomes known — either via a fresh `sched_commit_decision()` (live path, unchanged by this feature) or via this feature's reconstruction pass. Once set, both `sched_account_jobs()`'s re-charge and this feature's own reconstruction pass naturally skip the job on every subsequent round — no new state, no drift risk between two independently-maintained bookkeeping mechanisms. +- **Trade-offs**: None significant — this is a strict simplification over introducing new state. +- **Follow-up**: none. + +### Decision: Feed a reconstructed decision through the same reporting path as a fresh one +- **Context**: Requirement 3 needs decision *visibility*, not just accounting correctness — an operator must see the decision, not only have `resInUse` quietly become correct. +- **Alternatives Considered**: (1) only fix accounting, leave decision visibility as a separate/future concern; (2) synthesize a decision object from the recorded data and route it through the exact same callback (`on_decision`/`xce_repl_check_decision`) a freshly-drained decision uses. +- **Selected Approach**: (2). +- **Rationale**: Requirement 3.1 explicitly requires "the same decision-reporting channel used for a freshly-computed decision" — a parallel/different reporting path would be a second thing to keep in sync, exactly the class of bug this whole spec exists to close. +- **Trade-offs**: The reconstructed decision must be a genuine `xce_decision_t` (or converted to one), not a display-only string — slightly more construction work than a printf, but necessary for it to be indistinguishable from a live decision to any consumer (REPL, `scheduler-sim-repl-verbosity`'s round rollup, etc.). +- **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. + +## References +- `.wolf/buglog.json` bug-1611, bug-1613 — the originating findings. +- `.kiro/steering/scheduler-driver-equivalence.md` — the invariant this spec restores, including its testing discipline (resource-contention fixtures required). +- `.kiro/specs/scheduler-sim-repl-verbosity/` — sibling spec; its round-rollup dispatch counts benefit from this fix once implemented but do not depend on it. diff --git a/.kiro/specs/scheduler-sim-decision-parity/spec.json b/.kiro/specs/scheduler-sim-decision-parity/spec.json new file mode 100644 index 00000000..849339e6 --- /dev/null +++ b/.kiro/specs/scheduler-sim-decision-parity/spec.json @@ -0,0 +1,22 @@ +{ + "feature_name": "scheduler-sim-decision-parity", + "created_at": "2026-08-21T08:30:00Z", + "updated_at": "2026-08-21T09:05:00Z", + "language": "en", + "phase": "design-generated", + "approvals": { + "requirements": { + "generated": true, + "approved": true + }, + "design": { + "generated": true, + "approved": false + }, + "tasks": { + "generated": false, + "approved": false + } + }, + "ready_for_implementation": false +} diff --git a/.kiro/steering/scheduler-driver-equivalence.md b/.kiro/steering/scheduler-driver-equivalence.md new file mode 100644 index 00000000..6d54d000 --- /dev/null +++ b/.kiro/steering/scheduler-driver-equivalence.md @@ -0,0 +1,36 @@ +# Scheduler Driver Equivalence + +## Philosophy + +`scheduler-core` split the scheduling engine into a mode-agnostic **core** state machine (`core.c`/`core_events.c`/`core_session.c`, `xce_sched_core_t`) driven by two interchangeable **drivers**: `driver.c` (controller-connected, capnp-driven, live) and `sim.c`/`sim_loop.c` (standalone, `.replay`-file-driven). The entire value of simulation/replay mode rests on one premise: replaying the exact recording of a normal-mode run reproduces what actually happened, so an operator can debug and tune scheduling against recorded production data and trust the result. `src/scheduler/CLAUDE.md` states this as "one core, two consumers" — this file makes it an enforced constraint, not a description. + +## DESIGN INVARIANT: State equivalence, not just decision equivalence + +**Replaying the exact recording of a normal-mode run MUST reproduce identical scheduling decisions AND identical resource-accounting state** (`w->resInUse`, `core->sharedInUse`, `job->accounts`) **at every point in the replay** — not just a matching final decision sequence. + +Decision-only equivalence is an insufficient test of this invariant. A fixture with no resource contention (every job fits regardless of what `resInUse` says) can produce identical decisions between normal and sim mode while the underlying accounting state has silently diverged — because nothing in that fixture ever depends on `resInUse` being correct. The divergence only becomes decision-visible under contention, which is exactly the scenario most replay-based debugging exists to investigate. **Treat accounting-state parity as load-bearing, not incidental to decision parity.** + +This is a stricter, broader sibling of the REPL-vs-batch-sim invariant (`--interactive` and batch `--sim` both drive `sched_sim_driver_run()` gated only by `sim->interactive`, proven decision-equivalent by construction and enforced by `test_repl_equivalence.c`). That invariant is solid — both paths share the exact same driver code. Normal-mode-vs-sim-mode equivalence is weaker: they are *separate* driver implementations that must each independently uphold the same per-round contract around the shared core, and nothing mechanically forces them to stay in sync when one changes. + +## Why a driver can silently violate this + +The core deliberately does not own accounting. `xce_core_begin_session()`'s resval reset zeroes `resInUse` every session; re-summing it from currently-running jobs (`sched_account_jobs()`) is documented as "real, load-bearing production behavior... `xce_scheduler_t*`-coupled and therefore outside this pure-core module's reach" (`core.h`) — i.e. each driver must call it itself, in the gap between `xce_core_begin_session()` and `xce_core_signal_go()`. `driver.c`'s `xce_sched_driver_run_normal_mode_round()` does this correctly. Nothing forces a second driver implementation to remember to. + +Similarly, a `ReplaySession` carries a `decisions` list — the recorded decisions from the original live run — decoded every round via `decode_ReplaySession_ptr()`. There is no structural guarantee a driver applies that data (reconstructing `job->allocs`/`job->accounts` for a job already resolved in the recording) rather than only using it for optional comparison. + +## Known violations (evidence trail) + +- **bug-1613** (confirmed): `sched_account_jobs()` is called by `driver.c` but never by `sim_loop.c`, which still uses the older monolithic `xce_core_go()` with no interleaving point for it. Compounding this, `sched_account_jobs()`/`sched_charge_account()`/`sched_update_account()`/`sched_update_res_accounts()` are signatured on `xce_scheduler_t*` (the normal-mode-only driver struct) even though their bodies only ever touch `sched->core` — `xce_sched_sim_driver_t` deliberately has no `xce_scheduler_t*` field (Req 5 AC 17, simulation-isolation boundary), so these functions cannot even be called from the sim driver today without a signature change. `sched_resolve_job_picker()` went through this exact `xce_scheduler_t*` → `xce_sched_core_t*` relocation already (scheduler.h documents it) — the precedent to follow, not a new pattern to invent. +- **bug-1611** (suspected same root cause): `sim_loop.c` only reads `decoded_session->decisions` when `sim->verify` is set, purely to compare against freshly-drained live decisions. It is never applied to reconstruct state for a job the live picker in this replay isn't independently re-deciding. This plausibly explains both bug-1611 (no `DISPATCH` decision ever visible for such jobs) and the accounting gap in bug-1613 (their usage is never counted, not even transiently) as one underlying gap, not two. +- Both are tracked in `.wolf/buglog.json`; a dedicated fix spec is scoped separately (see `.kiro/specs/` — do not fix inline as a drive-by patch, per the testing discipline below). + +## Testing discipline + +Any change touching resource accounting (`resource.c`), core event application (`core_events.c`), or driver session-boundary handling (`driver.c`, `sim_loop.c`, `core_session.c`) must be validated against **a resource-contention fixture** — multiple jobs competing for capacity insufficient for all of them — not only a fixture where every job fits. A no-contention fixture cannot distinguish "accounting is correct" from "accounting is wrong but nothing currently reads it." Existing coverage in this space (a normal-mode-recording-vs-simulation-replay decision-identity test) evidently does not exercise contention, or bug-1613 would have failed it. + +When adding or modifying driver-specific per-round logic in either `driver.c` or `sim_loop.c`, check the other driver for the equivalent step before assuming none is needed — the two are not required to look identical, but any core-derived state (`resInUse`, `sharedInUse`, `job->accounts`, decision counts) they produce from equivalent input MUST match. + +## See Also + +- `src/scheduler/CLAUDE.md` — architecture overview; "Drivers" section points here for the equivalence contract. +- `.kiro/specs/scheduler-core/` — original driver-split design and requirements. diff --git a/.wolf/memory.md b/.wolf/memory.md index 0c0d1504..157bd492 100644 --- a/.wolf/memory.md +++ b/.wolf/memory.md @@ -2556,3 +2556,4 @@ | 07:46 | ASAN build+full testsuite verified for repl_break.c/sim.c session-count changes | src/scheduler/src/repl_break.c, src/scheduler/src/sim.c, src/scheduler/testsuite/test_repl_breakpoints_inspection.c | 42/43 Ok, only known bug-1560 admin_request symlink failure; new+changed tests all PASS | ~55k | | 00:48 | Created ../../../../../../tmp/claude-1000/-home-rshen-projects-xce-worktrees-scheduler-framework/0334ebbe-8d9c-4931-871d-87c678f83a79/scratchpad/log_bug_1602.py | — | ~1054 | | 08:31 | Added event time/session echo (session_count+1, events precede their own BEGIN_ROUND marker) and replay-file session-count pre-scan (ftell/fseek, shape-only) | src/scheduler/src/repl_break.c, src/scheduler/src/sim.c, src/scheduler/testsuite/test_repl_breakpoints_inspection.c, .wolf/buglog.json | GREEN, 42/43 Ok, only pre-existing bug-1560 remains | ~110000 | +| 09:06 | Created scheduler-sim-decision-parity spec (requirements+design) in fix-bug-1613 worktree, fixing the unified bug-1611/bug-1613 root cause; confirmed sched_check_host_satisfy_/sched_check_shared_satisfy_ read resInUse/sharedInUse directly, so contention-mode sim decisions are provably affected, not just display | .kiro/specs/scheduler-sim-decision-parity/{spec.json,requirements.md,research.md,design.md}, .kiro/steering/scheduler-driver-equivalence.md | design self-reviewed (all 14 req IDs traced), next: /kiro-spec-tasks | ~120000 tok | diff --git a/src/scheduler/CLAUDE.md b/src/scheduler/CLAUDE.md index f27834dd..c2e6e63b 100644 --- a/src/scheduler/CLAUDE.md +++ b/src/scheduler/CLAUDE.md @@ -12,7 +12,7 @@ C-based job scheduling engine with a plugin architecture. Determines job-to-node ## Architecture - **Core** (`core.c`/`core_events.c`/`core_session.c`/`core_clock.c`) — passive, event-driven `xce_sched_core_t` state machine. Zero controller I/O (no `netio`/`xceapi`/`client`/`delay`/`idgen`); `core_clock.c` is the one sanctioned exception (`clock_gettime`, virtualized two-base+speed clock formula, `xce_core_now()`). `test_core_isolation.c` mechanically enforces this for `core.c`/`core.h`/`core_events.c`/`core_session.c` (source-text scan) plus `core_clock.c` too on the `nm(1)` undefined-symbol layer (all four compiled together into the isolated `sched_core_isolation_lib` archive, meson.build) — `core_clock.c` is deliberately excluded only from the wall-clock text scan, since it owns the one sanctioned `clock_gettime()` call. Picker→allocator handoff is a strict `jbuf` cbuf capacity-1 rendezvous — this is the project's determinism invariant, not a tuning knob; never raise it without re-deriving Req 5 AC 7 in `.kiro/specs/scheduler-core/requirements.md`. - **Picker plugin interface** (`plugin.h`'s `xce_job_picker_iface_t`, `plugins/builtin_fcfs.c`) — job-selection strategies are pluggable; `scheduler.c`'s `sched_job_picker()` branches on whether a plugin still uses the legacy `job_elect_fn`-only shape (`plugins/fcfs.c`, kept as-is for a compat-wrapper exercise, not the live default) vs the native hook interface (`builtin_fcfs`, the registered default). New scheduling policies → add as a picker plugin under `src/plugins/`, do not modify the core loop. -- **Drivers** — `driver.c` (normal mode: capnp↔core event translation, decision publishing, deferred admin-request handling) and `sim.c`/`sim_loop.c`/`sim_record.c`/`sim_verify.c` (simulation mode: `.replay` file loading, discrete virtual clock, `--sim-verify`/`--sim-alt-events`/`--sim-output`). Both drive the identical core session/decision-callback protocol — one core, two consumers. +- **Drivers** — `driver.c` (normal mode: capnp↔core event translation, decision publishing, deferred admin-request handling) and `sim.c`/`sim_loop.c`/`sim_record.c`/`sim_verify.c` (simulation mode: `.replay` file loading, discrete virtual clock, `--sim-verify`/`--sim-alt-events`/`--sim-output`). Both drive the identical core session/decision-callback protocol — one core, two consumers. **This equivalence is a DESIGN INVARIANT, not just an architectural description — see `.kiro/steering/scheduler-driver-equivalence.md` for the precise (state-level, not just decision-level) contract, known violations (bug-1611, bug-1613), and the testing discipline required of any change in this area.** - **Replay/recording** — `recording.c`/`recording_events.c`/`recording_session.c` (live capture) and `sim.c`/`sim_loop.c` (simulation-side decode) do the actual capnp encode/decode of the `.replay` binary stream (`src/schema/controller.capnp`); `snapshot.c`/`snapshot_wire.c`/`snapshot_load.c` only populate/consume the plain in-memory `sched_snapshot_data_t` struct that `recording.c`'s `write_snapshot_entry()` then serializes — no capnp or file I/O lives in the snapshot files themselves. Controlled via `OPCode_schedAdmin` (`src/controller/cluster.c`) and `xceadmin scheduler record start|stop|status` / `scheduler dump `. - **Interactive REPL** (`repl.c`/`repl_break.c`/`repl_info.c`/`repl_params.c`, `--interactive`) — single-step/breakpoint/state-inspection/snapshot-capture control over the simulation driver; drives the exact same `sched_sim_driver_run()` as batch `--sim`, gated only by `sim->interactive`, so REPL and batch runs are decision-equivalent by construction (`test_repl_equivalence.c`).