test(scheduler-allocator-runner): characterize the seqjob allocator (task 2.1)
12 tests drive the real seqjob plugin through job_alloc_fn and record today's results, reasons and debits, with switch-stable OOM injection (sched_make_allocs stub, --wrap of make_host_alloc/make_job_alloc). Assertions the runner changes on purpose sit in *_today tests. The characterization showed bug-1893's real effect: vget() returns 0 on success, so the shared-reason loop (rc <= 0 skip) never records a shared NO_RESOURCE reason. Bug record and spec wording corrected; the runner skips only on rc < 0. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
31ad79d325
commit
91a8580948
8 changed files with 835 additions and 16 deletions
|
|
@ -36,7 +36,7 @@ Driver: `issues/new_requirements.md` item 4.
|
|||
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
|
||||
recorded for the first time (bug-1893, item 6); seqjob gets the `-EINVAL` guard and OOM
|
||||
rollback (Req 5 AC 1/5).
|
||||
- `new_requirements.md` item 4 closed.
|
||||
|
||||
|
|
@ -173,12 +173,14 @@ goal that plugins no longer export `my_alloc()`).
|
|||
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.
|
||||
6. **Decided 2026-09-29 (requirements review), corrected 2026-09-30 (task
|
||||
2.1): the shared pend-reason loop iterates the unmet list
|
||||
(`ctx->unmeet.count`) and skips only on `vget() < 0`, fixing bug-1893.**
|
||||
Both current `my_alloc()`s skip on `vget() <= 0`, but `vget` returns 0
|
||||
on success, so today no shared `NO_RESOURCE` reason is ever recorded
|
||||
(the loop also runs to `alloc_req.shared.count`). After the fix a job
|
||||
pending on shared resources names each short one; 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`,
|
||||
|
|
|
|||
|
|
@ -269,7 +269,7 @@ const xce_sched_allocator_iface_t* allocator_iface;
|
|||
|
||||
**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).
|
||||
- `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 only on `rc < 0` (`vget` returns 0 on success; today's `rc <= 0` skips every hit, bug-1893), `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.
|
||||
|
|
|
|||
|
|
@ -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 (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`.
|
||||
**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 shared-resource reasons bug-1893 never recorded, 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 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.
|
||||
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 today are never recorded (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.
|
||||
|
|
@ -41,7 +41,7 @@ Terms:
|
|||
|
||||
#### Acceptance Criteria
|
||||
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.)
|
||||
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 skip every successful resource lookup, because the lookup returns 0 on success and the loop skips on a result of 0 or less, so no shared-resource reason is ever recorded; the loop also runs over the number of requested shared resources instead of the unmet ones.)
|
||||
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.
|
||||
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@
|
|||
- _Requirements: 1.2, 1.6_
|
||||
|
||||
- [ ] 2. Characterization tests against the current allocators
|
||||
- [ ] 2.1 Characterize the sequential-job allocator through its registered entry point
|
||||
- [x] 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)
|
||||
|
|
@ -58,7 +58,7 @@
|
|||
|
||||
- [ ] 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
|
||||
- First add the parjob bug-1893 fixture (two shared resources, one short: exactly one host-less reason, naming the short one; today none is recorded) 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
|
||||
|
|
@ -67,7 +67,7 @@
|
|||
- _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)
|
||||
- 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 (the 2.1 fixture that records no shared reason today now expects one host-less reason naming the short resource); 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)
|
||||
|
|
@ -94,3 +94,6 @@
|
|||
## Implementation Notes
|
||||
- Build/test only in the dedicated dir `src/build.alloc` (`bash src/tools/xce-build.sh scheduler asan build.alloc --jobs N --incremental`; baseline 90/90). Another session uses `src/xce.build`.
|
||||
- Several white-box scheduler test targets run with `detect_leaks=0` because their fixtures are not torn down (`testsuite/meson.build:1756`); a leak summary from running such a binary directly is pre-existing, not a regression. New tests that assert "no leak" (3.3) need leak detection on for their own target.
|
||||
- bug-1893 (found in 2.1): `vget()` returns 0 on success, so today's shared-reason loop (`rc <= 0` skip) records NO shared reason at all. The runner must skip only on `rc < 0`. Characterization fixtures record "no shared reason today" in a separate `_today` test that 4.1/4.2 rewrite.
|
||||
- `xce_sched_core_fini()` does not free the string table `xce_sched_core_init()` creates; leak-checked test targets call `xstring_table_fini()` in teardown (see `test_alloc_equivalence_seqjob.c`).
|
||||
- `test_alloc_equivalence_seqjob.c` is at ~794/800 lines: 4.2's new fixtures go in a second file or after trimming.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue