myxce/.kiro/specs/scheduler-allocator-runner/brief.md
Rongsong Shen e2266582b2
docs(scheduler-allocator-runner): requirements, design and tasks; log bug-1892 and bug-1893
Requirements reviewed and revised (seqjob per-bucket reset, nslots/nhosts,
bug-1893 fix in the runner), design and 11-task plan generated and reviewed.
bug-1892: unsatisfy_() reads index-keyed uncommitted amounts by position.
bug-1893: allocators' shared pend-reason loop reads past the unmet list.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-09-30 07:35:14 +00:00

11 KiB

Brief: scheduler-allocator-runner

Problem

Scheduler plugin authors and maintainers. seqjob_allocator.c (my_alloc, line 213, ~215 lines) and parjob_allocator.c (my_alloc, line 545, ~220 lines) each own the entire allocation algorithm, although only plan generation is plugin-specific. ~100+ lines are duplicated verbatim (lazy pend-reason, sched_ctx_get_uncomit_shared, sched_check_shared_satisfy + PREASON_NO_RESOURCE fill, add-uncommitted-shared, plan-iterator loop skeleton, commit tail with sched_make_allocs + -ENOMEM cleanup chain). Every bug fix (bug-1882, bug-1767, bug-1768) has to land twice, and a new allocator plugin must copy the whole my_alloc(). Driver: issues/new_requirements.md item 4.

Current State

  • Framework dispatch sched_make_primary_alloc_() (scheduler.c:235) picks core->seqjob_allocator (maxTasks == 1) or core->parjob_allocator and calls plugin->job_alloc_fn(plugin, ctx).
  • Plan generation is already abstracted behind xce_alloc_plan_iter_t (include/sched/alloc_iter.h); host scan behind xce_host_iter_t.
  • The picker side already uses a framework-owned iface pattern (xce_job_picker_iface_t, plugin->picker_iface, plugin.h:88).
  • alloc_iter.h init's third parameter is xce_scheduler_t*, unused by both implementations ((void)sched); sim mode has no real scheduler.
  • parjob has parjob_try_plan_ / parjob_rollback_claims_ / parjob_snapshot_host_req_ claim bookkeeping; seqjob keeps a single w.

Desired Outcome

  • Neither plugin defines my_alloc(); both register .job_alloc_fn = sched_allocator_run and supply only an xce_sched_allocator_iface_t (optional accepts guard + plan iterator) plus their existing prepare_allocation.
  • 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. 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

Framework runner, lean (chosen 2026-09-28). The design proposal of 2026-09-23 (§5), with the optional commit hook left out because nothing uses it yet:

  • xce_sched_allocator_iface_t { accepts; plan_init; plan_next; plan_fini; } in plugin.h; allocator_iface pointer on xce_sched_plugin_t with the same ownership as picker_iface.
  • sched_allocator_run() = guards -> accepts -> sched_alloc_prepare_shared_() -> plan loop -> sched_try_plan_() (promoted parjob_try_plan_) -> sched_finalize_job_alloc_() (one make_host_alloc per claim -> make_job_alloc -> allocs_shared block).
  • Retype alloc_iter.h init's third parameter to xce_sched_core_t*.
  • Staged migration keeps the suite green: (1) extract shared helpers used by both existing my_alloc()s; (2) runner + iface, parjob first; (3) seqjob; (4) retype plan_init; (5) close item 4.

Rejected: adding the commit override hook (an unused extension point that would need its own tests); only extracting helpers (would not meet item 4's goal that plugins no longer export my_alloc()).

Scope

  • 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; the bug-1893 loop-bound fix (Constraints item 6); update issues/new_requirements.md item 4.
  • Out: see Out of Boundary.

Boundary Candidates

  • Plugin ABI (plugin.h iface + plugin template wiring)
  • Framework allocation runner (new plugin_alloc.c or scheduler.c) — shared-resource prep, claim loop/rollback, commit tail
  • Plan-iterator contract (alloc_iter.h retype; seqjob/parjob plan iters)

Out of Boundary

  • Retyping sched_make_allocs() to xce_sched_core_t* and removing the (xce_scheduler_t*)plugin->sched cast (follow-up; callers outside allocators).
  • 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, except the decided seqjob host-reason change in Constraints item 1.

Upstream / Downstream

  • Upstream: scheduler-allocation (plan/host iterators, Req 4 combinatorial parjob, Req 9 no-regression); scheduler-core (plugin/picker iface pattern, sim mode); libxce-alloc-preason-equality (job_alloc_eq used by sim verify).
  • Downstream: any future allocator plugin (it would need only a plan iterator); scheduler-preemption; the follow-up sched_make_allocs retype.

Existing Spec Touchpoints

  • Extends: none (scheduler-allocation is implementation-complete; its Req 9 is inherited as a constraint, not reopened).
  • Adjacent: scheduler-allocation, scheduler-core, scheduler-sim-discrete-clock (in flight, touches sim_loop — avoid overlapping files), libxce-alloc-preason-equality.

Constraints

  • TDD: tests in src/scheduler/testsuite/ first; full ASAN build via /xce-build scheduler.
  • 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, 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. 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, test_maxtasks_wiring_*, test_sim_*_parity, test_sim_live_allocation, test_snapshot_load_allocator_regression; /xce-audit-invariants scheduler (driver state-equivalence).