diff --git a/.kiro/specs/scheduler-allocator-runner/brief.md b/.kiro/specs/scheduler-allocator-runner/brief.md index 46a2bc05..bad41c99 100644 --- a/.kiro/specs/scheduler-allocator-runner/brief.md +++ b/.kiro/specs/scheduler-allocator-runner/brief.md @@ -33,7 +33,11 @@ Driver: `issues/new_requirements.md` item 4. - One framework function owns shared-resource prep, the bucket -> host-claim loop with rollback, and the commit tail. - Allocation decisions are bit-for-bit identical to today for every existing - test and sim-parity fixture. + test and sim-parity fixture. Intentional differences, all outside the + decision and main pend reason: pending seqjob jobs may gain per-host + `NO_RESOURCE` reasons (Constraints item 1); spurious shared reasons are + dropped (bug-1893, item 6); seqjob gets the `-EINVAL` guard and OOM + rollback (Req 5 AC 1/5). - `new_requirements.md` item 4 closed. ## Approach @@ -60,7 +64,8 @@ goal that plugins no longer export `my_alloc()`). - **In**: new allocator iface + framework runner; shared prep/try-plan/ finalize helpers; delete both `my_alloc()`s and parjob's claim helpers; retype `xce_alloc_plan_iter_t.init` to `xce_sched_core_t*`; equivalence - tests; update `issues/new_requirements.md` item 4. + tests; the bug-1893 loop-bound fix (Constraints item 6); update + `issues/new_requirements.md` item 4. - **Out**: see Out of Boundary. ## Boundary Candidates @@ -76,10 +81,16 @@ goal that plugins no longer export `my_alloc()`). - Collapsing `sched_make_seqjob_allocation` / `sched_make_parjob_allocation` dispatch in `scheduler.c` (optional, only if trivially falls out). - The `commit` override hook. +- bug-1892 (`unsatisfy_()` reads the index-keyed uncommitted vector by list + position; wrong admission for later primaries of COMPOUND/ALTER jobs). It + lives in `resource.c`'s satisfy checks, which the runner calls unchanged; + fix it separately. Equivalence tests for COMPOUND/ALTER must not encode its + wrong outcomes as expected values. - The `FIXME: no duration now` (NULL duration to `sched_make_allocs`): carry it over unchanged, don't fix it here. - Any change to plan ordering, host ordering, pend-reason content, or - preemption. + preemption, except the decided seqjob host-reason change in + Constraints item 1. ## Upstream / Downstream - **Upstream**: `scheduler-allocation` (plan/host iterators, Req 4 @@ -102,25 +113,72 @@ goal that plugins no longer export `my_alloc()`). - Behavioral equivalence is mandatory. Preserve exactly: bug-1882 (half-built alloc never left on `ctx->alloc`), bug-1767 (parjob `maxTasks <= 1` returns 0 with no fabricated reason, and before the lazy reason is - created), bug-1768; `-ENOMEM` cleanup order of both current tails. + created), bug-1768; `-ENOMEM` cleanup order of both current tails, plus + the seqjob OOM rollback of item 2 (Req 5 AC 5). - Divergences found in discovery that the design MUST resolve explicitly (they are not in the 2026-09-23 proposal): - 1. parjob does `xvec_idval_fini/init(&ctx->alloc_req.host)` per bucket - (`parjob_allocator.c:452`); seqjob never resets it. Decide whether the - shared loop always resets, and prove it is a no-op for seqjob. - 2. seqjob builds its halloc directly from `ctx->alloc_req.host`; under the - shared per-claim snapshot it gains a new `-ENOMEM` path. It must go - through the same rollback (`sched_ctx_uncommit_host`). - 3. seqjob does not set `ctx->nslots/nhosts` in its loop. Its values come - from `seqjob_prepare_allocation` (1,1). `sched_try_alloc_on_host_()` - (`resource.c`) does not read them directly; check which hook does, and - that `bucket_slots[0] == 1` for seqjob. - 4. seqjob has no `maxTasks` guard today. A `seqjob_accepts` - (`maxTasks == 1`) changes behaviour for any direct caller with another - value (several testsuite files reference `seqjob_allocator`). Either - make `accepts` NULL for seqjob or audit those callers. + 1. **Decided 2026-09-29: the shared loop resets `ctx->alloc_req.host` to + empty before every bucket's host scan, for both allocators.** Background: + the per-host request used by the authoritative check is already rebuilt + for every candidate (`sched_try_alloc_on_host_()` -> `plugin_prepare_ + allocation()` with `ctx->worker` set resets and refills it, + `plugin.c:639`). The per-bucket reset only affects the host iterator's + pre-filter (`host_iter.c:143`), which reads `alloc_req.host` before that + rebuild and silently skips hosts that fail. parjob already resets + (`parjob_allocator.c:479`), so its pre-filter does no resource filtering. + seqjob does not, so its pre-filter reads stale content: the job-level + `slots:1` for the first candidate, then the last rejected candidate's + full request. Resetting for seqjob too drops the stale-state dependency + and makes seqjob's pre-filter a resource no-op, like parjob's. + **Accepted consequence (intentional change):** host choice is unchanged + (a host the stale pre-filter rejects also fails the authoritative check + while the per-host request is host-independent; where a limit makes it + host-specific, the stale filter could wrongly skip a host, which the + reset fixes). But a pending seqjob job now gets a `PREASON_NO_RESOURCE` + host/resource reason from the authoritative check + (`resource.c:575`) for every host that passes the select filter and + lacks resources, not only for hosts the stale pre-filter let through. + `main_reason` is unaffected (host/resource reasons go to + `res_based_reasons`, `lib/preason.c:147`), so sim parity + (`sim_verify_pend_main_reason_eq`) is unaffected; no testsuite file + asserts `PREASON_*` host reasons (checked 2026-09-29). + Also accepted (found in design review): seqjob now runs the per-host + prepare fan-out on those hosts too, so the limit plugin may instantiate + a host limit's virtual resource and set its value on the worker earlier + than before (decision-neutral, lazy creation moves earlier), at the cost + of one prepare + check per short host per pending seqjob attempt. + Rejected: keeping the reset parjob-only (preserves the stale-state + quirk in shared code); staging each bucket's real request before the + scan so the pre-filter filters correctly (changes parjob's pend + reasons too; a separate follow-up if wanted). + 2. **Decided (requirements Req 5 AC 5):** seqjob builds its halloc directly + from `ctx->alloc_req.host`; under the shared per-claim snapshot it gains + a new `-ENOMEM` path, which goes through the same rollback + (`sched_ctx_uncommit_host`) as parjob's. + 3. **Decided 2026-09-29: for seqjob, `ctx->nslots` and `ctx->nhosts` are + always 1.** Their only consumer is `sched_fill_limit_requests()` in + `plugin_prepare_allocation()` (`plugin.c:682` job-level pass, `:699` + per-host pass); `sched_try_alloc_on_host_()` does not read them. Today + seqjob sets 1/1 only in its job-level prepare pass + (`seqjob_allocator.c:92`) and relies on the values carrying into the + per-host passes. The shared loop sets `ctx->nslots = + plan->bucket_slots[b]` and `ctx->nhosts = 1` before every bucket's + host scan (as parjob does, `parjob_allocator.c:492`); seqjob's plan is + always one bucket of one slot (`seqjob_allocator.c:163`), so seqjob + sees exactly 1/1 in every pass. The seqjob plan iterator must keep + `nbuckets == 1` and `bucket_slots[0] == 1` (assert in tests). + 4. **Decided (requirements Req 5 AC 3): seqjob's `accepts` is NULL.** seqjob + has no `maxTasks` guard today, and a `seqjob_accepts` (`maxTasks == 1`) + would change behaviour for direct callers with another value (48 + testsuite files reference the allocators). 5. Proposal-sketch defects to avoid: `sched_ctx_primary == NULL` typo; `claims[plan->nbuckets]` VLA (heap-allocate or bound by `maxTasks`). + 6. **Decided 2026-09-29 (requirements review): the shared pend-reason loop + iterates the unmet list (`ctx->unmeet.count`), fixing bug-1893.** Both + current `my_alloc()`s loop to `ctx->alloc_req.shared.count` over + `ctx->unmeet`, whose tail is zero-filled, and add reasons naming + resource index 0. Moving the loop into the runner is the natural point + to fix it; main reason and decisions are unaffected. - Claim `host_req` snapshots are owned by the framework: fini on every exit path (success, bucket failure, OOM). - Verify with: `test_seqjob_*`, `test_parjob_*`, `test_seqjob_rewrap_equivalence`, diff --git a/.kiro/specs/scheduler-allocator-runner/design.md b/.kiro/specs/scheduler-allocator-runner/design.md new file mode 100644 index 00000000..c49a13ec --- /dev/null +++ b/.kiro/specs/scheduler-allocator-runner/design.md @@ -0,0 +1,429 @@ +# Design Document: scheduler-allocator-runner + +## Overview +**Purpose**: The scheduler framework takes ownership of the job-allocation algorithm. One framework routine, `sched_allocator_run()`, performs the shared-resource precheck, the plan → bucket → host claim search with rollback, and result assembly for every allocator plugin. An allocator plugin supplies only a candidate-plan iterator, an optional acceptance guard, and its existing `prepare_allocation_hook`. + +**Users**: Scheduler plugin authors (a new allocator needs only a plan iterator) and scheduler maintainers (an allocation fix lands once). + +**Impact**: Deletes `my_alloc()` from `seqjob_allocator.c` and `parjob_allocator.c` and parjob's claim helpers; adds `alloc_runner.c`; adds an allocator iface to the plugin ABI; retypes the plan iterator's `init` third parameter from `xce_scheduler_t*` to `xce_sched_core_t*`. Allocation decisions, host choice, main pend reason and result codes are unchanged for every input that reaches the allocators today. The intentional changes are the four listed in `requirements.md` → Introduction; the side effects of the seqjob per-bucket reset are under System Flows → Consequences. + +### Goals +- One allocation algorithm, used by seqjob and parjob in normal and simulation mode (1.1, 6.3). +- Behavioural equivalence proven by characterization tests written against the current code before it is removed (7.2). +- `issues/new_requirements.md` item 4 closed (7.4). + +### Non-Goals +- A per-plugin result-assembly (`commit`) hook. +- Retyping `sched_make_allocs()` or removing `plugin->sched`. +- Merging `sched_make_seqjob_allocation()` / `sched_make_parjob_allocation()` dispatch. +- Adding an allocation duration (the `FIXME: no duration now` stays). +- Fixing bug-1892 (`unsatisfy_()` reads uncommitted amounts by position). + +## Boundary Commitments + +### This Spec Owns +- `xce_sched_allocator_iface_t` (shape, ownership, lifetime) and the `allocator_iface` field of `xce_sched_plugin_t`. +- `sched_allocator_run()` and its internal steps: input/acceptance guards, shared-resource precheck (incl. the bug-1893 loop-bound fix), claim search and rollback, per-bucket `alloc_req.host` reset and `nslots`/`nhosts` assignment, result assembly, cleanup of every attempt-owned resource. +- The `xce_alloc_plan_iter_t.init` parameter type. +- The seqjob/parjob plugin wiring (templates, iface instances, `parjob_accepts`) and the removal of their private allocation code. +- Equivalence and runner tests; the pend-reason factory audit's file list; `issues/new_requirements.md` item 4. + +### Out of Boundary +- Dispatch (`scheduler.c` `sched_make_primary_alloc_()` and the COMPOUND/ALTER callers): routing by `maxTasks`, the unsatisfiable cache, result merging, session stamping. +- `resource.c` satisfy checks (`sched_check_shared_satisfy`, `sched_check_host_satisfy`, `unsatisfy_`), `sched_try_alloc_on_host_()`, `plugin_prepare_allocation()`, the host iterator, and both plugins' `prepare_allocation_hook`s: called unchanged. +- The plan iterators' plan sequences (only the parameter type changes). +- `sched_make_allocs()`, `make_*_alloc` constructors in libxce, `sim_alloc.c` registration (comment updates only). +- Snapshot, WAL, recording formats: nothing persisted changes. + +### Allowed Dependencies +- `alloc_runner.c` → `resource.c` (`sched_try_alloc_on_host_`, `sched_check_shared_satisfy`, `sched_ctx_uncommit_host`), `schedobj.c` (`sched_ctx_get_uncomit_shared`), `host_iter.c` (`sched_host_iter_*`), `scheduler.c` (`sched_make_allocs` only), libxce (`make_pendreason`, `add_job_preason`, `make_host_alloc`, `make_job_alloc`, `free_*`, `xvec_*`). +- Plugins → `alloc_runner.c` only through `sched_allocator_run` and the iface type. +- `alloc_runner.c` must not reference any plugin file or plugin-specific symbol; plugins must not call runner internals. + +### Revalidation Triggers +- Any change to `xce_sched_allocator_iface_t` or `xce_alloc_plan_iter_t` signatures (all allocator plugins, `test_parjob_combinatorial.c`, `test_alloc_plan_iter.c`, `test_alloc_runner.c`'s fake iface). +- Any change to the order of guard → precheck → search → assembly, or to when `ctx->reason`/`ctx->alloc` are set (dispatch, unsatisfiable cache, COMPOUND merge, `scheduler-alter-pend-reasons`). +- Fixing bug-1892 (changes COMPOUND admission; re-run COMPOUND characterization tests). +- A new allocator plugin (must supply a plan iterator whose buckets are ≤ its task count). + +## Architecture + +### Existing Architecture Analysis +`sched_make_primary_alloc_()` picks `core->seqjob_allocator` (`maxTasks == 1`) or `core->parjob_allocator` and calls `plugin->job_alloc_fn(plugin, ctx)`. Each plugin's `my_alloc()` then repeats the whole algorithm. Plan generation already sits behind `xce_alloc_plan_iter_t`, host scanning behind `xce_host_iter_t`, per-host preparation behind `plugin_prepare_allocation()`. The picker side already uses a framework-owned iface (`picker_iface`) with a forward typedef in `plugin.h`. + +### Architecture Pattern & Boundary Map +Template method: the framework owns the algorithm; the plugin fills in plan generation. + +```mermaid +graph TB + Dispatch[scheduler dispatch] --> JobAllocFn[plugin job_alloc_fn] + JobAllocFn --> Runner[sched_allocator_run] + Runner --> Iface[allocator iface] + Iface --> SeqPlans[seqjob plan iterator] + Iface --> ParPlans[parjob plan iterator] + Iface --> ParAccepts[parjob accepts] + Runner --> SharedCheck[resource shared check] + Runner --> HostIter[host iterator] + Runner --> TryHost[try alloc on host] + TryHost --> Prepare[plugin prepare allocation] + Prepare --> SeqPrep[seqjob prepare hook] + Prepare --> ParPrep[parjob prepare hook] + Runner --> MakeAllocs[sched make allocs] +``` + +- **Dependency direction**: libxce → `resource.c`/`schedobj.c`/`host_iter.c` → `alloc_runner.c` → plugins (via iface data only) → dispatch. Plugins reference `sched_allocator_run` by address in their template; the runner never names a plugin. One sanctioned exception: the runner calls `scheduler.c`'s `sched_make_allocs()` (a leftward call into the dispatch file), exactly as both `my_alloc()`s do today; removing it is the out-of-boundary `sched_make_allocs()` retype follow-up. No other `scheduler.c` symbol may be used. +- **Existing patterns preserved**: static iface storage referenced from the plugin struct (picker); white-box test `#include` of `.c` files; `sched_make_allocs()` stubbed in white-box tests. +- **Steering compliance**: new file under the 800-line cap; plugins stay under `src/plugins/`; no I/O, no locks, no blocking added (allocator thread only). + +### Technology Stack + +| Layer | Choice / Version | Role in Feature | Notes | +|-------|------------------|-----------------|-------| +| Scheduler daemon | C (gnu17), Meson | Runner, iface, plugin wiring | No new dependency | +| Tests | Unity, white-box `#include` | Characterization + runner tests | Existing convention | + +## File Structure Plan + +### Directory Structure +``` +src/scheduler/ +├── include/ +│ ├── plugin.h # + forward typedef xce_sched_allocator_iface_t; + allocator_iface field +│ └── sched/alloc_iter.h # plan iterator init retyped; + xce_sched_allocator_iface_t; + sched_allocator_run prototype +├── src/ +│ ├── alloc_runner.c # NEW: sched_allocator_run and its static steps +│ ├── sim_alloc.c # comments only (my_alloc references) +│ └── plugins/ +│ ├── seqjob_allocator.c # - my_alloc; + seqjob_alloc_iface; template -> sched_allocator_run +│ └── parjob_allocator.c # - my_alloc, claim helpers; + parjob_accepts, parjob_alloc_iface +├── meson.build # + src/alloc_runner.c in scheduler_srcs +└── testsuite/ + ├── test_alloc_runner.c # NEW: runner tests with an in-test fake plan iterator + ├── test_alloc_equivalence_seqjob.c # NEW: seqjob characterization tests via job_alloc_fn + ├── test_alloc_equivalence_parjob.c # NEW: parjob characterization tests via job_alloc_fn + └── meson.build # + three targets +``` + +### Modified Files +- `src/scheduler/include/sched/alloc_iter.h` — `init`'s third parameter becomes `xce_sched_core_t* core`; defines `xce_sched_allocator_iface_t` and declares `sched_allocator_run`. +- `src/scheduler/include/plugin.h` — forward typedef and `const xce_sched_allocator_iface_t* allocator_iface;` in `xce_sched_plugin_t`, documented with the same ownership note as `picker_iface`. +- `src/scheduler/src/plugins/seqjob_allocator.c` — plan-iterator `init` signature; delete `my_alloc()`; add `static const xce_sched_allocator_iface_t seqjob_alloc_iface` (`accepts = NULL`); template `.job_alloc_fn = sched_allocator_run`, `.allocator_iface = &seqjob_alloc_iface`; update comments. +- `src/scheduler/src/plugins/parjob_allocator.c` — same, plus `parjob_accepts()` (`maxTasks > 1`); delete `parjob_claim_t`, `parjob_is_claimed_`, `parjob_snapshot_host_req_`, `parjob_rollback_claims_`, `parjob_try_plan_`. +- `src/scheduler/src/sim_alloc.c` — comment text only. +- `src/scheduler/meson.build` — add `src/alloc_runner.c`. +- `src/scheduler/testsuite/test_seqjob_allocator.c`, `test_seqjob_rewrap_equivalence.c`, `test_parjob_combinatorial.c` — add `#include "../src/alloc_runner.c"`; replace direct `my_alloc()`/`parjob_try_plan_()`/`parjob_rollback_claims_()` calls with `sched_allocator_run()`/`sched_try_plan_()`/`sched_rollback_claims_()`; plan-iterator `init` third argument becomes a core pointer. +- `src/scheduler/testsuite/test_alloc_plan_iter.c` — its `stub_plan_iter_init` third parameter becomes `xce_sched_core_t*` (plan-iterator retype). +- `src/scheduler/testsuite/test_picker_pendreason_factory_audit.c` — add `src/alloc_runner.c` to the scanned list. +- `src/scheduler/testsuite/meson.build` — three new targets. +- `issues/new_requirements.md` — item 4 marked resolved. + +## System Flows + +```mermaid +flowchart TD + Start[sched_allocator_run] --> Inputs{inputs present} + Inputs -->|no| EInval[return EINVAL] + Inputs -->|yes| Accept{accepts} + Accept -->|rejected| Pend0[return 0] + Accept -->|accepted or NULL| Reason[ensure pend reason record] + Reason --> Shared{shared request met} + Shared -->|error| Err[return error] + Shared -->|no| SharedReasons[one reason per unmet entry then return 0] + Shared -->|yes| Uncommit[count shared as uncommitted] + Uncommit --> Init{plan init ok} + Init -->|no| Pend1[return 0] + Init -->|yes| Next{next plan} + Next -->|none| Fini0[fini and return 0] + Next -->|plan| Grow{claims buffer fits} + Grow -->|grow fails| FiniOom[fini and return ENOMEM] + Grow -->|malformed plan| Next + Grow -->|ok| TryPlan[try plan] + TryPlan -->|bucket unsatisfied| Next + TryPlan -->|snapshot OOM| FiniErr[fini and return error] + TryPlan -->|all buckets claimed| Finalize[build result] + Finalize -->|OOM| FiniOom2[fini and return ENOMEM] + Finalize -->|ok| Done[publish ctx alloc then fini and return 1] +``` + +**Try plan** (per bucket, in order): reset `alloc_req.host` to empty → new host iterator → for each candidate: skip if already claimed in this plan; set `nslots = bucket_slots[b]`, `nhosts = 1`; `sched_try_alloc_on_host_() > 0` → snapshot `alloc_req.host` into the claim (snapshot OOM: uncommit the candidate with the live request, roll back earlier claims, return the error) → next bucket. A bucket with no host rolls back every claim of the plan and returns 0. + +**Key decisions**: the reset and the `nslots`/`nhosts` assignment are unconditional (brief items 1 and 3); every return after `plan_init` succeeded calls `plan_fini` and frees the claims buffer; claim snapshots are freed by whichever function owns them at that point (see AllocRunner → State Management). + +## Requirements Traceability + +| Requirement | Summary | Components | Interfaces | Flows | +|-------------|---------|------------|------------|-------| +| 1.1 | One routine for both plugins | AllocRunner, PluginWiring | `sched_allocator_run` | Start | +| 1.2 | Plugin supplies plan iterator, accepts, prepare hook only | AllocatorIface | `xce_sched_allocator_iface_t` | — | +| 1.3 | NULL accepts = accept all | AllocRunner | `accepts` | Accept | +| 1.4 | Rejected = pending, no reason, no checks | AllocRunner | `accepts` | Accept → Pend0 | +| 1.5 | No private allocation routine in plugins | PluginWiring | — | — | +| 1.6 | Iface static, not freed, plugin lifetime | AllocatorIface, PluginWiring | `allocator_iface` | — | +| 2.1 | Reason record created after guards | AllocRunner (`ensure_reason`) | — | Reason | +| 2.2 | One reason per unmet entry (bug-1893) | AllocRunner (`prepare_shared`) | — | SharedReasons | +| 2.3 | Shared counted once, not released | AllocRunner (`prepare_shared`) | — | Uncommit | +| 2.4 | Prep failures return their error | AllocRunner | — | Err | +| 3.1 | Plans in generator order, stop at first satisfied | AllocRunner | `plan_next` | Next | +| 3.2 | Buckets in order, first unclaimed satisfying host | ClaimSearch | `sched_host_iter_*` | TryPlan | +| 3.3 | Roll back claims on unsatisfied bucket | ClaimSearch | `sched_ctx_uncommit_host` | TryPlan | +| 3.4 | Init failure / no plan = pending | AllocRunner | `plan_init` | Pend1, Fini0 | +| 3.5 | `nslots` = bucket slots, `nhosts` = 1 | ClaimSearch | — | TryPlan | +| 3.6 | Empty per-host request per bucket | ClaimSearch | — | TryPlan | +| 3.7 | seqjob per-host reasons complete | ClaimSearch (via 3.6) | `sched_try_alloc_on_host_` | TryPlan | +| 3.8 | No fixed bucket limit | AllocRunner (claims buffer) | — | Grow | +| 4.1 | One host alloc per claim, claim order, snapshot request | ResultAssembly | `sched_make_allocs`, `make_host_alloc` | Finalize | +| 4.2 | Shared record per shared-request entry | ResultAssembly | `sched_make_allocs` | Finalize | +| 4.3 | jobid 0, empty reserve, no duration | ResultAssembly | `make_job_alloc(0, …)` | Finalize | +| 4.4 | Satisfied → set alloc, return 1 | AllocRunner | — | Done | +| 4.5 | Results identical to current allocators | ResultAssembly, EquivalenceTests | — | — | +| 5.1 | Missing inputs → EINVAL, no reason | AllocRunner | — | EInval | +| 5.2 | parjob `maxTasks <= 1` → pending | ParjobAccepts | `parjob_accepts` | Pend0 | +| 5.3 | seqjob accepts all | PluginWiring (`accepts = NULL`) | — | Accept | +| 5.4 | OOM in result → ENOMEM, nothing left | ResultAssembly | — | FiniOom2 | +| 5.5 | OOM releases unconverted claims + failed candidate | ClaimSearch, ResultAssembly | `sched_rollback_claims_` | FiniErr, FiniOom2 | +| 5.6 | `ctx->alloc` set only on success | ResultAssembly | — | Done | +| 5.7 | Fini iterator and free snapshots on every exit | AllocRunner | `plan_fini` | all Fini nodes | +| 5.8 | Size from each primary's own request | AllocRunner (uses `ctx->primary` / `ctx->alloc_req` only) | — | — | +| 6.1 | Plan init takes core | PlanIterContract | `init(iter, resreq, core)` | Init | +| 6.2 | Identical plan sequences | PlanIterContract, EquivalenceTests | — | — | +| 6.3 | Sim uses the same routine, no scheduler object to plan iterators | PluginWiring, PlanIterContract | — | — | +| 7.1 | Existing tests pass, limited test edits | TestMigration | — | — | +| 7.2 | Equivalence tests | EquivalenceTests, RunnerTests | — | — | +| 7.3 | Factory audit covers runner | TestMigration | — | — | +| 7.4 | Item 4 resolved | Closure | — | — | +| 7.5 | Invariant audit clean | Closure | — | — | + +## Components and Interfaces + +| Component | Domain/Layer | Intent | Req Coverage | Key Dependencies | Contracts | +|-----------|--------------|--------|--------------|------------------|-----------| +| AllocatorIface | Plugin ABI (`alloc_iter.h`, `plugin.h`) | Declares what an allocator plugin supplies | 1.2, 1.3, 1.6 | — | Service | +| PlanIterContract | Plugin ABI (`alloc_iter.h`) | Plan iterator `init` takes the core | 6.1, 6.2, 6.3 | — | Service | +| AllocRunner | Framework (`alloc_runner.c`) | Guards, shared precheck, plan loop, cleanup | 1.1, 1.3, 1.4, 2.x, 3.1, 3.4, 3.8, 4.4, 5.1, 5.7, 5.8 | resource.c (P0), schedobj.c (P0) | Service, State | +| ClaimSearch | Framework (`alloc_runner.c`) | Bucket → host claim, snapshot, rollback | 3.2, 3.3, 3.5, 3.6, 3.7, 5.5 | host_iter.c (P0), resource.c (P0) | Service | +| ResultAssembly | Framework (`alloc_runner.c`) | Build and publish `xce_job_alloc_t` | 4.1–4.5, 5.4–5.6 | `sched_make_allocs` (P0), libxce (P0) | Service | +| PluginWiring | Plugins (`seqjob_allocator.c`, `parjob_allocator.c`) | Templates + iface instances; delete private code | 1.1, 1.5, 1.6, 5.2, 5.3, 6.3 | AllocRunner (P0) | — | +| ParjobAccepts | Plugin (`parjob_allocator.c`) | `maxTasks > 1` guard | 5.2 | — | Service | +| EquivalenceTests / RunnerTests / TestMigration / Closure | Tests, docs | Evidence and closure | 4.5, 6.2, 7.1–7.5 | — | — | + +### Plugin ABI + +#### AllocatorIface and PlanIterContract + +| Field | Detail | +|-------|--------| +| Intent | The allocator plugin contract and the retyped plan-iterator initialiser | +| Requirements | 1.2, 1.3, 1.6, 6.1, 6.2, 6.3 | + +**Contracts**: Service [x] + +##### Service Interface +```c +/* include/sched/alloc_iter.h */ +struct xce_alloc_plan_iter_s { + int (*init)(xce_alloc_plan_iter_t* iter, xce_primary_resreq_t* resreq, + xce_sched_core_t* core); /* was xce_scheduler_t* */ + xce_alloc_plan_t* (*next)(xce_alloc_plan_iter_t* iter); + void (*fini)(xce_alloc_plan_iter_t* iter); + void* private; +}; + +struct xce_sched_allocator_iface_s { + /* Optional. 1 = this plugin allocates `primary`, 0 = not (pending, no + * reason, no checks). NULL = accept every request. */ + int (*accepts)(const xce_primary_resreq_t* primary); + /* Required. Same contract as xce_alloc_plan_iter_t init/next/fini. */ + int (*plan_init)(xce_alloc_plan_iter_t* iter, xce_primary_resreq_t* resreq, + xce_sched_core_t* core); + xce_alloc_plan_t* (*plan_next)(xce_alloc_plan_iter_t* iter); + void (*plan_fini)(xce_alloc_plan_iter_t* iter); +}; + +/* job_alloc_fn for every allocator plugin; `plugin` is xce_sched_plugin_t*. + * > 0 satisfied (ctx->alloc set), 0 pending, < 0 negative errno. */ +extern int sched_allocator_run(void* plugin, xce_sched_ctx_t* ctx); + +/* include/plugin.h */ +typedef struct xce_sched_allocator_iface_s xce_sched_allocator_iface_t; +/* in xce_sched_plugin_t: static storage owned by the plugin file; the + * registry never frees it (same ownership as picker_iface). NULL for + * non-allocator plugins. */ +const xce_sched_allocator_iface_t* allocator_iface; +``` +- **Preconditions**: for an allocator plugin, `allocator_iface` and its three `plan_*` members are non-NULL; every plan's `nbuckets >= 1` and each `bucket_slots[i] >= 1`. +- **Postconditions**: `plan_init` returns 0 and leaves `iter` ready for `plan_next`, or non-zero with nothing for the runner to release (the runner then skips `plan_fini`). It may read the core; both built-in iterators ignore it. +- **Invariants**: the iface is `static const` in the plugin file; plugin templates copy the pointer on registration (1.6). + +**Implementation Notes** +- Integration: seqjob/parjob iterator functions and `test_alloc_plan_iter.c`'s stub iterator change only their third parameter's type; `test_parjob_combinatorial.c`'s poisoned-scheduler case becomes a poisoned-core case (6.2 evidence: plan sequences equal with `NULL` and poisoned core). +- Risks: a plugin that sets `job_alloc_fn = sched_allocator_run` without an iface gets `-EINVAL` (5.1), not a crash. + +### Framework runner (`src/scheduler/src/alloc_runner.c`) + +#### AllocRunner + +| Field | Detail | +|-------|--------| +| Intent | The one allocation routine | +| Requirements | 1.1, 1.3, 1.4, 2.1–2.4, 3.1, 3.4, 3.8, 4.4, 5.1, 5.7, 5.8 | + +**Responsibilities & Constraints** +- Order: (1) input guard: `plugin`, `ctx`, `plugin->core`, `plugin->sched`, `ctx->primary`, `plugin->allocator_iface` and its `plan_*` members non-NULL, else `-EINVAL` before touching `ctx->reason` (5.1); (2) `accepts` if non-NULL, reject → 0 (1.3, 1.4); (3) `ensure_reason`: create `make_pendreason(0)` if `ctx->reason == NULL`, `-ENOMEM` on failure (2.1, 2.4); (4) `prepare_shared` (below); (5) `plan_init(iter, ctx->primary, plugin->core)`, non-zero → 0 without `plan_fini` (3.4, as today); (6) plan loop; (7) cleanup. +- `prepare_shared`: `sched_ctx_get_uncomit_shared`; `xvec_idval_fini(&ctx->unmeet)`; `sched_check_shared_satisfy(core, uncomit, &ctx->alloc_req.shared, &ctx->unmeet)`; `< 0` → return it; `== 0` → for `i < ctx->unmeet.count`: `vget(resmgr, unmeet.idxs[i])`, skip on `rc <= 0`, `add_job_preason(reason, PREASON_NO_RESOURCE, NULL, r->name)`; return 0 (2.2); `> 0` → `xvec_idval(uncomit, &ctx->alloc_req.shared, XVEC_ADD_FLOAT)` once, error → return it (2.3, 2.4). Shared amounts are never subtracted by the runner (2.3). +- Claims buffer: `sched_alloc_claim_t* claims` (heap), capacity grown with `realloc` to `plan->nbuckets` before a plan is tried; growth failure → `plan_fini`, free, `-ENOMEM` (3.8). No claim is outstanding between plans. +- Uses only `ctx->primary` and `ctx->alloc_req` for sizing (5.8). +- Precondition `ctx->alloc == NULL` on entry (the dispatch and COMPOUND/ALTER callers move or free any previous result before the next attempt); the runner never frees or overwrites a non-NULL `ctx->alloc` except with its own result on success. +- A plan with `nbuckets < 1` or any `bucket_slots[i] < 1` violates the iterator precondition; the runner skips it as unsatisfied (built-in iterators never produce one, so this changes no current outcome). + +**Contracts**: Service [x] / State [x] + +##### Service Interface +```c +int sched_allocator_run(void* plugin, xce_sched_ctx_t* ctx); + +/* static, reachable from white-box tests */ +static int sched_alloc_ensure_reason_(xce_sched_ctx_t* ctx); /* 0 | -ENOMEM */ +static int sched_alloc_prepare_shared_(xce_sched_core_t* core, + xce_sched_ctx_t* ctx); /* 1 met | 0 pend | <0 */ +``` + +##### State Management +- Attempt-owned: plan iterator private state and the claims buffer, released by the runner on every return path after `plan_init` succeeded (5.7). Claim snapshots are owned by exactly one function at a time: `sched_try_plan_()` creates them and frees them itself on 0 / `< 0`; on 1 it hands all of them to `sched_finalize_job_alloc_()`, which frees every one on every path. The runner itself never frees a snapshot, so no path can free one twice. +- Thread cancellation: `plan_next` (both iterators) and `sched_host_iter_next()` are `pthread_testcancel()` points, and the allocator thread is cancelled only at shutdown (`scheduler.c:905`, `sim.c:727`). A cancel inside an attempt abandons the attempt-scoped heap (iterator state, host-iterator state, claim snapshots, and now the claims buffer) exactly as today; the runner adds no `pthread_cleanup_push()` handlers, consistent with the rest of the scheduler, which has none. "Every return" in 5.7 means every return path, not an asynchronous cancel. +- Context-owned (not released by the runner): `ctx->reason`, shared uncommitted amounts, per-host debits of claims already converted into host allocations (5.5 wording), `ctx->alloc` on success. + +#### ClaimSearch + +| Field | Detail | +|-------|--------| +| Intent | Try one plan: claim a distinct host per bucket, or roll back | +| Requirements | 3.2, 3.3, 3.5, 3.6, 3.7, 5.5 | + +##### Service Interface +```c +typedef struct { + xce_sched_worker_t* w; + xvec_idval_t host_req; /* deep copy of ctx->alloc_req.host at claim */ +} sched_alloc_claim_t; + +/* 1 all buckets claimed (claims[0..*nclaimed) owned by caller) + * 0 a bucket had no host (all claims rolled back, snapshots freed) + * <0 snapshot failure (candidate + earlier claims rolled back, snapshots freed) */ +static int sched_try_plan_(xce_sched_core_t* core, xce_sched_ctx_t* ctx, + xce_alloc_plan_t* plan, sched_alloc_claim_t* claims, + int* nclaimed); +static void sched_rollback_claims_(xce_sched_ctx_t* ctx, + sched_alloc_claim_t* claims, int n); +static int sched_snapshot_host_req_(xvec_idval_t* dst, const xvec_idval_t* src); +``` +- **Preconditions**: `claims` has capacity `>= plan->nbuckets`. +- **Postconditions**: on 0 or `< 0`, every per-host debit made in this call is released (3.3, 5.5) and no snapshot remains. +- Behaviour is parjob's current `parjob_try_plan_()` exactly, with `plan->ctx = ctx`, the per-bucket reset (3.6) and the per-candidate `nslots`/`nhosts` assignment (3.5) now applied to every plugin. seqjob's complete per-host reasons (3.7) follow from 3.6: the pre-filter no longer skips hosts on stale state, so `sched_try_alloc_on_host_()` records its reasons for every selection-matching host. + +#### ResultAssembly + +| Field | Detail | +|-------|--------| +| Intent | Turn claims into the published `xce_job_alloc_t` | +| Requirements | 4.1–4.5, 5.4, 5.5, 5.6 | + +##### Service Interface +```c +/* 1: ctx->alloc set; -ENOMEM: ctx->alloc untouched, everything built freed. + * Consumes claims: each snapshot is freed once its host alloc is built; + * on failure, claims not yet converted are rolled back and freed. */ +static int sched_finalize_job_alloc_(xce_scheduler_t* sched, xce_sched_ctx_t* ctx, + sched_alloc_claim_t* claims, int nclaimed); +``` +- Steps, matching parjob's current tail: `calloc(nclaimed)` host-alloc array (failure: roll back all claims); per claim `sched_make_allocs(sched, "", nres, idxs, vals, NULL)` (`/* FIXME: no duration now */` kept) then `make_host_alloc(w->name, nres, allocs)`, where `nres == 0` allows a NULL `allocs` (failure: free `allocs`, roll back claims `[i, n)`, `free_HostAlloc_list(nclaimed, hallocs)`); `make_job_alloc(0, nclaimed, hallocs)` into a local (failure: `free_HostAlloc_list`); shared records `sched_make_allocs(sched, "", shared.count, …, NULL)` when `alloc_req.shared.count > 0`, else `allocs_shared = NULL, nres_shared = 0` (failure: `free_JobAlloc_ptr(&local)`); then `ctx->alloc = local` (5.6). +- `sched` is `(xce_scheduler_t*)plugin->sched` (the stand-in wrapper in sim), used only for `sched_make_allocs()` (out-of-boundary retype). +- Equivalence note (4.5): seqjob today builds its one-element host array with `make_host_allocs(1, halloc)`; the runner's `calloc` + store produces the same array, freed by the same `free_HostAlloc_list`. + +### Plugins + +#### PluginWiring and ParjobAccepts + +| Field | Detail | +|-------|--------| +| Intent | Register both plugins on the runner | +| Requirements | 1.1, 1.5, 1.6, 5.2, 5.3, 6.3 | + +```c +/* parjob_allocator.c */ +static int parjob_accepts(const xce_primary_resreq_t* primary) +{ return primary->maxTasks > 1; } /* bug-1767 */ +static const xce_sched_allocator_iface_t parjob_alloc_iface = { + .accepts = parjob_accepts, .plan_init = parjob_plan_iter_init, + .plan_next = parjob_plan_iter_next, .plan_fini = parjob_plan_iter_fini }; + +/* seqjob_allocator.c */ +static const xce_sched_allocator_iface_t seqjob_alloc_iface = { + .accepts = NULL, .plan_init = seqjob_plan_iter_init, + .plan_next = seqjob_plan_iter_next, .plan_fini = seqjob_plan_iter_fini }; + +/* both templates */ +.job_alloc_fn = (plugin_job_allocate_fn_t)sched_allocator_run, +.allocator_iface = &_alloc_iface, +``` +- `prepare_allocation_hook`, `init_fn`, `fini_fn`, `*_new_plugin()`, `register_*()` unchanged; sim registration in `sim_alloc.c` therefore reaches the runner unchanged (6.3). + +### Consequences of the per-bucket reset for seqjob (3.6) +Because seqjob's pre-filter no longer skips hosts on stale state, a pending seqjob attempt now runs `sched_try_alloc_on_host_()`, and so the full `plugin_prepare_allocation()` fan-out, on every selection-matching host that is short of resources, as parjob already does. Besides the per-host pend reasons (3.7), this means: +- **Limit plugin side effects** (`limit.c` `match_job_host_limit_`): for a host with a matching host-based limit it allocates the job's `obj->hlimits[worker]` list, may instantiate the limit's virtual resource in `resmgr` (`instantiate_limit`) and writes the limit value into `worker->resvals`. This is decision-neutral: any later check of that host by any job runs the same lazy instantiation before reading the value, so it only moves when the instance is created. It happens identically in normal and simulation mode (driver state-equivalence holds). +- **Cost**: one prepare fan-out plus satisfy check per short host per pending seqjob attempt, where the stale pre-filter used to reject some hosts with a single satisfy check. Repeated attempts for the same unsatisfiable request are still short-circuited by the unsatisfiable cache before the allocator runs (`scheduler.c:248`). + +## Data Models +No persisted or wire data changes. In-memory only: `sched_alloc_claim_t` (attempt-scoped) and the `allocator_iface` pointer (static). Snapshot/WAL/recording field-completeness is not affected (scheduler audit dimension 4: no change). + +## Error Handling + +### Error Strategy +The runner follows the allocator contract: `> 0` satisfied, `0` pending, `< 0` negative errno, early return on each error (project C convention), with cleanup of attempt-owned state on every path after `plan_init`. + +| Condition | Result | State left behind | +|-----------|--------|-------------------| +| Missing plugin/ctx/core/sched/primary/iface | `-EINVAL` | nothing; `ctx->reason` untouched | +| `accepts` rejects | 0 | nothing | +| `make_pendreason` fails | `-ENOMEM` | `ctx->reason == NULL` | +| Shared check error / uncommit add error | that error | reason record | +| Shared unmet | 0 | reason record with one `NO_RESOURCE` per unmet entry | +| `plan_init` fails | 0 | reason record, shared uncommitted | +| No plan satisfied | 0 | reason record (+ host reasons), shared uncommitted | +| Plan with `nbuckets < 1` or a bucket of `< 1` slots | plan skipped as unsatisfied | nothing from that plan | +| Claims buffer growth fails | `-ENOMEM` | as above; no host debit outstanding | +| Snapshot fails | its error (`-ENOMEM`) | candidate and earlier claims released | +| Host alloc build fails | `-ENOMEM` | unconverted claims released; converted ones' debits stay in ctx (as today) | +| Job alloc / shared records fail | `-ENOMEM` | `ctx->alloc` untouched; debits stay in ctx (as today) | + +### Monitoring +Existing `TRACE` lines in `resource.c` are unchanged; the runner adds none (the allocator path runs per candidate and is already traced there). + +## Testing Strategy + +TDD order follows the staged migration (research.md): characterization tests are written and pass against the current code first, then must pass unchanged after each switch. + +### Unit Tests +- `test_alloc_runner.c` (white-box `#include` of `core.c`, `resource.c`, `schedobj.c`, `host_iter.c`, `alloc_runner.c`; stubbed `sched_make_allocs` with OOM injection; stubbed `plugin_prepare_allocation()` that records `ctx->nslots`/`ctx->nhosts`/`ctx->alloc_req.host.count` on each call and stages a fixed per-host request, the same link-stub convention `test_resource_accounting.c` uses; an in-test fake iface whose plans are scripted): + - guards: each missing input → `-EINVAL`, `ctx->reason` still NULL (5.1); rejecting `accepts` → 0, no reason, `plan_init` not called (1.4); NULL `accepts` reaches `plan_init` (1.3). + - shared: two shared resources, one short → exactly one host-less `NO_RESOURCE` reason naming it (2.2, bug-1893); met → uncommitted added once across a 3-plan attempt (2.3). + - search: plan order and first-satisfied stop (3.1); distinct hosts per bucket (3.2); failing bucket 2 rolls back bucket 1's debit (`w` uncommitted back to 0) (3.3); `nslots/nhosts` seen by a recording prepare hook equal `bucket_slots[b]`/1 (3.5); `alloc_req.host` empty when each bucket's host scan starts, observed through a candidate whose stale request would have been rejected by the pre-filter (3.6); a plan with more buckets than a first small plan triggers buffer growth (3.8). + - errors: snapshot OOM, host-alloc OOM at claim 0 and claim k, job-alloc OOM, shared-records OOM → `-ENOMEM`, `ctx->alloc == NULL`, released debits as in the Error table, ASAN clean (5.4, 5.5, 5.6, 5.7). +- `test_parjob_combinatorial.c`: plan sequences identical for `NULL` and poisoned core (6.2). + +### Equivalence (characterization) Tests +- `test_alloc_equivalence_seqjob.c` / `test_alloc_equivalence_parjob.c` (white-box `#include` of `core.c`, `resource.c`, `schedobj.c`, `host_iter.c`, the plugin file, and from its switch step `alloc_runner.c`; `sched_make_allocs` stubbed with OOM injection; `plugin_prepare_allocation()` stubbed by the existing convention of `test_seqjob_allocator.c:84` — reset `alloc_req.shared` or `.host` like `plugin.c`, then call the plugin's own prepare hook — so limit fill is not exercised here; limits stay covered through the real `plugin.c` by `test_sim_alloc_limit_fanout`): drive the real plugin through `plugin->job_alloc_fn` on fixed fixtures (success; pending on shared; pending on hosts; parjob mid-plan rollback; parjob `maxTasks` 0 and 1; seqjob with `maxTasks` 0/2 via direct call; COMPOUND second primary with non-overlapping resources so bug-1892 is not exercised). Assert result code, host count/names, per-host resource entries/values, shared records, per-host and shared uncommitted debits, and pend reasons. Written and passing against today's code first (step 2). The fixtures for the intentional changes that the current code can run (seqjob per-host reasons, 3.7; bug-1893, 2.2; seqjob OOM rollback, 5.5) assert the new expected values: they are added at the start of the step that switches that plugin (RED against the not-yet-switched plugin) and turn GREEN within that step, so no step ends with a failing test (4.5, 7.2). seqjob's missing-input fixtures (5.1) are added after the switch only: today's seqjob dereferences the missing pointer, so they cannot fail as an assertion first; `test_alloc_runner.c` covers the same guard with the fake iface before the switch. + +### Regression (existing) +- The Req 7.1 list, unchanged in expected outcomes: `test_seqjob_allocator`, `test_seqjob_rewrap_equivalence`, `test_parjob_combinatorial`, `test_parjob_e2e_multislot`, `test_maxtasks_wiring_*`, `test_scheduler_cache_dispatch*`, `test_sim_contention_*parity`, `test_sim_live_allocation`, `test_sim_alloc_limit_fanout`, `test_sched_election_sim_fixture`, `test_snapshot_load_allocator_regression`; `test_picker_pendreason_factory_audit` with the new file listed (7.3). Full `/xce-build scheduler` (ASAN) suite green. + +### Invariant audit +- `/xce-audit-invariants scheduler` after the switch (7.5). Dimensions checked at design time: (1) driver state-equivalence — the runner is reached identically from `driver.c` and `sim_loop.c` via the same plugins; (2) core I/O isolation — `alloc_runner.c` does no I/O; (3) picker→allocator rendezvous — untouched; (4) snapshot parity — no persisted field changes. + +## Migration Strategy +1. Retype plan-iterator `init` (header, both iterators, `test_parjob_combinatorial.c`, `test_alloc_plan_iter.c`). +2. Characterization tests against current code (pass). +3. Add `alloc_runner.c`, iface, `plugin.h` field; switch parjob; delete parjob's private code; update white-box tests. +4. Switch seqjob; delete `my_alloc()`; update white-box tests. +5. Closure: comments in both plugins and `sim_alloc.c`, factory audit list, `issues/new_requirements.md` item 4, invariant audit. + +Each step ends with the full scheduler suite green; the only RED tests within a step are that step's own intentional-change fixtures, and a failing step-2 characterization test at steps 3–4 blocks the step. diff --git a/.kiro/specs/scheduler-allocator-runner/requirements.md b/.kiro/specs/scheduler-allocator-runner/requirements.md index 2509ddef..b17ea659 100644 --- a/.kiro/specs/scheduler-allocator-runner/requirements.md +++ b/.kiro/specs/scheduler-allocator-runner/requirements.md @@ -5,12 +5,12 @@ **Current situation:** `seqjob_allocator.c` (`my_alloc`, ~215 lines) and `parjob_allocator.c` (`my_alloc`, ~220 lines) each implement the whole allocation algorithm, although only plan generation is plugin-specific. Over 100 lines are duplicated almost verbatim: lazy pend-reason creation, getting the uncommitted shared resources, the shared-resource satisfaction check with its `PREASON_NO_RESOURCE` fill, adding the uncommitted shared resources, the plan-iterator loop skeleton, and the commit tail (`make_job_alloc` + `sched_make_allocs` + the `-ENOMEM` cleanup chain). Every allocation bug fix (bug-1882, bug-1767, bug-1768) has to land twice, and a new allocator plugin would have to copy the whole `my_alloc()`. Plan generation is already abstracted behind `xce_alloc_plan_iter_t`, whose `init` takes an unused `xce_scheduler_t*` even though sim mode has no real scheduler. Tracked as `issues/new_requirements.md` item 4. -**What should change:** the scheduler framework owns allocation. A new `xce_sched_allocator_iface_t` (an optional `accepts` guard plus `plan_init`/`plan_next`/`plan_fini`), referenced from `xce_sched_plugin_t` in the same way as `picker_iface`, lets one framework function, `sched_allocator_run()`, carry out shared-resource preparation, the bucket → host claim loop with rollback, and the commit tail. Both plugins register `.job_alloc_fn = sched_allocator_run`, supply only their plan iterator and `prepare_allocation`, and no longer define `my_alloc()`. The third parameter of `xce_alloc_plan_iter_t.init` is retyped to `xce_sched_core_t*`. Allocation decisions, pend reasons, and error and `-ENOMEM` behaviour stay identical for every existing test and sim-parity fixture, including bug-1882, bug-1767 and bug-1768. There is no per-plugin `commit` hook and no `sched_make_allocs()` retype (both left out of scope). The design must settle explicitly the seqjob/parjob divergences listed in `brief.md` → Constraints (the per-bucket `alloc_req.host` reset, seqjob's new OOM snapshot path, where `nslots`/`nhosts` are consumed, seqjob `accepts` for direct callers, and no VLA). Full context: `.kiro/specs/scheduler-allocator-runner/brief.md`. +**What should change:** the scheduler framework owns allocation. A new `xce_sched_allocator_iface_t` (an optional `accepts` guard plus `plan_init`/`plan_next`/`plan_fini`), referenced from `xce_sched_plugin_t` in the same way as `picker_iface`, lets one framework function, `sched_allocator_run()`, carry out shared-resource preparation, the bucket → host claim loop with rollback, and the commit tail. Both plugins register `.job_alloc_fn = sched_allocator_run`, supply only their plan iterator and `prepare_allocation`, and no longer define `my_alloc()`. The third parameter of `xce_alloc_plan_iter_t.init` is retyped to `xce_sched_core_t*`. Allocation decisions, pend reasons (except seqjob's per-host reasons, which become complete, Requirement 3 AC 6-7, and the spurious shared reasons of bug-1893, Requirement 2 AC 2), and error and `-ENOMEM` behaviour (except the seqjob hardening of Requirement 5 AC 1 and AC 5) stay identical for every existing test and sim-parity fixture, including bug-1882, bug-1767 and bug-1768. There is no per-plugin `commit` hook and no `sched_make_allocs()` retype (both left out of scope). The design must settle explicitly the seqjob/parjob divergences listed in `brief.md` → Constraints (the per-bucket `alloc_req.host` reset, decided 2026-09-29 as an unconditional reset for both allocators; seqjob's new OOM snapshot path, where `nslots`/`nhosts` are consumed, seqjob `accepts` for direct callers, and no VLA). Full context: `.kiro/specs/scheduler-allocator-runner/brief.md`. ## Introduction The scheduler's two built-in allocator plugins, the sequential-job allocator (seqjob, jobs with exactly one task) and the parallel-job allocator (parjob, jobs with more than one task), each carry a private copy of the whole allocation algorithm. This feature moves that algorithm into the scheduler plugin framework. It becomes one framework-owned allocation routine, and an allocator plugin contributes only what is genuinely its own: the candidate-plan generator, an optional "is this job mine" guard, and its existing per-allocation preparation hook. -This is a behaviour-preserving refactor. For every job, the scheduler must reach the same allocation decision, choose the same hosts, debit the same resources, record the same pend reasons, and return the same result codes as it does today, in normal (live) mode and in simulation mode. The only intentional behaviour changes are the two hardening points in Requirement 5 (AC 1 and AC 5), which make formerly undefined sequential-allocator behaviour defined. +This is a behaviour-preserving refactor. For every job, the scheduler must reach the same allocation decision, choose the same hosts, debit the same resources, record the same main pend reason, and return the same result codes as it does today, in normal (live) mode and in simulation mode. The only intentional behaviour changes are the two hardening points in Requirement 5 (AC 1 and AC 5), which make formerly undefined sequential-allocator behaviour defined, the sequential-allocator host pend reasons of Requirement 3 AC 6 and AC 7, which become complete instead of depending on stale state, and the shared-resource pend reasons of Requirement 2 AC 2, which lose the spurious entries today's loop reads past the unmet list (bug-1893). For every input that reaches the allocators today, host choice, main pend reason, and result codes stay the same. Terms: - **Allocation attempt**: one call from the scheduler's allocation dispatch to an allocator for one primary resource request. @@ -19,7 +19,7 @@ Terms: - **Pending signal**: the allocator reports "not satisfiable now" (result 0) without an error. ## Boundary Context -- **In scope**: a framework-owned allocation routine used by both built-in allocators; the allocator plugin contract (plan generator, optional acceptance guard, and the existing preparation hook); removal of the per-plugin allocation routines; changing the plan-generator initialisation to take the scheduling core instead of the scheduler object; keeping every existing allocation, pend-reason, error and simulation-parity behaviour; closing `issues/new_requirements.md` item 4. +- **In scope**: a framework-owned allocation routine used by both built-in allocators; the allocator plugin contract (plan generator, optional acceptance guard, and the existing preparation hook); removal of the per-plugin allocation routines; changing the plan-generator initialisation to take the scheduling core instead of the scheduler object; keeping every existing allocation, pend-reason, error and simulation-parity behaviour apart from the intentional changes listed in the Introduction; closing `issues/new_requirements.md` item 4. - **Out of scope**: a per-plugin override hook for assembling results; retyping the allocation-record constructor (`sched_make_allocs`) or removing the plugin's scheduler back-pointer; merging the two dispatch paths (sequential vs parallel) in the scheduler; fixing the missing allocation duration (the existing "no duration now" FIXME is kept as is); any change to plan order, host order, pend-reason vocabulary, the unsatisfiable-request cache, preemption, or the job picker. - **Adjacent expectations**: the scheduler's allocation dispatch keeps sending `maxTasks == 1` primaries to seqjob and every other primary to parjob, and keeps treating result `> 0` as satisfied, `0` as pending and `< 0` as error, with an allocation published only on `> 0`. COMPOUND/ALTER handling (bug-1766: a merged allocation is published only if every primary succeeds) and snapshot-restored schedobj construction (bug-1767) stay owned by their existing code. The plan and host iterator contracts from `scheduler-allocation` keep their current ordering semantics. @@ -32,16 +32,17 @@ Terms: 1. The Scheduler Plugin Framework shall provide one allocation routine that both built-in allocator plugins (seqjob and parjob) register as their job-allocation entry point. 2. The Scheduler Plugin Framework shall let an allocator plugin supply a candidate-plan generator (initialise, next plan, finish), an optional acceptance guard, and its existing per-allocation preparation hook, and nothing else, to take part in allocation. 3. Where an allocator plugin supplies no acceptance guard, the Scheduler Plugin Framework shall treat every primary resource request passed to that plugin as accepted. -4. The seqjob and parjob allocator plugins shall contain no private allocation routine that duplicates the shared-resource check, the plan/host search, or the result assembly. -5. The Scheduler Plugin Framework shall keep the allocator plugin contract as data the plugin registry does not free, with the same lifetime as the existing job-picker contract: it lives as long as the registered plugin, and the plugin does not release it separately. +4. When an allocator plugin's acceptance guard rejects a primary resource request, the Scheduler Plugin Framework shall return the pending signal without creating a pend-reason record, checking resources, or initialising the plan generator. +5. The seqjob and parjob allocator plugins shall contain no private allocation routine that duplicates the shared-resource check, the plan/host search, or the result assembly. +6. The Scheduler Plugin Framework shall keep the allocator plugin contract as data the plugin registry does not free, with the same lifetime as the existing job-picker contract: it lives as long as the registered plugin, and the plugin does not release it separately. ### Requirement 2: Shared-resource precheck -**Objective:** As a scheduler operator, I want jobs whose cluster-wide (shared) resources are unavailable to pend with the same reasons as today, so that pend diagnostics do not change. +**Objective:** As a scheduler operator, I want jobs whose cluster-wide (shared) resources are unavailable to pend with the same main reason as today and a reason for exactly the shared resources that are short, so that pend diagnostics stay stable and become accurate. #### Acceptance Criteria -1. When an allocation attempt starts and the attempt has no pend-reason record yet, the Scheduler Plugin Framework shall create one before checking shared resources. -2. When the job's shared-resource request cannot be met, the Scheduler Plugin Framework shall add one "no resource" pend reason, with no host and naming the resource, for each unmet shared resource. It shall then return the pending signal without trying any host. -3. When the job's shared-resource request can be met, the Scheduler Plugin Framework shall count the requested shared amounts as uncommitted for the attempt before trying any plan. It shall count them once per attempt, whatever the number of plans or buckets tried. +1. When an allocation attempt has passed the missing-input check and the acceptance guard, and the attempt has no pend-reason record yet, the Scheduler Plugin Framework shall create one before checking shared resources. +2. When the job's shared-resource request cannot be met, the Scheduler Plugin Framework shall add one "no resource" pend reason, with no host and naming the resource, for each entry of the unmet-resource list the shared check returns, and no other shared-resource reason. It shall then return the pending signal without trying any host. (Intentional change, bug-1893: today both allocators loop over the number of requested shared resources instead of unmet ones and add reasons naming resource index 0.) +3. When the job's shared-resource request can be met, the Scheduler Plugin Framework shall count the requested shared amounts as uncommitted for the attempt before trying any plan. It shall count them once per attempt, whatever the number of plans or buckets tried, and shall not release them when the attempt then pends or fails, as today (the attempt context, which holds them, is discarded by its caller, or shared with the next primary of the same COMPOUND job or of the same ALTER alternative; each ALTER alternative gets a fresh context). 4. If creating the pend-reason record, reading the uncommitted shared state, checking shared satisfaction, or counting shared amounts as uncommitted fails, the Scheduler Plugin Framework shall return that failure's negative error code, with no allocation on the attempt. ### Requirement 3: Plan and host search @@ -52,9 +53,9 @@ Terms: 2. While trying a plan, the Scheduler Plugin Framework shall try, for each bucket in order, the candidate hosts in the shared host-iteration order. It shall claim the first candidate host that satisfies the bucket and has not already been claimed by an earlier bucket of the same plan. 3. If any bucket of a plan cannot be satisfied, the Scheduler Plugin Framework shall release the per-host resource debits of every claim already made for that plan before trying the next plan. 4. When the plan generator cannot be initialised or yields no plan whose buckets are all satisfied, the Scheduler Plugin Framework shall return the pending signal with no allocation on the attempt. -5. The Scheduler Plugin Framework shall give each bucket the task count and single-host shape that parjob uses today when checking a candidate host, and shall leave seqjob's per-host checks with the one-task, one-host shape they have today. -6. While seqjob runs an attempt for a primary that shares its attempt context with an earlier primary of the same COMPOUND or ALTER job, the Scheduler Plugin Framework shall start seqjob's host search from that primary's own per-host request only, as it does today: the job-level preparation pass resets the per-host request for every primary (bug-1883, fixed in `c10f32cc`), so no earlier primary's per-host request reaches seqjob's host pre-filtering or pend reasons. -7. While parjob runs an attempt, the Scheduler Plugin Framework shall start every bucket's host search from an empty per-host request, as parjob does today. +5. Before each bucket's host search, the Scheduler Plugin Framework shall set the attempt's slot count to that bucket's task count and its host count to 1, as parjob does today. For seqjob, whose only plan is one bucket of one task, the slot count and host count shall always be 1, in the job-level preparation pass and in every per-host check. +6. The Scheduler Plugin Framework shall start every bucket's host search from an empty per-host request, for both seqjob and parjob, so that no earlier bucket, earlier candidate host, or earlier primary of the same COMPOUND or ALTER job (bug-1883) affects which hosts the host search skips. This is today's parjob behaviour and an intentional change for seqjob, which today pre-filters candidates against whatever per-host request was left from the job-level preparation pass or the last rejected candidate. +7. When seqjob's host search finds no host for a pending job, the Scheduler Plugin Framework shall record a "no resource" pend reason, naming the host and the resource, for every candidate host that matches the job's host selection and that the existing per-host check finds short of a requested resource, and shall leave the job's main pend reason as it is today. (Intentional change: today such reasons appear only for hosts that also pass the stale pre-filter.) 8. The Scheduler Plugin Framework shall support plans with any number of buckets up to the job's task count, with no fixed upper limit on the number of buckets that a large task count could exceed. ### Requirement 4: Result assembly @@ -62,8 +63,8 @@ Terms: #### Acceptance Criteria 1. When every bucket of a plan is satisfied, the Scheduler Plugin Framework shall build one host allocation per claim, in claim order. Each is made from the claimed host's name and that host's resource request as it stood when the claim was made. -2. When every bucket of a plan is satisfied and the job requests shared resources, the Scheduler Plugin Framework shall attach one shared allocation record per requested shared resource. When the job requests none, it shall attach no shared records. -3. The Scheduler Plugin Framework shall produce allocation results with the same job identifier, empty reservation name, and absent duration that seqjob and parjob produce today. +2. When every bucket of a plan is satisfied and the job requests shared resources, the Scheduler Plugin Framework shall attach one shared allocation record per entry of the attempt's shared-resource request, including entries added by limits. When that request is empty, it shall attach no shared records. +3. The Scheduler Plugin Framework shall produce allocation results as seqjob and parjob do today: job identifier 0 (filled in by later stages), empty reservation name on every resource record, and no duration. 4. When a plan is satisfied, the Scheduler Plugin Framework shall set the attempt's allocation to the newly built result and return a positive result. 5. When seqjob or parjob is given an input that reaches it today, the Scheduler Plugin Framework shall produce an allocation result identical to that allocator's current result: same host count, host names, per-host resource entries and values, shared records, and return code. @@ -72,16 +73,16 @@ Terms: #### Acceptance Criteria 1. If the scheduling core, the plugin's scheduler back-reference, or the attempt's primary resource request is missing, the Scheduler Plugin Framework shall return an invalid-argument error without creating a pend-reason record, for both allocators. (Hardening for seqjob, whose current behaviour is undefined.) -2. If parjob receives a primary resource request whose task count is 1 or less, the Scheduler Plugin Framework shall return the pending signal without creating a pend-reason record or checking resources (bug-1767). +2. If parjob receives a primary resource request whose task count is 1 or less, the Scheduler Plugin Framework shall return the pending signal without creating a pend-reason record or checking resources (bug-1767). parjob's acceptance guard provides this (Requirement 1 AC 4), after the missing-input check of AC 1. 3. The Scheduler Plugin Framework shall let seqjob accept every primary resource request that reaches it, whatever its task count, as today. 4. If building any part of the allocation result fails for lack of memory, the Scheduler Plugin Framework shall return an out-of-memory error, leave no allocation on the attempt, and free every partly built piece (bug-1882). -5. If a failure for lack of memory happens after one or more claims were made, the Scheduler Plugin Framework shall release the per-host resource debits of every claim not yet handed over to a built result, for both allocators. (Hardening for seqjob, which today leaves its single debit in place until the attempt's context is discarded.) +5. If a failure for lack of memory happens after one or more claims were made, the Scheduler Plugin Framework shall release the per-host resource debits of every claim not yet turned into a host allocation, and of the candidate host whose claim could not be recorded, for both allocators. (Hardening for seqjob, which today leaves its single debit in place until the attempt's context is discarded.) 6. The Scheduler Plugin Framework shall set the attempt's allocation only when it returns a positive result, so that callers merging COMPOUND/ALTER results keep publishing only fully satisfied jobs (bug-1766). 7. When an attempt that initialised the plan generator exits (success, pending, or error), the Scheduler Plugin Framework shall finish the plan generator and release every per-claim resource-request copy it holds. 8. The Scheduler Plugin Framework shall size each primary's allocation from that primary's own resource request, including primaries nested in COMPOUND and ALTER jobs (bug-1768). ### Requirement 6: Plan-generator initialisation contract -**Objective:** As a scheduler maintainer, I want plan generators to receive the scheduling core, which exists in both modes, rather than a scheduler object that simulation mode lacks, so that the contract is honest in every mode. +**Objective:** As a scheduler maintainer, I want plan generators to receive the scheduling core, which exists in both modes, rather than a scheduler object that simulation mode has only as a stand-in (a partial wrapper kept for building allocation records, `sim_alloc.c`), so that the contract is honest in every mode. #### Acceptance Criteria 1. The Scheduler Plugin Framework shall initialise an allocator plugin's plan generator with the primary resource request and the scheduling core. @@ -89,11 +90,11 @@ Terms: 3. While running in simulation mode, the Scheduler Plugin Framework shall run allocation through the same framework routine and plan generators as normal mode, with no scheduler object passed to any plan generator. ### Requirement 7: Equivalence evidence and closure -**Objective:** As a project maintainer, I want test evidence that nothing changed and the tracked item closed, so that the refactor can be accepted with confidence. +**Objective:** As a project maintainer, I want test evidence that nothing changed beyond the intentional changes listed in the Introduction, and the tracked item closed, so that the refactor can be accepted with confidence. #### Acceptance Criteria 1. The Scheduler Plugin Framework shall pass, unchanged in their expected outcomes, the existing seqjob allocator, seqjob rewrap-equivalence, parjob combinatorial, parjob end-to-end multi-slot, task-count wiring, unsatisfiable-cache dispatch, simulation parity, simulation live-allocation, simulation limit fan-out, election simulation fixture, and snapshot-load allocator regression tests. Test changes are limited to how the allocation routine is reached and the parameter type of plan-generator initialisation. -2. The Scheduler Plugin Framework shall have tests showing that, for the same inputs, the framework routine gives the same result code, allocation result, pend reasons, and per-host debits as the removed seqjob and parjob routines. These tests cover success, pending on shared resources, pending on hosts, mid-plan rollback, out-of-memory at each result-building step, the parjob out-of-contract task count, and missing inputs. +2. The Scheduler Plugin Framework shall have tests showing that, for the same inputs, the framework routine gives the same result code, allocation result, pend reasons, and per-host debits as the removed seqjob and parjob routines. These tests cover success, pending on shared resources, pending on hosts (for seqjob, the per-host reasons given by Requirement 3 AC 7 rather than today's; for shared resources, the reasons given by Requirement 2 AC 2), mid-plan rollback, out-of-memory at each result-building step, the parjob out-of-contract task count, and missing inputs. 3. The Scheduler Plugin Framework shall keep the existing check that pend-reason records are created only through the shared pend-reason factory, and that check shall cover the new framework allocation code. 4. When this feature is complete, `issues/new_requirements.md` item 4 shall be marked resolved, with a reference to this spec. 5. When this feature is complete, the scheduler state-equivalence invariant audit shall report no new violations in the allocation path. diff --git a/.kiro/specs/scheduler-allocator-runner/research.md b/.kiro/specs/scheduler-allocator-runner/research.md new file mode 100644 index 00000000..a5550fac --- /dev/null +++ b/.kiro/specs/scheduler-allocator-runner/research.md @@ -0,0 +1,109 @@ +# Research & Design Decisions + +## Summary +- **Feature**: `scheduler-allocator-runner` +- **Discovery Scope**: Extension (light discovery). Refactor of an existing plugin seam inside `src/scheduler`; no new external dependency. +- **Key Findings**: + - The two `my_alloc()`s differ only in: the plan iterator they wire, parjob's `maxTasks <= 1` guard and NULL-input guard, parjob's per-bucket `alloc_req.host` reset, per-candidate `nslots/nhosts` assignment, and claim bookkeeping (parjob snapshots each claim's `host_req`; seqjob keeps a single `w` and reads the live `alloc_req.host` at commit). Everything else is duplicated text. + - `plugin.h` includes no headers and `xce_alloc_plan_t` is an anonymous typedef, so the full allocator iface cannot live in `plugin.h` without either a header cycle or a tag change. The picker iface already solves the same problem with a forward typedef in `plugin.h`. + - Three white-box tests call the static `my_alloc()` / `parjob_try_plan_()` / `parjob_rollback_claims_()` directly and stub `sched_make_allocs()` (bug-1882 OOM injection); none asserts pend-reason content. `test_parjob_combinatorial.c` passes `NULL` and a poisoned `xce_scheduler_t*` as the plan iterator's third argument. + +## Research Log + +### Current allocator code paths +- **Context**: Establish exactly what the runner must reproduce (Req 3, 4, 5). +- **Sources Consulted**: `src/scheduler/src/plugins/seqjob_allocator.c:213-420`, `parjob_allocator.c:300-760`, `src/scheduler/src/resource.c:493-620`, `src/scheduler/src/plugin.c:612-705`, `src/scheduler/src/host_iter.c:100-170`, `src/scheduler/src/scheduler.c:235-435`. +- **Findings**: + - Shared prep is byte-identical in both: lazy `make_pendreason(0)` → `sched_ctx_get_uncomit_shared` → `sched_check_shared_satisfy` → `NO_RESOURCE` fill → `xvec_idval(uncomit, &shared, XVEC_ADD_FLOAT)`. The fill loop is bounded by `alloc_req.shared.count` over `ctx->unmeet` (bug-1893). + - Plan-init failure returns 0 in both (no `fini`). + - parjob: `claimed[primary->maxTasks]` VLA; per bucket: reset `alloc_req.host`, host iter, skip claimed, set `nslots = bucket_slots[b]`, `nhosts = 1`, `sched_try_alloc_on_host_() > 0` → snapshot `alloc_req.host` (OOM → uncommit candidate with live request + rollback earlier claims → return rc). + - Commit tail: parjob `calloc` of `hallocs`, per claim `sched_make_allocs(sched, "", nres, idxs, vals, NULL)` + `make_host_alloc`; failure rolls back claims `[i, n)` and frees `hallocs`; then `make_job_alloc(0, n, hallocs)`; then the shared-record block. seqjob uses `make_host_allocs(1, halloc)`, which produces the same one-element array. + - Both set `ctx->alloc` before building the shared records and `free_JobAlloc_ptr(&ctx->alloc)` (which NULLs it) on failure. +- **Implications**: The runner is parjob's algorithm, generalised, plus the shared-prep block; seqjob becomes a degenerate case (1 plan, 1 bucket of 1 slot). + +### Where `nslots/nhosts` are consumed +- **Context**: brief Constraints item 3. +- **Findings**: Only `sched_fill_limit_requests()` reads them (`plugin.c:682` job-level pass, `:699` per-host pass). seqjob sets 1/1 only in its job-level pass (`seqjob_allocator.c:92`). +- **Implications**: The runner sets both per candidate from the plan (Req 3.5); seqjob sees 1/1 everywhere. + +### The per-host request and the host iterator pre-filter +- **Context**: brief Constraints item 1, Req 3.6/3.7. +- **Findings**: `plugin_prepare_allocation()` with `ctx->worker` set clears and refills `alloc_req.host` per candidate (`plugin.c:639`), so the authoritative check always uses a fresh request. `sched_host_iter_next()` pre-filters against whatever `alloc_req.host` holds before that refill and skips failures silently. `add_job_preason(..., host, res)` writes `res_based_reasons`, never `main_reason` (`lib/preason.c:147`); the unsatisfiable cache reads `main_reason` only (`scheduler.c:271`); sim parity compares `main_reason` only (`sim_verify.c:126`). +- **Implications**: The unconditional per-bucket reset changes only seqjob's host/resource reasons. + +### Plugin ABI and header layout +- **Context**: Req 1.1–1.6, 6.1. +- **Sources Consulted**: `include/plugin.h`, `include/sched/alloc_iter.h`, `include/scheduler.h`, `include/core.h`. +- **Findings**: `plugin.h` has no `#include`s; `core.h` and `scheduler.h` include it. `alloc_iter.h` includes `scheduler.h`. `picker_iface` is a forward-declared `xce_job_picker_iface_t*` pointing at static storage that the registry never frees (`plugin.c:111,149`). Plugin templates use designated initialisers and are `memcpy`'d at registration. +- **Implications**: Forward typedef in `plugin.h`; full struct and runner prototype in `alloc_iter.h`; iface storage `static const` in each plugin file. + +### Simulation mode +- **Context**: Req 5.1, 6.3. +- **Sources Consulted**: `src/scheduler/src/sim_alloc.c`. +- **Findings**: sim registers the same plugins through `seqjob_allocator_new_plugin()` / `parjob_allocator_new_plugin()` against `sim->core` and sets `plugin->sched` to a stand-in wrapper used only by `sched_make_allocs()`. So `plugin->sched` is non-NULL in both modes. +- **Implications**: No sim code change beyond comments; the runner reaches sim through the plugin templates. + +### Build and test wiring +- **Sources Consulted**: `src/scheduler/meson.build:25-45`, `testsuite/meson.build:1700-1760`, `testsuite/test_picker_pendreason_factory_audit.c:60-80`. +- **Findings**: `scheduler_srcs` lists every source; many test targets link it. `test_seqjob_allocator.c`, `test_seqjob_rewrap_equivalence.c`, `test_parjob_combinatorial.c` white-box `#include` the allocator `.c` plus `core.c`, `resource.c`, `schedobj.c`, `host_iter.c`. The factory audit scans a hard-coded file list. +- **Implications**: New source goes into `scheduler_srcs`, into the white-box tests' `#include` lists, and into the audit's file list. + +### File size cap +- **Findings**: `src/scheduler/CLAUDE.md`: 800-line cap. `parjob_allocator.c` is 814 lines, `scheduler.c` 2002 (pre-existing exceptions, not precedent). +- **Implications**: The runner goes into a new file, not `scheduler.c`; parjob drops below the cap. + +## Architecture Pattern Evaluation + +| Option | Description | Strengths | Risks / Limitations | Notes | +|--------|-------------|-----------|---------------------|-------| +| Template method via iface (chosen) | Framework runner owns the algorithm; plugin supplies plan iterator + optional `accepts` | One copy of the algorithm; matches picker-iface precedent; new allocators need only a plan iterator | Plugin ABI change | Brief Approach, "Framework runner, lean" | +| Shared helpers only | Extract helpers, keep each `my_alloc()` | Smallest diff | Plugins still own the loop; fails item 4 | Rejected in brief | +| Runner + `commit` hook | Also let plugins override result assembly | Future flexibility | Unused extension point needing its own tests | Rejected in brief | + +## Design Decisions + +### Decision: Iface lives in `alloc_iter.h`, forward-declared in `plugin.h` +- **Context**: Req 1.2, 1.6; `plugin.h` cannot see plan-iterator types. +- **Alternatives Considered**: + 1. Full struct in `plugin.h` (needs a named tag on `xce_alloc_plan_t` and forward declarations of resreq types). + 2. Forward typedef in `plugin.h`, full struct in `alloc_iter.h` (picker pattern). +- **Selected Approach**: 2. +- **Rationale**: No header cycle, no change to `xce_alloc_plan_t`; `alloc_iter.h` is already the allocation-contract header both plugins include. +- **Trade-offs**: The iface is not visible from `plugin.h` alone; plugin authors include `sched/alloc_iter.h`, which they already do. + +### Decision: Claims buffer grows with the largest plan, not bound by `maxTasks` +- **Context**: Req 3.8, 5.3; brief item 5 (no VLA). seqjob accepts any `maxTasks` (0 reaches it from direct callers), so `maxTasks` is not a safe bound. +- **Selected Approach**: A heap buffer owned by the attempt, grown with `realloc` to `plan->nbuckets` before trying a plan that needs more entries; freed on every exit. +- **Trade-offs**: One extra error path (`-ENOMEM` on growth, taken between plans with no claims outstanding); no stack-size risk for large task counts. + +### Decision: Result built into a local, published to `ctx->alloc` last +- **Context**: Req 4.4, 5.4, 5.6, bug-1882. +- **Selected Approach**: `sched_finalize_job_alloc_()` builds the `xce_job_alloc_t` including shared records in a local and assigns `ctx->alloc` only on success. +- **Rationale**: Today's code sets `ctx->alloc` early and relies on `free_JobAlloc_ptr` NULLing it; building locally makes Req 5.6 structural. Observable result is identical. + +### Decision: Staged migration +- **Selected Approach**: (1) retype plan-iterator `init`; (2) characterization tests that drive each plugin through `plugin->job_alloc_fn` against the current code; (3) runner + iface, parjob switched; (4) seqjob switched; (5) closure (comments, audit list, item 4). Each stage leaves the suite green. +- **Rationale**: Characterization tests written through `job_alloc_fn` pass before and after the switch, giving Req 7.2's equivalence evidence without keeping the old code. + +### Synthesis outcomes +- **Generalization**: seqjob is parjob's algorithm with one plan of one one-slot bucket; the runner is written once for the general case (N plans × M buckets), and seqjob needs no special path. +- **Build vs adopt**: internal refactor; nothing external applies. The claim/rollback/snapshot helpers are adopted from parjob unchanged in behaviour, only renamed and moved. +- **Simplification**: no `commit` hook; `accepts` is optional (NULL for seqjob); helper functions stay `static` in the runner file (tests reach them by white-box `#include`, the project's existing convention) instead of widening the scheduler header. + +### Decisions added in design review (2026-09-29) +- **Snapshot ownership handoff**: `sched_try_plan_()` owns claim snapshots until it returns 1, then `sched_finalize_job_alloc_()` owns and frees all of them; the runner never frees one. Prevents a double free between the finalize failure path and the runner's cleanup. +- **Thread cancellation**: `plan_next` and `sched_host_iter_next()` are cancellation points; no scheduler code uses `pthread_cleanup_push()`. Cancel happens only at shutdown, so a mid-attempt cancel keeps abandoning attempt-scoped heap as today (the claims buffer joins that set). No cleanup handlers added. +- **Per-bucket reset side effects**: seqjob now runs the prepare fan-out on hosts the stale pre-filter used to skip, so the limit plugin's lazy host-limit instantiation (`limit.c` `match_job_host_limit_`: `obj->hlimits`, `instantiate_limit`, `worker->resvals`) can happen earlier. Decision-neutral; extra cost per short host per pending seqjob attempt, bounded by the unsatisfiable cache. Recorded in brief item 1. +- **Characterization tests** reuse the existing `plugin_prepare_allocation()` stub convention (`test_seqjob_allocator.c:84`); limit fill stays covered by `test_sim_alloc_limit_fanout` through the real `plugin.c`. seqjob `-EINVAL` fixtures come after the switch (today's code dereferences NULL). +- **Retype scope** includes `test_alloc_plan_iter.c`'s stub iterator. + +## Risks & Mitigations +- Unintended behaviour change in seqjob beyond the listed ones — characterization tests written before the switch, run unchanged after; sim parity tests. +- White-box tests fail to link after the move — the runner file is added to each white-box test's `#include` list in the same task that deletes the plugin code. +- bug-1892 (positional uncommitted read in `unsatisfy_`) makes some COMPOUND outcomes wrong today — out of scope; COMPOUND characterization tests avoid asserting outcomes that depend on it. +- `parjob_allocator.c` and `seqjob_allocator.c` comments reference `my_alloc()` extensively; stale comments mislead — comment cleanup is its own closure task. + +## References +- `.kiro/specs/scheduler-allocator-runner/brief.md` — decisions 1–6. +- `.kiro/specs/scheduler-allocation/` — plan/host iterator contracts (upstream). +- `.wolf/bugs/bug-1892.md`, `.wolf/bugs/bug-1893.md`. diff --git a/.kiro/specs/scheduler-allocator-runner/spec.json b/.kiro/specs/scheduler-allocator-runner/spec.json index be7b26ca..20f1093a 100644 --- a/.kiro/specs/scheduler-allocator-runner/spec.json +++ b/.kiro/specs/scheduler-allocator-runner/spec.json @@ -1,24 +1,24 @@ { "feature_name": "scheduler-allocator-runner", "created_at": "2026-09-29T03:45:23Z", - "updated_at": "2026-09-29T04:07:44Z", + "updated_at": "2026-09-30T07:34:49Z", "language": "en", - "phase": "requirements-generated", + "phase": "tasks-generated", "approvals": { "requirements": { "generated": true, - "approved": false + "approved": true }, "design": { - "generated": false, - "approved": false + "generated": true, + "approved": true }, "tasks": { - "generated": false, - "approved": false + "generated": true, + "approved": true } }, - "ready_for_implementation": false, + "ready_for_implementation": true, "gitea": { "milestone_id": 67, "issues": { diff --git a/.kiro/specs/scheduler-allocator-runner/tasks.md b/.kiro/specs/scheduler-allocator-runner/tasks.md new file mode 100644 index 00000000..4624c4fa --- /dev/null +++ b/.kiro/specs/scheduler-allocator-runner/tasks.md @@ -0,0 +1,92 @@ +# Implementation Plan + +- [ ] 1. Foundation: plan-iterator contract and allocator contract +- [ ] 1.1 Retype the plan generator's initialisation to take the scheduling core + - Change the plan-iterator initialisation parameter from the scheduler object to the scheduling core in the shared contract, in both built-in plan generators, in both allocators' current calls that initialise their plan generator (they pass the scheduling core instead of the scheduler object), and in the stub generator used by the plan-iterator contract tests + - Update the parjob combinatorial tests' direct initialisation calls; turn the poisoned-scheduler case into a poisoned-core case that shows the plan sequence is identical with no core and with a poisoned core + - Done when the full scheduler suite builds and passes under ASAN with no plan generator taking a scheduler object, and the poisoned-core test shows identical plan sequences for parjob + - _Requirements: 6.1, 6.2_ + +- [ ] 1.2 Declare the allocator plugin contract and the plugin's reference to it + - Add the allocator contract (optional acceptance guard plus plan initialise / next / finish) next to the plan-iterator contract, and declare the framework allocation entry point there + - Add the plugin's reference to its allocator contract, documented with the same ownership rule as the job-picker contract: static storage in the plugin file, never freed by the registry, NULL for non-allocator plugins + - Done when the scheduler and its suite build and pass unchanged with the new declarations present and no plugin using them yet + - _Requirements: 1.2, 1.6_ + +- [ ] 2. Characterization tests against the current allocators +- [ ] 2.1 Characterize the sequential-job allocator through its registered entry point + - New test target that drives the real seqjob plugin through its job-allocation entry point, using the existing prepare-allocation stub convention and an allocation-record stub with out-of-memory injection + - Set up the out-of-memory injection that 2.2 reuses, using only injection points that stay the same across the switch: the existing allocation-record stub failing on its N-th call (per-host records in claim order, then shared records) and link-time wrapping (the testsuite's existing wrap precedent) of the libxce host-allocation and job-allocation constructors with fail-on-call-N counters; the host-array allocation is not injected here because its origin changes at the switch (seqjob's libxce helper and parjob's in-file allocation both become a runner allocation) and is covered by 3.3 + - Fixtures: success; pending on a shared resource where the job requests exactly the shared resources that are short (so bug-1893's extra reasons cannot appear); pending on hosts with the pre-filter state arranged so no host is skipped on stale state (assert host reasons only where 3.7 does not change them, and the main reason); out of memory at the host allocation, the job allocation and the shared records (today's result code and allocation state; for the host-allocation case the per-host debit is asserted separately so 4.2 can change it); task counts 0 and 2 reaching the plugin directly; the second primary of a COMPOUND job with non-overlapping resources (bug-1892 not exercised) + - Each fixture asserts result code, host count and names, per-host resource entries and values, shared records, per-host and shared uncommitted amounts, and pend reasons + - Done when the new target passes against the unchanged seqjob allocator + - _Requirements: 4.5, 5.4, 5.8, 7.2_ + +- [ ] 2.2 Characterize the parallel-job allocator through its registered entry point + - New test target for the real parjob plugin with the same stubs, injection mechanism and assertions as 2.1 + - Fixtures: multi-bucket success; pending on a shared resource requesting exactly the short resources; pending on hosts; mid-plan rollback where a later bucket fails and a later plan succeeds; task counts 0 and 1 (pending, no pend-reason record); missing core, scheduler back-reference or primary request (invalid-argument, no pend-reason record); out of memory at the allocation records of host 0 and of a later host, at host allocation 0 and at a later host allocation, at the job allocation and at the shared records; the second primary of a COMPOUND job with non-overlapping resources + - Done when the new target passes against the unchanged parjob allocator + - _Requirements: 4.5, 5.1, 5.2, 5.4, 5.8, 7.2_ + +- [ ] 3. Framework allocation routine +- [ ] 3.1 Build the routine's guards and shared-resource precheck, test first + - Add the new runner source to the scheduler build and to the pend-reason factory audit's scanned files, and add a runner test target that uses a scripted fake allocator contract + - Missing plugin, context, core, scheduler back-reference, primary request, contract or plan functions returns invalid-argument without creating a pend-reason record + - A rejecting acceptance guard returns pending with no pend-reason record, no resource check and no plan initialisation; an absent guard accepts + - The pend-reason record is created only after the guards; the shared check adds exactly one host-less "no resource" reason per unmet entry (bug-1893 fixture: two shared resources, one short); shared amounts are counted as uncommitted when met and are not released when the attempt then pends; each failure returns its own error + - Done when the runner tests for guards and shared precheck pass under ASAN and the factory audit passes with the runner scanned + - _Depends: 1.1, 1.2_ + - _Requirements: 1.3, 1.4, 2.1, 2.2, 2.3, 2.4, 5.1, 7.2, 7.3_ + +- [ ] 3.2 Build the plan and claim search with rollback, test first + - Plans are tried in generator order and the first fully satisfied plan wins; each bucket claims the first unclaimed satisfying host in host-iteration order + - Shared amounts are counted as uncommitted exactly once across an attempt that tries several plans + - Before each bucket's host scan the per-host request is empty; before each candidate the slot count is the bucket's task count and the host count is 1 + - A failing bucket releases every claim of the plan; a claim-record (snapshot) failure releases the candidate and earlier claims and returns the error; a plan with no buckets or a zero-slot bucket is skipped + - Claims-list growth failure is injected with a call-counting replacement of the heap allocator applied only to the white-box-included runner source (reused by 3.3); snapshot failure through a new link-time wrap of the libxce vector-copy function (same precedent) that is armed to fail only its next call, because other libxce code also calls it + - The claims list grows to the largest plan with no fixed limit; plan-generator initialisation failure returns pending; every return path after initialisation finishes the generator and frees the claims list + - Done when runner tests covering order, single shared count, distinct hosts, rollback debits, empty-request-per-bucket, slot/host counts, growth, malformed plans and snapshot failure pass under ASAN + - _Requirements: 2.3, 3.1, 3.2, 3.3, 3.4, 3.5, 3.6, 3.8, 5.5, 5.7, 7.2_ + +- [ ] 3.3 Build result assembly and its out-of-memory paths, test first + - One host allocation per claim in claim order from the claimed host's name and its request snapshot; one shared record per shared-request entry, none when empty; job identifier 0, empty reservation name, no duration + - The result is built privately and published to the attempt only on success, which returns a positive result + - Out of memory at the host array, any host allocation, the job allocation or the shared records returns out-of-memory, leaves no allocation on the attempt, frees every partly built piece and releases the debits of claims not yet turned into a host allocation; claim snapshots are freed exactly once on every path + - Out-of-memory steps are injected with the constructor wrapping and record stub from 2.1, plus a call-counting replacement of the heap allocator applied only to the white-box-included runner source (host array and claims list), so the host-array step Requirement 7 AC 2 needs is covered here + - Done when runner tests for success and each out-of-memory step pass under ASAN with no leak or double free + - _Requirements: 4.1, 4.2, 4.3, 4.4, 5.4, 5.5, 5.6, 5.7, 7.2_ + +- [ ] 4. Move both allocators onto the framework routine +- [ ] 4.1 Switch the parallel-job allocator + - First add the parjob bug-1893 fixture (two shared resources, one short: only the short one is named) to the parjob characterization tests and see it fail against the old code + - Give parjob its allocator contract with a task-count acceptance guard (more than one task), register the framework routine as its allocation entry point, and remove its private allocation routine and claim helpers + - Build the runner into the parjob characterization target (2.2) so it keeps reaching the plugin through its entry point + - Update the parjob combinatorial tests, the only permitted kind of change: build the runner into the white-box test, give their hand-built plugin structs the parjob allocator contract, and call the framework routine and the framework's claim search and rollback instead of the removed helpers + - Done when every parjob characterization fixture (including the new one), the parjob combinatorial and end-to-end multi-slot tests, and the full scheduler suite pass under ASAN + - _Depends: 2.2, 3.3_ + - _Requirements: 1.1, 1.2, 1.5, 1.6, 2.2, 5.2, 7.1_ + +- [ ] 4.2 Switch the sequential-job allocator + - First add the seqjob intentional-change fixtures to the seqjob characterization tests and see them fail against the old code: a candidate host that the stale per-host request left by the job-level pass used to skip is now checked; complete per-host "no resource" reasons for every selection-matching short host with the main reason unchanged; bug-1893; release of the claimed host's debit when result building runs out of memory (this rewrites the 2.1 host-allocation out-of-memory fixture's per-host debit assertion to the new expected value) + - Give seqjob its allocator contract with no acceptance guard, register the framework routine as its allocation entry point, and remove its private allocation routine + - After the switch, add seqjob missing-input fixtures (invalid-argument, no pend-reason record) + - Build the runner into the seqjob characterization target (2.1) + - Update the seqjob allocator and seqjob rewrap-equivalence tests, the only permitted kind of change: build the runner into the white-box tests, give their hand-built plugin structs the seqjob allocator contract, and call the framework routine + - Done when every seqjob characterization fixture, the seqjob allocator and rewrap-equivalence tests, and the full scheduler suite pass under ASAN + - _Depends: 2.1, 4.1_ + - _Requirements: 1.1, 1.2, 1.5, 1.6, 3.6, 3.7, 5.1, 5.3, 5.5, 7.1_ + +- [ ] 5. Validation and closure +- [ ] 5.1 Remove stale references to the removed routines + - Rewrite comments in both allocator plugins and the simulation allocator bootstrap that describe the removed private routines, so they describe the framework routine + - Rewrite testsuite comments that describe current behaviour in terms of the removed routines (host-iterator, simulation limit fan-out, election simulation fixture and parjob end-to-end multi-slot tests); comments that record history may keep the old names + - Done when no scheduler source or test comment presents the removed routines as current code, and the suite still passes + - _Requirements: 1.5_ + +- [ ] 5.2 Prove equivalence across modes and close the tracked item + - Run the full scheduler ASAN suite and confirm the existing allocation, task-count wiring, unsatisfiable-cache dispatch, simulation parity, simulation live-allocation, limit fan-out, election fixture and snapshot-load allocator tests pass with unchanged expectations + - Run the scheduler invariant audit and confirm no new violations in the allocation path (driver state-equivalence between normal and simulation mode included) + - Mark `issues/new_requirements.md` item 4 resolved with a reference to this spec + - Done when the suite and the audit are clean and item 4 is marked resolved + - _Depends: 4.2, 5.1_ + - _Requirements: 6.3, 7.1, 7.4, 7.5_ diff --git a/.wolf/bugs/bug-1892.md b/.wolf/bugs/bug-1892.md new file mode 100644 index 00000000..3551a5f4 --- /dev/null +++ b/.wolf/bugs/bug-1892.md @@ -0,0 +1,86 @@ +--- +id: "bug-1892" +status: "open" +status_updated: "2026-09-30" +status_note: "newly logged" +component: "scheduler" +severity: "wrong-result" +timestamp: "2026-09-30T03:58:52Z" +last_seen: "2026-09-30T03:58:52Z" +occurrences: 1 +files: ["src/scheduler/src/resource.c", "src/lib/xvec.c"] +tags: ["scheduler", "allocation", "compound", "code-review"] +related: ["bug-1893"] +--- + +# bug-1892: xce_scheduler admission check reads per-resource uncommitted amounts by list position, not index + +## Description +**Observed:** found by code reading, so there is no failing run yet. `unsatisfy_()` (`src/scheduler/src/resource.c:130-169`) compares +```c +if (total[i] < inuse[i] + uncomitted[i] + wanted[i]) +``` +where `total`, `inuse` and `wanted` are *positional* arrays (entry `i` is the +`i`-th resource of the request, gathered by `get_resvals(..., wanted->idxs, ...)`), +but `uncomitted` is the context's *dense, resource-index-keyed* vector +(`ctx->uncomitted.shared` / `ctx->uncomitted.hosts[w]`, 1024 floats, +`schedobj.c:514/595`), filled by `xvec_idval(uncomit, &req, XVEC_ADD_FLOAT)`, +which writes `d[idx] += val` (`lib/xvec.c`, `DEFINE_XVEC_IDVAL_OP`). Both +`sched_check_shared_satisfy_()` (`resource.c:309`) and +`sched_check_host_satisfy_()` (`resource.c:376`) pass `uncomitted->data` +straight through, so entry `i` reads the uncommitted amount of resource index +`i`, not of resource `wanted->idxs[i]`. + +**Expected:** entry `i` uses `uncomitted[wanted->idxs[i]]` (gather it +positionally like `total`/`inuse`, e.g. with the existing `get_resvals()`). + +**Impact:** wrong admission result. Uncommitted amounts are nonzero only +within one allocation context that already committed something, i.e. from the +second primary of a COMPOUND (or ALTER) job on, since those primaries share one +`ctx` (`scheduler.c:348-354`); the first primary and ordinary PRIMARY jobs +always see all-zero uncommitted vectors, and parjob rolls back failed plans. +For a later primary, the check subtracts an unrelated resource's uncommitted +amount (or 0) instead of the earlier primaries' real claim on the same +resource, so it can over-admit (earlier primary's claim ignored: the job is +dispatched beyond host/shared capacity) or wrongly pend (another resource's +amount charged). Pend reasons printed for the same check carry the wrong +shortfall. + +**Found by:** review of the `scheduler-allocator-runner` requirements +(2026-09-29), while checking the shared-resource precheck the runner inherits. + +## Environment +- OS: no /etc/os-release; Docker container; NixOS kernel; kernel Linux 6.18.33 x86_64 +- libc: glibc 2.42-47 (interpreter /nix/store/wb6rhpznjfczwlwx23zmdrrw74bayxw4-glibc-2.42-47/lib/ld-linux-x86-64.so.2) +- Toolchain: gcc (GCC) 15.2.0, meson 1.9.1 +- Build: asan (xce.build) +- Code: scheduler-framework @ a370d724-dirty +- Deployment: n/a: found by code reading; affects any deployment scheduling COMPOUND/ALTER jobs + +## Reproduction +1. `git checkout a370d724`; read `src/scheduler/src/resource.c:130-169` + (`unsatisfy_`) and its two callers at `:309` and `:376`, and + `src/scheduler/src/schedobj.c:509-514,586-600` (dense 1024-entry uncommitted + vectors) and `src/lib/xvec.c` `DEFINE_XVEC_IDVAL_OP` (`d[idx] += val`). +2. Unit-test sketch (white-box `#include "resource.c"`, like + `test_resource_accounting.c`): one host with `mem` total 10 at resource idx 5 + (idx 0 = `slots`, plenty); set the host's uncommitted vector `[5] = 8` + (an earlier primary's claim); call `sched_check_host_satisfy()` with + `wanted = {idx 5: 4}`. Expected 0 (8 + 4 > 10); today it returns 1, because + it reads `uncomitted[0]` (= 0). +- Frequency: deterministic (by code reading; no test exercises it yet) +- Evidence: not kept (code reading only) + +## Root cause +TODO: required for fixed/partial + +## Fix +TODO: required for fixed/partial + +## Verification +- Test: TODO: name + file; RED before the fix, GREEN after +- Suites: TODO: suite results and build profile +- Live: TODO: soak/deploy run, or n/a and why + +## References +- Related: none diff --git a/.wolf/bugs/bug-1893.md b/.wolf/bugs/bug-1893.md new file mode 100644 index 00000000..05b960f7 --- /dev/null +++ b/.wolf/bugs/bug-1893.md @@ -0,0 +1,80 @@ +--- +id: "bug-1893" +status: "open" +status_updated: "2026-09-30" +status_note: "newly logged" +component: "scheduler" +severity: "wrong-result" +timestamp: "2026-09-30T03:58:52Z" +last_seen: "2026-09-30T03:58:52Z" +occurrences: 1 +files: ["src/scheduler/src/plugins/seqjob_allocator.c", "src/scheduler/src/plugins/parjob_allocator.c"] +tags: ["scheduler", "allocation", "preason", "code-review"] +related: ["bug-1892"] +--- + +# bug-1893: xce_scheduler allocators add spurious resource-0 shared pend reasons (loop reads past unmet list) + +## Description +**Observed:** found by code reading, so there is no failing run yet. When the shared-resource check +fails, both allocators add the pend reasons with +```c +for (i = 0; i < ctx->alloc_req.shared.count; i++) { + rc = vget(core->resmgr, ((int*)ctx->unmeet.idxs.data)[i], (void**)&r); + if (rc <= 0) continue; + add_job_preason(reason->reasons, PREASON_NO_RESOURCE, NULL, r->name); +} +``` +(`seqjob_allocator.c` `my_alloc`, `parjob_allocator.c` `my_alloc`). The loop +bound is the number of *requested* shared resources, but `ctx->unmeet` holds +only the *unmet* ones, packed at `[0, unmeet.count)` by `unsatisfy_()` +(`resource.c:152-165`). Its arrays were `calloc`ed with capacity +`shared.count` (`resource.c:280`), so entries `[unmeet.count, shared.count)` +are 0. + +**Expected:** iterate `i < ctx->unmeet.count` (as the host path does, +`resource.c:566`), giving one `NO_RESOURCE` reason per unmet shared resource. + +**Impact:** wrong pend reason, noise level. When a job requests N shared +resources and only M < N are unmet, the loop adds N - M extra +`PREASON_NO_RESOURCE` reasons naming whatever resource has index 0 (if one +exists), which that job may never have requested. Main reason and allocation +decision are unaffected. No out-of-bounds memory access (the tail is inside the +allocation). + +**Found by:** review of the `scheduler-allocator-runner` requirements +(2026-09-29); the loop moves into the framework runner in that spec. + +## Environment +- OS: no /etc/os-release; Docker container; NixOS kernel; kernel Linux 6.18.33 x86_64 +- libc: glibc 2.42-47 (interpreter /nix/store/wb6rhpznjfczwlwx23zmdrrw74bayxw4-glibc-2.42-47/lib/ld-linux-x86-64.so.2) +- Toolchain: gcc (GCC) 15.2.0, meson 1.9.1 +- Build: asan (xce.build) +- Code: scheduler-framework @ a370d724-dirty +- Deployment: n/a: found by code reading; any deployment where a job requests 2+ shared resources and only some are short + +## Reproduction +1. `git checkout a370d724`; read the loop in + `src/scheduler/src/plugins/seqjob_allocator.c` / `parjob_allocator.c` + `my_alloc()` (after `sched_check_shared_satisfy`), and + `src/scheduler/src/resource.c:130-169,266-316`. +2. Unit-test sketch: resource idx 0 = `slots`; two shared resources A (idx 7, + plenty) and B (idx 8, total 0); a seqjob primary requesting 1 of each. + Expected: exactly one host-less reason (`NO_RESOURCE`, res "B"); today a + second reason names `slots`. +- Frequency: deterministic (by code reading; no test exercises it yet) +- Evidence: not kept (code reading only) + +## Root cause +TODO: required for fixed/partial + +## Fix +TODO: required for fixed/partial + +## Verification +- Test: TODO: name + file; RED before the fix, GREEN after +- Suites: TODO: suite results and build profile +- Live: TODO: soak/deploy run, or n/a and why + +## References +- Related: none