Retypes sched_make_allocation()/sched_make_primary_allocation()/ sched_make_compound_allocation()/sched_make_alter_allocation()/ sched_commit_decision(), plus 3 private helpers found necessary during migration (sched_make_primary_alloc_(), sched_make_seqjob_ allocation(), sched_make_parjob_allocation()) from xce_scheduler_t* to xce_sched_core_t* in scheduler.c. Wires the one genuine normal-mode-only touch point -- the unsatisfiable-resreq cache, inside sched_make_primary_alloc_() -- through the task-1.1 NULL-safe callback seam (core->unsatisfiable_ lookup_cb/populate_cb) instead of a direct field read. No other normal-mode-only state was found in any of the 8 migrated functions, independently confirmed by both the implementer and reviewer against xce_scheduler_t's own field list -- no escalation needed. Minimal &(sched->core) call-site wraps applied inside sched_allocator()/ commit_decision() (task 3.1's home, not yet migrated), matching the pattern established in tasks 1.2/1.3/2.1. TDD: two-part RED -- a compile-error RED from the sched_commit_decision retype, and a behavioral RED using spy callbacks proving the cache seam was genuinely unconsulted before this fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RLSNJhNoULvVcUhXEFafQC
15 KiB
Implementation Plan: scheduler-sim-live-allocation
-
1. Foundation: independent core-safe seams and leaf migrations
-
1.1 (P) Give the allocation dispatch chokepoint an optional, normal-mode-only performance cache seam
- Add two NULL-by-default callback fields to the shared core state for consulting and populating the unsatisfiable-resreq cache, mirroring the existing session-reset callback's proven shape (a bare function pointer taking the core state directly, no wrapper struct, no opaque handle)
- Implement the two adapter functions against the real cache, in the same file the existing session-reset adapter already lives in, registered only during normal-mode startup
- TDD: add a test asserting that with both callbacks left NULL, the allocation dispatch chokepoint always computes fresh with no crash and no incorrect skip; add a normal-mode test confirming the cache is still consulted/populated exactly as before this change
- Observable: a simulation-mode session (which never registers these callbacks) never touches the real cache implementation at all, while a normal-mode session's cache behavior is provably unchanged
- Requirements: 5.2
- Boundary: Unsatisfiable-Cache Callback Seam
-
1.2 (P) Make plugin hook dispatch driver-agnostic and give every plugin an honest reference to shared core state
- Re-point the three plugin-hook dispatch functions (job election, prepare-allocation, commit-decision) to iterate the shared plugin registry directly instead of deriving it from a normal-mode-only driver struct, mirroring the session-begin/session-end dispatchers' own already-completed migration
- Add a new, honestly-typed core-state reference field to the plugin instance struct, set by BOTH existing plugin-registration entry points (the normal-mode one and the simulation-mode one) — leave the plugin's existing, differently-typed driver-reference field completely untouched, since an unrelated legacy picker plugin still depends on its current meaning
- TDD: extend the existing plugin-hook dispatch tests to run unchanged against a core-state fixture built either way (embedded in a normal-mode-shaped or simulation-mode-shaped container), and add a test confirming a plugin registered via the simulation-mode entry point has a non-NULL core-state reference (closing today's gap where it silently stays unset)
- Observable: a plugin registered through either registration path can reach shared core state without ever reading the normal-mode-only field, and the existing dispatch behavior (each registered plugin's hook invoked exactly once, in registration order) is unchanged
- Requirements: 1.1, 2.1, 2.2, 5.1
- Boundary: Plugin Hook Dispatchers
-
1.3 (P) Make the per-host and shared capacity checks driver-agnostic
- Re-type the host-selection match, shared-capacity check, and per-host capacity check functions to operate on shared core state directly instead of a normal-mode-only driver struct
- TDD: re-run their existing unit tests unchanged (pure signature migration) plus a new test calling each function with core state drawn from a simulation-mode-shaped container, confirming identical results to the normal-mode-shaped case
- Observable: these three functions compile and pass identically regardless of which driver's core state they're given
- Requirements: 1.3, 3.1, 3.2
- Boundary: Core-Safe Allocation Chain
-
2. Core: driver-agnostic allocation search and dispatch
-
2.1 Thread shared core state through the per-job allocation context and the per-host allocation attempt
- Change the per-job allocation context's driver-reference field to carry shared core state instead of a normal-mode-only pointer, updating its handful of existing cast sites
- Re-type the per-host allocation-attempt function (which calls the three capacity checks from 1.3 and the prepare-allocation hook from 1.2) to take shared core state directly
- Thread the same change through the existing multi-host allocation search's entry points, preserving their host-iteration and bucket/plan-construction algorithm exactly as-is
- TDD: re-run the existing multi-host allocation and per-host-attempt test suites unchanged; add a test driving the per-host attempt function with simulation-mode-shaped core state end to end
- Observable: the per-host allocation attempt and the multi-host search both run correctly when given either driver's core state, with zero change to which hosts get selected or in what order
- Depends: 1.2, 1.3
- Requirements: 1.3, 3.1, 3.2
- Boundary: Core-Safe Allocation Chain
-
2.2 Make primary/compound/alter allocation dispatch and decision commit driver-agnostic
- Re-type the top-level allocation dispatch switch and its primary/compound/alter sub-functions, and the decision-commit function, to take shared core state directly
- Wire the one genuine normal-mode-only touch point (the unsatisfiable-resreq cache) through the seam built in 1.1 instead of reading it directly
- Confirm by reading each sub-function's body before migrating it that it touches nothing beyond core state, worker/context fields, and the one cache touch point. If a sub-function is found to touch additional normal-mode-only state, stop and split that state's seam into a new follow-up task (mirroring 1.1's pattern) rather than designing and building it within this task
- TDD: re-run the existing allocation-dispatch and decision-commit test suites unchanged; add a normal-mode test confirming the cache fast path still activates correctly through the new seam
- Observable: a job's dispatch decision and resulting resource-accounting state are identical before and after this migration, for every existing allocation-dispatch test
- Depends: 2.1, 1.1
- Requirements: 1.1, 1.3, 3.1, 3.2, 5.2
- Boundary: Core-Safe Allocation Chain, Unsatisfiable-Cache Callback Seam
-
2.3 (P) Make global and host-scoped resource-limit matching driver-agnostic
- Change the limit plugin's global-limit and host-scoped-limit matching functions to read the new honest core-state reference from 1.2 instead of deriving anything from the plugin's normal-mode-only field
- Also address the confirmed-dead, type-confused write at plugins/limit.c:319 (
ctx.sched = plugin->sched;, flagged during task 2.1's review — never read back today, but stores anxce_scheduler_t*-shaped value into a field that is now genuinelyxce_sched_core_t*-typed): either populate it honestly from the plugin's new core-state reference, or remove the dead write entirely if nothing in the migrated code path needs it - Keep the existing plugin-hook-registered entry points' external signature unchanged; have them resolve core state from the new field and delegate to the now-driver-agnostic matching logic
- TDD: extend the limit plugin's existing test suite with a case that registers the plugin via the simulation-mode entry point and confirms a match still succeeds and still registers the matched limit's usage, where today it would silently no-op
- Observable: a limit configured on a job registered via the simulation-mode plugin path is matched and its usage instance created, verifiable by reading the plugin's own configured-limits/usage-instances accessors
- Depends: 1.2
- Requirements: 2.1, 2.2, 2.3, 2.4
- Boundary: Limit Plugin Core-Safe Matching
-
3. Core: unify job election and allocation into one implementation
-
3.1 Extract the job-picking and allocation loop into a single implementation shared by both drivers
- Move the job-picking thread body and the allocation thread body out of the normal-mode-only file they live in today, into their own file, re-typed to operate on shared core state directly
- Confirm every call this pair makes is already driver-agnostic after tasks 1.2/2.1/2.2 — no remaining reference to anything normal-mode-only
- Update normal mode's own startup to spawn these (now-relocated) threads unchanged in behavior
- TDD: re-run the full existing normal-mode scheduling test suite unchanged (behavior-preserving relocation); add a test spawning both threads against a simulation-mode-shaped core state fixture and confirming a job flows all the way from being picked to a committed decision
- Observable: normal mode's own scheduling behavior is bit-for-bit unchanged, and the identical thread pair now also runs correctly when spawned against simulation-mode's own core state in a test fixture
- Depends: 2.2, 2.3
- Requirements: 1.1, 1.2, 5.1
- Boundary: Unified Picker/Allocator Threads
-
4. Integration: wire simulation mode onto the shared implementation
-
4.1 Replace simulation mode's placeholder picker with the shared picker/allocator pair
- Replace simulation mode's own always-pend placeholder thread with the shared job-picking and allocation thread pair from 3.1, spawned against simulation mode's own core state
- Confirm the existing one-job-at-a-time hand-off guarantee between picking and allocating still holds now that simulation mode genuinely uses it for the first time
- TDD: write a new fixture using a non-stack-local (static or heap-allocated) simulation driver instance — a prior, still-recurring bug class in this exact area came from a stack-local instance outliving a never-joined thread, and this change doubles the number of such threads, so this must be positively confirmed, not assumed safe by analogy
- Observable: a simulation-mode session, driven from a real replay with no changes to the replay format, produces a genuine dispatch or pend decision for a job based on its actual resource availability, instead of unconditionally pending it
- Depends: 3.1
- Requirements: 1.1, 1.2, 3.3, 4.2, 4.3, 5.1
- Boundary: Unified Picker/Allocator Threads, Sim Driver
-
4.2 Narrow recorded-decision reconciliation to the timing-gap safety net
- Add a call to the now-driver-agnostic job-election dispatch (from 1.2) for a job this mechanism reconstructs from the recording, so a job whose outcome comes from the recording still gets its global-limit usage registered
- Confirm the existing trigger guard (a job already decided is never reconsidered) needs no change, since it already excludes any job the live path from 4.1 has already handled
- TDD: extend the existing reconciliation test suite with a case confirming a reconciled job's global limit is registered, and a case confirming a job the live path already decided is never reprocessed by this mechanism
- Observable: replaying a recorded session in which a job's dispatch was found only via reconciliation still shows that job's limit usage through the existing limit-usage accessors
- Depends: 4.1, 2.3
- Requirements: 3.4, 3.5
- Boundary: Sim-Mode Reconciliation Narrowing
-
5. Validation: prove equivalence and the concrete acceptance test
-
5.1 Prove identical decisions and accounting state under resource contention across both drivers
- Build a resource-contention fixture (multiple jobs, capacity insufficient for all of them, including capacity constrained by a matching limit) and drive it through both drivers
- Observable: both drivers produce the same admission/pend outcome and the same final resource-accounting state for the identical input
- Depends: 4.1
- Requirements: 3.1, 3.2
-
5.2 Prove the concrete acceptance test: real global and host-scoped limit usage visible against a real replay
- Drive a real simulation session through a replay whose original live session had both a global and a host-scoped limit actively enforced
- Observable: the existing limit-inspection command displays real, non-zero usage for both limits against this replay, where today it reports no active usage instance for either
- Depends: 4.1, 4.2
- Requirements: 2.1, 2.2, 2.3, 2.4
-
5.3 Re-run existing simulation-mode equivalence suites and reconcile any behavior change
- Re-run the REPL/batch decision-equivalence suite and the existing normal-mode-recording-vs-simulation-replay decision-identity suites in full
- Investigate and explain, rather than silently update, any assertion whose expected output changes as a result of this feature
- Observable: all three suites pass, or every changed assertion has a written explanation of why the new behavior is correct
- Depends: 4.1
- Requirements: 4.1, 4.2, 4.3
-
5.4 Validate against real recorded sessions and root-cause any divergence found
- Run live-vs-recorded verification against a range of real recorded sessions, not only crafted fixtures
- Treat any reported mismatch between a live-computed decision and its recorded counterpart as a finding to root-cause (either a defect in this feature's own migration, or a genuine issue the old always-pend behavior could never have exposed), never as something to suppress
- Observable: a written disposition (fixed, or explained as a pre-existing issue) exists for every mismatch found this way
- Depends: 5.1, 5.2, 5.3
- Requirements: 3.3, 3.5
Implementation Notes
- Task 1.1: initial submission implemented before tests were written (TDD-mandate violation, caught in review round 1) — the real RED phase must be captured by reverting production files to HEAD via a saved patch +
git checkout --, rebuilding to see the genuine failure, then reapplying. Retroactively-written tests plus a passing GREEN build alone do not satisfy this project's TDD discipline; the reviewer independently re-derives RED by reverting your changes, so skipping this is always caught. - Task 1.3: grep for ALL call sites of a function before migrating it, not just the ones named in the task brief —
host_iter.c'ssched_host_iter_next()andparjob_allocator.c/seqjob_allocator.c'smy_alloc()all call the capacity-check functions directly, not only viasched_try_alloc_on_host_(). A privatestatichelper with no other callers besides the functions being migrated should be migrated alongside them (leaving it stale forces an ugly cast, which this codebase's own design explicitly forbids in shared code). - Task 1.2: retyping a widely-called function's signature (e.g.
plugin_job_election,plugin_prepare_allocation,plugin_commit_decision) breaks ALL of its call sites as hard compile errors in one step (this toolchain treats-Wincompatible-pointer-typesas an error, not a warning) — there is no way to migrate a signature and keep the build green without touching every call site in the same commit, even when those call sites live in files nominally owned by a later task. The correct scope is the minimal fix (wrap the existing argument, e.g.sched→&(sched->core); touch nothing else in that function), not a rewrite of the caller. Also: design.md's Implementation Notes saidplugin_prepare_allocation()has "one normal-mode call site" — it actually has two (resource.c'ssched_try_alloc_on_host_()andschedobj.c'ssched_ctx_set_primary()). Grep for ALL call sites before assuming a design doc's count is exhaustive.