From 58e6a855f966647996021644fed8970cb0b3d22d Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sat, 5 Sep 2026 02:10:49 +0000 Subject: [PATCH] chore(scheduler-allocation): confirm full regression pass (task 6.1) Audit-only: confirmed all 4 pre-existing scheduler test files touched by this feature (meson.build, test_core_events.c, test_job_update.c, test_resource_accounting.c) contain only pure additions or link-scaffold replacement, zero altered assertions; confirmed every existing COMPOUND/ALTER-dispatch test is byte-identical to its pre-feature state despite task 5.1's shared sched_make_primary_alloc_() change. Full suite: 74/75 OK, sole failure the pre-existing, unrelated bug-1758 (Req 9.2, 9.3). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01XYdtrEWWZfKRRGED7raEYg --- .kiro/specs/scheduler-allocation/tasks.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.kiro/specs/scheduler-allocation/tasks.md b/.kiro/specs/scheduler-allocation/tasks.md index 2c100e82..d21a0520 100644 --- a/.kiro/specs/scheduler-allocation/tasks.md +++ b/.kiro/specs/scheduler-allocation/tasks.md @@ -92,7 +92,7 @@ - _Depends: 5.1, 5.2, 4.3_ - [ ] 6. Validation: regression and soak -- [ ] 6.1 Full existing scheduler regression pass +- [x] 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 @@ -124,4 +124,5 @@ - (5.1) Two significant, pre-existing, out-of-scope bugs were discovered and confirmed during this task's review, logged as bug-1768 (compound/alter-nested primaries never have real slot/rusage availability checked — `seqjob_prepare_allocation()`/`parjob_prepare_allocation()` both no-op when `ctx->schedobj->tag != XCE_RESREQ_PRIMARY`) and bug-1769 (`sched_make_alter_allocation()` type-confuses `xce_compound_schedobj_t*` as `xce_sched_obj_t*`, a ~112-byte heap over-read, currently completely untested — the existing test believed to cover ALTER dispatch actually parses as PRIMARY per the resreq grammar). Neither is in this feature's scope to fix; both need dedicated follow-up. - (5.2) `core_begin_session_internal()` (`core_session.c`) is called from TWO structurally incompatible types sharing the same `xce_sched_core_t core` prefix: `driver.c` (`xce_scheduler_t*`) and `sim_loop.c` (`xce_sched_sim_driver_t*`, deliberately isolated, no cache field). Round 1's `__attribute__((weak))` cast-based approach caused every simulation session to silently corrupt `xce_sched_sim_driver_t.alt_events_file` (same byte offset as `xce_scheduler_t.unsatisfiable_cache`), invisible to ASAN (in-bounds) and to the whole test suite (no `test_sim_*.c` links `alloc_cache.c`). **Any future per-session hook that needs the real `xce_scheduler_t` (not just the shared `xce_sched_core_t` prefix) must use the driver-registered-callback pattern** (`xce_sched_core_t.cache_clear_cb`, set via `main.c` only in the driver-mode branch — `--sim` mode returns before reaching that registration — mirroring the existing `on_decision`/`xce_core_set_decision_cb()` precedent), never a cast of `core` back to `xce_scheduler_t*` inside code shared by both driver and sim paths. - (5.2) The real `cache_clear(xce_scheduler_t*)`→`xce_alloc_cache_clear_cb(xce_sched_core_t*)` adapter lives in `alloc_cache.c`, not `driver.c`, specifically because ~11 test files white-box `#include "../src/driver.c"` without linking `alloc_cache.c` — placing the adapter in `driver.c` would break their link. Any future adapter bridging a driver-only hook to a subsystem those tests don't link should follow this same placement rule (adapter lives with the subsystem it calls into, not with `driver.c`). +- (6.1) Regression audit (base `c4f2fec0` vs. HEAD `c009f108`, full feature range): only 4 pre-existing `src/scheduler/testsuite/` files were touched (`meson.build`, `test_core_events.c`, `test_job_update.c`, `test_resource_accounting.c`), all pure additions or link-scaffolding replacement — zero existing assertions altered. All COMPOUND/ALTER-referencing existing tests (`test_snapshot_load_allocator_regression.c`, `test_sim_record_output.c`, `test_sim_alt_events.c`, `test_snapshot_save_load.c`, `test_scheduler_backpressure.c`) confirmed byte-identical to pre-feature state despite task 5.1 modifying their shared `sched_make_primary_alloc_()` funnel. One benign, out-of-formal-scope gap noted: `src/lib/testsuite/test.c` (a different testsuite tree) also got a 2-line pure-wiring addition for task 1.2's new fingerprint test, omitted from the audit's "4 files" framing but not an altered assertion. - (5.3) **CRITICAL, feature-level finding**: `parjob_allocator` (tasks 4.1-4.3, 5.1-5.2, all already implemented/reviewed/committed) has ZERO live production path today. `parse_resreq_primary()` (`src/lib/resreq.c:431-432`) unconditionally hardcodes `primary->minTasks = 1; primary->maxTasks = 1` regardless of what the job actually requests — this predates the entire scheduler-allocation feature by ~7 months (commit `cd4f3953`, 2026-02-05), untouched by task 1.2's diff. The real, user-facing submission pipeline (`apiserver/models/jobs.go`'s `num_processors`/`maxnum_processors` → `controllers/jobs.go` → `backend/convert.go:286-292` → `controller.capnp` → `core_build_job()`/`make_sched_job()`) correctly threads the requested task count onto the TOP-LEVEL `xce_sched_job_t.mintasks/maxtasks` — but NOTHING ever copies that onto the NESTED `xce_primary_resreq_t.minTasks/maxTasks` that `sched_make_primary_alloc_()`'s seqjob-vs-parjob dispatch branch (`scheduler.c:290`) and `parjob_allocator.c`'s `my_alloc()` actually read. Separately, the resreq grammar's own `span[ptile=N]` syntax parses into `xce_resreq_data_t.spanString` and is then discarded (`primary->span` stays a stub empty struct) — a fix needs to reconcile BOTH the job-level submitted task count AND resreq's own span syntax, not just wire one through. Logged as bug-1770 (with bug-1771, a related fcfs NULL-queue-deref crash discovered while building task 5.3's fixture). **This must be a dedicated, high-priority follow-up task/spec — the scheduler-allocation feature has no practical effect on real clusters until it lands.**