docs(scheduler-allocation): approve requirements, add design and tasks
Extensive requirements review found and fixed a real correctness bug (host_iter shared across plan buckets instead of re-scanned fresh per bucket, which would silently drop solvable multi-host allocations), a self-contradiction around parjob_prepare_allocation's scope, a plan/host-iterator init calling-convention mismatch, and an inverted kvtab .cmp polarity that would have made the unsatisfiable-resreq cache permanently non-functional. brief.md brought back into sync with requirements.md's corrected architecture. design.md and tasks.md generated and validated (kiro-validate-design GO after two fixes; independent task-graph sanity review PASS). Two pre-existing, out-of-boundary gaps found along the way logged as bug-1766/bug-1767. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XYdtrEWWZfKRRGED7raEYg
This commit is contained in:
parent
c4f2fec0d5
commit
48c789dfcb
7 changed files with 1044 additions and 75 deletions
|
|
@ -20860,6 +20860,50 @@
|
|||
"related_bugs": [],
|
||||
"occurrences": 1,
|
||||
"last_seen": "2026-09-03T14:51:54Z"
|
||||
},
|
||||
{
|
||||
"id": "bug-1766",
|
||||
"timestamp": "2026-09-04T08:03:28Z",
|
||||
"error_message": "OPEN, NOT A CRASH -- discovered while reviewing the scheduler-allocation spec requirements against live scheduler.c source: sched_make_compound_allocation() does not roll back already-succeeded primaries when a LATER primary in the same COMPOUND resreq fails allocation, and its own post-loop code can misattribute a partial allocation to a failed compound.",
|
||||
"root_cause": "sched_make_compound_allocation() (src/scheduler/src/scheduler.c:206-238) loops over a COMPOUND resreq's primaries, calling sched_make_primary_alloc_(sched, &ctx) for each with ONE shared ctx across the whole loop. On a mid-loop failure (rc <= 0) it just `break`s -- it never undoes the primaries that already succeeded (no zeroing of ctx.alloc, no reversal of any resource state those primaries touched). Worse: seqjob/parjob's job_alloc_fn sets ctx->alloc directly on success (see seqjob_allocator.c my_alloc()), so if primary 1 of a 2-primary compound succeeds (leaving ctx.alloc non-NULL) and primary 2 then fails, the function's unconditional post-loop `if (ctx.alloc != NULL) { MOVE_PTR(obj->alloc, ctx.alloc); }` still fires regardless of the overall `rc <= 0` -- so a COMPOUND job that is reported as allocation-FAILED can still end up with obj->alloc set to primary 1's partial, orphaned allocation. requirements.md for scheduler-allocation (Req 9 AC2) had assumed, based on the sibling scheduler-framework brief's own text, that this path already 'tracks allocations in ctx and cleans up on failure' -- that claim does not hold against the actual code; requirements.md was corrected during this review. Not yet confirmed whether the orphaned ctx.alloc is ever read for a job later found to be PEND/failed (i.e. whether this is dormant/leak-only or produces an observably wrong result) -- not investigated further since it is out of the scheduler-allocation spec's own boundary (Req 9 AC2 explicitly freezes COMPOUND/ALTER dispatch as unchanged).",
|
||||
"fix": "NOT FIXED -- out of scope for the scheduler-allocation spec (which explicitly leaves COMPOUND/ALTER dispatch unchanged, Req 9 AC2) and no other spec currently owns sched_make_compound_allocation(). Discovered only because scheduler-allocation's own NEW per-plan rollback (Req 4's rollback_plan_allocation(), for the parjob multi-host-bucket case) needed a reusable precedent to model itself on; this path was found not to provide one. Needs its own follow-up: likely either (a) track which primaries in the compound loop actually succeeded and explicitly free/clear their contribution to ctx.alloc before returning on a later failure, or (b) defer the MOVE_PTR(obj->alloc, ctx.alloc) until the loop confirms overall success, freeing ctx.alloc on the failure path instead. Whether this ever produces an observably wrong result (vs. a dormant orphaned pointer) is unconfirmed and should be checked first.",
|
||||
"tags": [
|
||||
"scheduler",
|
||||
"compound-resreq",
|
||||
"rollback",
|
||||
"partial-allocation",
|
||||
"open-gap",
|
||||
"scheduler-allocation-spec",
|
||||
"requirements-review",
|
||||
"not-fixed"
|
||||
],
|
||||
"related_bugs": [],
|
||||
"occurrences": 1,
|
||||
"last_seen": "2026-09-04T08:03:28Z"
|
||||
},
|
||||
{
|
||||
"id": "bug-1767",
|
||||
"timestamp": "2026-09-04T08:03:28Z",
|
||||
"error_message": "OPEN, NOT A CRASH -- recurring pattern finding, not a single defect: snapshot_load.c's local, hand-duplicated reconstruction of xce_sched_obj_t/schedobj (kept independent of core_events.c's core_build_schedobj() because that helper is `static` and snapshot.c/snapshot_load.c are compiled as separate objects) has now caused THREE independent field-completeness gaps in the same narrow reconstruction path, each discovered separately.",
|
||||
"root_cause": "snapshot.c's own header comment (Why not reuse core_events.c's core_build_job/core_build_queue/core_build_worker, snapshot.c:296-314) documents the deliberate choice to keep snapshot_load.c's job/queue/worker/schedobj reconstruction as a second, independent, field-for-field-identical implementation rather than exporting core_events.c's static builders -- justified there for core_build_job/queue/worker specifically (they also need an independent SAVE-direction implementation regardless, and funneling through xce_core_push_events() reintroduces a config/queue double-application bug). That reasoning does not equally apply to core_build_schedobj() alone: schedobj is never wire-captured (no build_wire_schedobj(), no SAVE-direction counterpart needed) and has no xce_core_push_events() involvement, so the only real objection to exporting it is 'larger, unrequested change to core_events.c's surface' -- a scope call, not an architectural wall. This duplication has now independently produced: (1) scheduler-snapshot-job-allocation spec's own motivating bug -- job->allocs/job->accounts never restored from snapshot (documented, deliberate scope decision at the time, later fixed); (2) the same spec's own further finding -- every snapshot-restored job (PENDING or RUNNING) got schedobj==NULL, an undocumented latent crash risk, fixed by adding a local field-for-field mirror of core_build_schedobj() to snapshot_load.c; (3) scheduler-allocation spec (this one) -- the new xce_resreq_fingerprint_t resreq_fp field added to xce_primary_schedobj_t (Req 7) is computed at core_build_schedobj()'s call site but has no reason to be computed by snapshot_load.c's independent copy, silently degrading Req 7's unsatisfiable-resreq cache to a permanent miss for any session started via snapshot+replay (the documented common real-investigation path) -- addressed in scheduler-allocation's own requirements.md (Req 7 AC 6) as a one-line addition, not by eliminating the duplication.",
|
||||
"fix": "NOT FIXED -- each individual instance above was/will be fixed at its own site (additively, preserving the duplication), not by addressing the duplication itself. Recommended follow-up, out of scope for any single already-scoped spec: export core_build_schedobj() (with a signature change to take user/queue/project strings directly instead of a full xce_job_t*, since snapshot_load.c has the restored strings but not a real xce_job_t/submit struct at reconstruction time) and have snapshot_load.c call it instead of maintaining its own copy -- eliminating the ONE remaining independent-reconstruction risk in this path that isn't justified by a genuine SAVE-direction or xce_core_push_events() concern the way core_build_job/queue/worker's duplication is. Tag for persistence.md's 'Snapshot Save/Load Field-Completeness (Scheduler)' section and xce-audit-invariants scheduler dimension 4 -- this is exactly the class of gap that invariant was formalized to catch, and has now recurred a third time in the same function.",
|
||||
"tags": [
|
||||
"scheduler",
|
||||
"snapshot",
|
||||
"schedobj",
|
||||
"field-completeness",
|
||||
"duplication",
|
||||
"open-gap",
|
||||
"persistence",
|
||||
"xce-audit-invariants",
|
||||
"not-fixed",
|
||||
"recurring-pattern"
|
||||
],
|
||||
"related_bugs": [
|
||||
"bug-1766"
|
||||
],
|
||||
"occurrences": 3,
|
||||
"last_seen": "2026-09-04T08:03:28Z"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue