Add a dedicated equivalence suite (test_seqjob_rewrap_equivalence.c) covering multi-host fixtures the trivial single-host test doesn't reach: select-filter-only rejection, resource-only rejection, a combined select-then-resource-then-success ordering, and per-host debit correctness. The iteration-order test is grounded in the deleted pre-rewrap sched_collect_workers() code path (verified via git show against the parent commit) and the unmodified xlist.c primitive, not merely in host_iter.c's own traversal, so it proves equivalence to the old behavior rather than internal self-consistency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XYdtrEWWZfKRRGED7raEYg
15 KiB
Implementation Plan
-
1. Foundation: allocation data types and extension points
-
1.1 Add the lazy plan-generation and host-candidate iterator contracts
- Define the plan representation as a partition of a job's task count into per-host slot buckets, most-preferred ordering to be produced by whichever component generates plans
- Define the plan-iterator and host-iterator contracts (initialize, produce next candidate, release resources) as reusable abstractions any allocator can adopt
- Observable: the contracts compile and are referenced by at least a stub/no-op implementation with no allocator wired in yet
- Requirements: 2.1, 3.1
-
1.2 Add resreq fingerprinting for the unsatisfiable-resreq cache
- Compute a stable resource-usage fingerprint once when a resource request is parsed
- Store that precomputed fingerprint on the job's scheduling object once at ingest, so every later cache lookup for that job reuses it instead of recomputing it (the full cache key, fingerprint plus user/queue/project identity, is still assembled fresh at each lookup)
- Write a unit test proving the fingerprint is stable across repeated computation for an identical resource request and differs when any contributing field changes
- Observable: a passing test demonstrates identical requests produce identical fingerprints and any differing field changes the fingerprint
- Requirements: 7.1
-
1.3 Add the forward-compatible allocation-mode field
- Add an allocation-mode field to the per-attempt allocation context, defaulting to normal-resources-only
- Observable: the field exists, defaults correctly, and no existing allocation code path branches on any value other than the default
- Requirements: 9.5
-
1.4 Add the per-host resource-debit reversal primitive
- Add a helper that reverses exactly the provisional resource debit an earlier successful host-claim made, using the existing subtraction primitive
- Write a unit test that debits a host's provisional usage, reverses it, and confirms the host's provisional usage returns to its pre-debit value
- Observable: a passing test shows debit-then-reverse leaves provisional usage unchanged from baseline
- Requirements: 4.2
-
2. Core: shared allocation infrastructure
-
2.1 (P) Implement and test the lazy host-candidate iterator
- Implement the shared host-candidate iterator against the existing select-expression evaluator and resource-availability check, with no allocator-specific logic
- Ensure each call advances a real cancellation point so a long-running scan can be safely interrupted
- Write a unit test proving a host too small for one candidate search remains a valid candidate for a later, smaller search using a freshly-initialized iterator over the same underlying worker set
- Observable: a passing test suite covers select-expression filtering, availability filtering, exhaustion signaling, and the too-small-then-fits-smaller-search case
- Requirements: 3.1, 3.2, 3.3, 3.4, 3.5, 3.6
- Boundary: host_iter.c
-
2.2 (P) Implement and test the unsatisfiable-resreq cache
- Implement the cache's key type and its hash/comparison/copy/release operations, backed by the existing keyed-table infrastructure, storing a reason category (not a bare present/absent marker) as each entry's value
- Implement lookup, insert, and clear operations against the cache
- Write a unit test proving a lookup on an identical fingerprint-plus-identity is a hit that returns the same reason category given at insert time, a lookup on any differing field is a miss, and clear empties all entries
- Observable: a passing test suite includes a same-key-twice hit assertion (not only an insert-then-miss-on-a-different-key round-trip) and a reason-category round-trip assertion
- Requirements: 7.1, 7.3, 7.4, 7.5
- Boundary: alloc_cache.c
-
3. Core: seqjob allocator rewrap
-
3.1 Rewrap seqjob's allocation onto the shared iterators
- Restructure seqjob's allocation to use the shared plan-iterator and host-iterator contracts as a trivial single-bucket case, replacing its own inline worker-collection loop
- Add a guard so seqjob's resource-request preparation only ever acts on single-task jobs, closing the fan-out gap this creates for the parallel allocator's later fix
- Observable: seqjob's allocation still runs end to end using the shared iterators, with no allocator-specific worker-collection logic left in seqjob's own code
- Requirements: 9.1, 4.6
-
3.2 Prove seqjob's rewrap preserves today's host-selection behavior
- Write a test suite constructing fixture states with multiple candidate hosts, including at least one host failing the select filter, asserting identical host selection to today's pre-rewrap behavior given identical worker/resource state
- Observable: a passing equivalence test suite demonstrates no behavior change across fixture states beyond the single-obvious-host trivial case
- Requirements: 9.1
- Depends: 3.1
-
4. Core: parjob combinatorial allocation
-
4.1 Implement parjob's lazy combinatorial plan generation
- Generate candidate plans as a pure function of a job's task count: ascending host-count order, one balanced partition per host-count, descending bucket order within each plan
- Ensure each call advances a real cancellation point and that plan generation never inspects live worker state (a pure function of the task count alone)
- Write a unit test proving one-plan-per-call generation, most-preferred-first ordering (including an uneven split, e.g. splitting 7 tasks across 3 hosts), and a stable exhaustion signal
- Observable: a passing test suite covers plan ordering, uneven-split partitioning, and exhaustion signaling
- Requirements: 2.1, 2.2, 2.3, 2.4, 2.5, 2.6, 4.1
-
4.2 Implement parjob's per-bucket allocation loop
- Replace the parallel allocator's stub with a real implementation that, for each candidate plan, claims a distinct host for every bucket using a freshly-initialized host search per bucket, excluding only hosts already claimed elsewhere in the same plan attempt
- Fix the parallel allocator's resource-request preparation to compute each bucket's want from that bucket's own slot count rather than a fixed value
- On a bucket failure, reverse the provisional debits already made for that plan's earlier successful buckets before advancing to the next candidate plan
- Return the same not-satisfied signal as before when every candidate plan is exhausted, leaving pend-reason handling unaffected
- Write a unit test proving a plan whose buckets are tried in descending size order succeeds where the same host pool would fail in ascending order, and a test proving a mid-plan bucket failure correctly reverses earlier successful claims before the next plan is tried
- Observable: a passing test suite covers multi-bucket success, the descending-vs-ascending worked example, and partial-failure-then-reversal
- Requirements: 4.2, 4.4, 4.5, 4.6, 4.7
- Depends: 3.1
-
4.3 Implement parjob's multi-host allocation commit
- Merge every claimed bucket's host allocation into a single job allocation record spanning all claimed hosts, leaving job-level shared-resource allocation computed once and unaffected by how many hosts were used
- Observable: a successfully allocated multi-host job's allocation record correctly lists every claimed host with its own slot count
- Requirements: 4.3
-
5. Integration: dispatch and cache wiring
-
5.1 (P) Wire the unsatisfiable-resreq cache around the allocator dispatch
- Before running full allocation for a primary resource request (whether the job's own request is a primary, or one nested inside a compound or an alternative), look up its fingerprint in the cache; on a hit, pend the job immediately using the stored reason category without invoking either allocator
- When an allocator exhausts every candidate plan, populate the cache with the job's fingerprint and the resulting reason category before pending the job
- Observable: a job sharing an identical fingerprint and identity with a previously-exhausted job is pended immediately with the same reason category, without either allocator running
- Requirements: 7.3, 7.4, 7.5
- Boundary: scheduler.c dispatch
- Depends: 2.2, 3.1, 4.2
-
5.2 (P) Clear the cache at the start of every scheduling session
- Clear the cache as part of the existing per-session reset step, alongside the other state that step already resets
- Observable: a job cached as unsatisfiable in one session is re-evaluated fresh (not immediately pended from the cache) in the next session
- Requirements: 7.2
- Boundary: core_session.c
- Depends: 2.2
-
5.3 End-to-end multi-slot parallel job allocation proof
- Drive a real multi-host parallel job through the full dispatch path from submission to a committed multi-host allocation
- Observable: the resulting job allocation spans more than one host with correct per-host slot counts, confirmed through the same dispatch path production jobs use
- Requirements: 4.1, 4.2, 4.3
- Depends: 5.1, 5.2, 4.3
-
6. Validation: regression and soak
-
6.1 Full existing scheduler regression pass
- Run the existing scheduler test suite, updating only the assertions that directly exercised the eager-one-shot or stub-return behavior this feature replaces
- Confirm the compound and alternative resource-request dispatch paths remain exercised, unmodified, by their existing tests
- Observable: the full existing scheduler suite passes with no unexpected assertion changes
- Requirements: 9.2, 9.3
- Depends: 5.3
-
6.2 HA soak regression
- Run a full sequential and chaotic HA regression soak
- Observable: the soak reports zero new job loss, divergence, or liveness stall attributable to this feature
- Requirements: 9.4
- Depends: 6.1
Implementation Notes
- (2.2)
xce_scheduler_t.unsatisfiable_cache(constructed insched_init()/destroyed insched_fini(), matchingxce_select.c'ssel_node_table_init()construction convention exactly) lives on the outerxce_scheduler_t, per Req 7 AC1. Task 5.2's clear call site (core_begin_session_internal(),core_session.c) only receivesxce_sched_core_t* core— sincecoreis confirmed the first field ofxce_scheduler_t, task 5.2 should recover the outer struct via(xce_scheduler_t*)core(valid, defined C behavior), not by widening the function's signature. - (2.1)
xce_alloc_plan_tgained actxfield (xce_sched_ctx_t*) sohost_iter_init()can reachctx->sched/ctx->primary->select/ctx->alloc_req.host—host_iter_init()rejects a plan withctx == NULLwith-EINVALrather than crashing. Tasks 3.1 and 4.1 must setplan->ctx = ctxon every plan their plan-iterator returns fromnext()(not just once) before constructing ahost_iterfrom it — a plan-iterator that reuses one internal buffer across calls/jobs must refresh this field each time, since a stale-but-non-NULL pointer from a torn-downctxis not caught by the NULL guard. - (1.4) When a new scheduler test needs a real function from
schedobj.c/core.c(e.g.sched_ctx_get_uncomit_host()), white-box#includethe real source file rather than privately reimplementing its logic in the test — established precedent:test_picker_config_fields.c's#include "../src/core.c"+#include "../src/schedobj.c"pairing. A private reimplementation may pass today but silently drifts from the real function with nothing to catch it later. - (1.1)
xce_host_iter_t.next()returnsxce_sched_worker_t*(sched/types.h, carriesresvals/resInUse/host_eval_ctx) — neverxce_worker_t*(xce/types.h, the wire/config typemake_sched_worker()converts FROM). requirements.md/design.md briefly had this wrong; fixed in commitd1a3e158. Task 2.1 (host_iter.c's real implementation) and any later task touching worker candidates must usexce_sched_worker_t*from the start. - (1.2)
resreq_fp's ingest-time population lives incore_build_primary_schedobj()(src/scheduler/src/core_events.c), right afterprimary->resreq = resreq;— confirmed wired (min_tasks/max_tasks/select_node_id/rusage_hash all set from the parsedxce_primary_resreq_t) as of task 1.2 round 2, regression-covered bytest_core_build_primary_schedobj_populates_resreq_fp_from_parsed_resreqinsrc/scheduler/testsuite/test_core_events.c. Round 1 added the type/field/hash machinery but never wired this call site, leavingresreq_fppermanently zeroed — do not rediscover this gap in a later task. - (3.1) The per-host try-alloc leaf (
sched_try_alloc_on_host_(xce_sched_worker_t* w, xce_scheduler_t* sched, xce_sched_ctx_t* ctx)— select check →sched_ctx_get_uncomit_host→plugin_prepare_allocation→sched_check_host_satisfy→xvec_idval(..., XVEC_ADD_FLOAT)) now lives inresource.c, exported (non-static, non-variadic) and declared inscheduler.h. Task 4.2 (parjob's per-bucket allocation loop) must call this exact shared function from its own bucket/host_iter loop, not write a second copy. Round 1 of this task wrote a private ~70-line duplicate (seqjob_try_alloc_on_host_) because the original wasstatic/va_list-based; round 2 fixed this by exporting the real function and deleting both the duplicate and the now-deadsched_try_alloc_on_hosts()variadic wrapper. This is the same reimplementation-vs-share risk task 1.4 hit in test code — here it recurred in production code, one task later. - (3.1)
src/scheduler/meson.build's productionscheduler_srcslist was missinghost_iter.csince task 2.1 only wired that file intotestsuite/meson.build— invisible until task 3.1 became the first production caller ofhost_iter.c's functions (a shared .c file with zero production callers builds/links fine without being inscheduler_srcsat all, so task 2.1's own green build/test run couldn't have caught this). Any future task adding a new shared scheduler.cfile should double check it lands inscheduler_srcs, not onlytestsuite/meson.build, even if nothing calls it yet. - (3.2)
host_iter.c's pre-filter select check andsched_try_alloc_on_host_()'s (resource.c) own independentsched_meet_host_selectionre-check are pre-existing, intentional defense-in-depth: both run the identical check against the same live state, so a regression in either one alone is invisible at the observable-pick level — only a regression in both simultaneously changes behavior. This is benign and predates this feature; task 4.2 (parjob) will inherit the same double-check when it reuseshost_iter.c/sched_try_alloc_on_host_(), and should not "fix" the redundancy as part of that task. - (3.2)
sched_collect_workers()'s deleted (task 3.1) iteration andhost_iter.c'siter_make()/iter_next()iteration are structurally identical, not coincidentally similar: both called the exact same third-partyiter_make()/iter_next()(RTTI lib) over the literal samesched->core.workersvar, and the oldxlist_find()walk was forward/insertion-order — confirmed viagit show b18c5f78^:src/scheduler/src/scheduler.candxlist.c(unmodified across this feature). Useful precedent if task 4.1/4.2's parjob equivalence claims ever need the same grounding.