chore(resreq-maxtasks-wiring): close out feature-level validation

/kiro-validate-impl returned GO: 21/21 requirement ACs covered, zero
boundary violations (parjob_allocator/seqjob_allocator/alloc_cache/
scheduler.c untouched, no revalidation trigger fired), zero blocked
tasks, cross-task integration chain (job_submit_data_t ->
apply_job_task_count() -> xce_primary_resreq_t -> scheduler dispatch
-> controller/worker launch) confirmed real end to end.

Closes two minor documentation gaps the audit found:
- tasks.md Implementation Notes now records tasks 2.1/2.2's colocated
  unit tests (in pre-existing test_core_events.c/test_snapshot_save_
  load.c) as a deliberate, reviewed File Structure Plan deviation.
- bug-1770's buglog.json entry updated from OPEN/NOT FIXED to FIXED,
  referencing this feature's commits and end-to-end verification.

spec.json phase set to implementation-complete.

This closes out all 12 sub-tasks of the resreq-maxtasks-wiring
feature.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XYdtrEWWZfKRRGED7raEYg
This commit is contained in:
Rongsong Shen 2026-09-06 10:48:06 +00:00
parent 5c87f1ccd2
commit a221b6ae58
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
3 changed files with 8 additions and 5 deletions

View file

@ -1,9 +1,9 @@
{
"feature_name": "resreq-maxtasks-wiring",
"created_at": "2026-09-05T07:18:28Z",
"updated_at": "2026-09-05T19:52:00Z",
"updated_at": "2026-09-06T10:20:00Z",
"language": "en",
"phase": "tasks-generated",
"phase": "implementation-complete",
"approvals": {
"requirements": {
"generated": true,
@ -19,6 +19,8 @@
}
},
"ready_for_implementation": false,
"implementation_complete": true,
"validate_impl_decision": "GO",
"related_specs": ["scheduler-allocation"],
"related_bugs": ["bug-1770"],
"motivation": "scheduler-allocation's task 5.3/6.2 review confirmed parjob_allocator's entire multi-host combinatorial allocation path (implemented, reviewed, and soak-tested by that feature) has zero live production reachability today, because no real job submission can ever produce xce_primary_resreq_t.maxTasks > 1. This spec exists to close that gap."

View file

@ -91,4 +91,5 @@
## Implementation Notes
- Task 4.1's `xce-test-multihost` harness reuses a fixed, wipe-and-recreate deploy directory (`xce-test-ha`'s convention) instead of design.md's stated "fresh, uniquely-named cluster instance; no shared state across runs" bullet (`xce-test-resources`'s convention). Reason: task 4.1's own text requires "the existing soak-test convention for pre-flight zombie-process/port checks," and a meaningful pre-flight check needs a stable, known directory/port to check against — `xce-test-resources`'s fresh-mktemp-per-run model has no such check at all (confirmed: it has none) because one would be moot there. The "no shared state" intent is preserved functionally: `deploy()` unconditionally wipes and recreates the directory on every invocation, verified clean (zero cross-run residue) across two consecutive live runs. No production code or requirement scope changed.
- Task 3.2's driver/simulation-parity test was split into a second new file, `src/scheduler/testsuite/test_maxtasks_wiring_sim_driver_parity.c` (registered separately in `meson.build`), rather than living inside `test_maxtasks_wiring_ingest.c` as originally scoped. Reason: proving this specific claim requires white-box `#include`-ing `sim_loop.c`/`core_events.c` to reach their real `static` translators (the only way to avoid a mirrored test that can't catch a real translator regression — confirmed via an independent mutation-test reproduction during review), but `test_maxtasks_wiring_ingest.c` already links the full real `scheduler_srcs` object list for its own task-3.1 test, and the two techniques collide at link time (duplicate symbols) in one translation unit. No production code or requirement scope changed; this is a file-boundary split only, driven by a real toolchain constraint discovered during implementation.
- Task 2.1 surfaced an out-of-boundary side effect: `src/scheduler/testsuite/test_repl_breakpoints_inspection.c:735`'s `test_info_job_reports_state_queue_priority` hardcodes an incidental `"Tasks: min = 0, max = 0"` byproduct of its fixture's `parse_select()` failing on an unregistered `"type"` resource (which previously left the primary's task-count fields at memset-zero, before `apply_job_task_count()` existed). Now correctly overwritten to `(1,1)` per Requirement 1.2. Before task 5.1 is considered complete, change that assertion's expected substring to `"Tasks: min = 1, max = 1"`.
- Task 2.1 surfaced an out-of-boundary side effect: `src/scheduler/testsuite/test_repl_breakpoints_inspection.c:735`'s `test_info_job_reports_state_queue_priority` hardcodes an incidental `"Tasks: min = 0, max = 0"` byproduct of its fixture's `parse_select()` failing on an unregistered `"type"` resource (which previously left the primary's task-count fields at memset-zero, before `apply_job_task_count()` existed). Now correctly overwritten to `(1,1)` per Requirement 1.2. Before task 5.1 is considered complete, change that assertion's expected substring to `"Tasks: min = 1, max = 1"`. (Applied in task 5.1, commit c454f065.)
- Tasks 2.1 and 2.2 each added their own focused white-box test directly into the pre-existing scheduler test files for the call sites they wired (`test_core_events.c`: `test_core_build_job_wires_real_task_count_into_resreq_primary`, `test_core_build_job_default_task_count_still_one`; `test_snapshot_save_load.c`: `test_snapshot_load_wires_real_task_count_into_resreq_primary`) rather than deferring all ingest-level test coverage to task 3.1's new `test_maxtasks_wiring_ingest.c` file. design.md's File Structure Plan doesn't list these two existing files under "Modified Files." Reason: each one-line production wiring change needed its own colocated, immediate proof (matching this codebase's own established convention of a test living alongside the function it directly modifies); task 3.1's later file added complementary cross-call-site comparative and behavioral-parity coverage that neither of these narrower tests attempted (see task 3.1's own text). No production code or requirement scope changed; feature-level validation (`/kiro-validate-impl`) confirmed both additions are correct, non-redundant, and appropriately scoped.

View file

@ -20951,9 +20951,9 @@
{
"id": "bug-1770",
"timestamp": "2026-09-04T00:00:00Z",
"error_message": "OPEN, NOT A CRASH -- pre-existing, out-of-scope gap discovered during scheduler-allocation task 5.3's end-to-end integration test. The REAL job-ingest path can NEVER produce a primary resource request with maxTasks > 1, regardless of what task count a real job submission specifies -- parjob_allocator.c's my_alloc() (maxTasks > 1) is therefore UNREACHABLE from any real production job submission today.",
"error_message": "FIXED (2026-09-06, resreq-maxtasks-wiring feature, commits 799a71ec..5c87f1cc). Originally: pre-existing, out-of-scope gap discovered during scheduler-allocation task 5.3's end-to-end integration test. The REAL job-ingest path could NEVER produce a primary resource request with maxTasks > 1, regardless of what task count a real job submission specified -- parjob_allocator.c's my_alloc() (maxTasks > 1) was therefore UNREACHABLE from any real production job submission.",
"root_cause": "CONFIRMED via direct code read and a genuine RED-phase run of test_parjob_e2e_multislot.c (src/scheduler/testsuite/): parse_resreq_primary() (src/lib/resreq.c:431-432) unconditionally hardcodes `primary->minTasks = 1; primary->maxTasks = 1;` after parsing a resreq string's select/rusage/span/same/affinity clauses, regardless of their content -- there is no resreq-string syntax for task count at all (LSF-style task count is a job-submission-level parameter, `job_submit_data_t.mintasks/maxtasks`, sibling to the resreq string, not nested inside it). Neither of this codebase's two job-ingest constructors -- core_build_job()/core_build_primary_schedobj() (src/scheduler/src/core_events.c, the real production JOBS_PUSH path) nor make_sched_job() (src/scheduler/src/schedobj.c, the sibling non-event construction path) -- ever copies the job's own submitted job_submit_data_t.mintasks/maxtasks (correctly propagated onto the TOP-LEVEL xce_sched_job_t.mintasks/maxtasks by both functions, confirmed working) onto the NESTED xce_primary_resreq_t.minTasks/maxTasks that sched_make_primary_alloc_()'s maxTasks>1 dispatch check (scheduler.c:290) and parjob_allocator.c's my_alloc() actually read. Net effect: every job's primary resreq that parses successfully has maxTasks pinned to 1, so sched_make_primary_alloc_() always routes it to seqjob_allocator instead of parjob_allocator (scheduler.c:290-294) -- parjob_allocator's real combinatorial multi-host allocation (scheduler-allocation spec, tasks 4.1-4.3) is fully implemented and unit-tested but cannot be exercised by any real job submission as the system stands today. Live-confirmed: test_parjob_e2e_multislot.c's own RED-phase run (maxTasks patch temporarily disabled) showed the job routing to seqjob and dispatching onto exactly 1 host instead of the intended 2 (`Expected 2 Was 1`).",
"fix": "NOT FIXED -- discovered and characterized during scheduler-allocation task 5.3's review (2026-09-04), deliberately left out of that task's boundary (an end-to-end TEST task, not scoped to touch resreq-parsing/job-submission plumbing). Fixing requires a real design decision: parse_resreq_primary()'s signature would need the job's task count threaded in from BOTH real call sites (core_build_job()/core_build_primary_schedobj() and make_sched_job()), plus a decision about how a COMPOUND resreq's several nested primaries should each receive a single job-level task count (today's model has exactly one job-level mintasks/maxtasks pair, but a COMPOUND can nest multiple independent primaries). Well outside scheduler-allocation's own Boundary Context (the iterator/allocator rework, not resreq-parsing/job-submission wiring) -- exactly the same class of pre-existing, out-of-scope gap tasks 5.1/5.2 already found and deferred in this same feature (bug-1766, bug-1768, bug-1769). test_parjob_e2e_multislot.c's own fixture compensates with one minimal, explicitly-labeled patch of the real, already-ingested xce_primary_resreq_t's task-count fields (PATCH_MAXTASKS_FOR_BUG_1770 in that file) immediately after real JOBS_PUSH ingest and before the dispatch round runs -- everything downstream of that point (cache lookup, parjob dispatch, plan iteration, host claiming, multi-host commit, decision production) is exercised through the unmodified real dispatch path. Needs its own dedicated follow-up task/spec given the severity (parjob_allocator is effectively dead code in production without this fix) and the cross-cutting design decision required for COMPOUND.",
"fix": "FIXED (2026-09-06, resreq-maxtasks-wiring feature, commits 799a71ec..5c87f1cc, 12 tasks). Added apply_job_task_count(xce_resreq_t*, uint32_t mintasks, uint32_t maxtasks) (src/lib/resreq.c/resreq.h) -- overwrites a standalone top-level primary's minTasks/maxTasks with the job's real requested count (no-op for COMPOUND/ALTER nested primaries, deliberately left out of scope per bug-1768; defensive min>max clamp). Wired at both live ingest call sites: core_build_job() (core_events.c, fresh submission) using job->submit->mintasks/maxtasks, and the snapshot-restore path (snapshot_load.c, --sim/replay driver) using the job's already-restored mintasks/maxtasks -- both called strictly before their respective schedobj/resreq_fp builders read the task count. make_sched_job() (schedobj.c) intentionally NOT wired -- confirmed dead code (unreachable via sched_update(), which has zero live callers). Verified end-to-end via a new xce-test-multihost skill: a real multi-task job (and a multi-task array-job element) submitted via plain jobsub is genuinely split across two real xcecd_virtual hosts by the scheduler's own decision and reaches DONE on both. Full regression clean (scheduler/controller/worker suites, only pre-existing unrelated bug-1758/bug-1723 remain) and HA soak clean (sequential + chaotic, zero job loss/divergence). test_parjob_e2e_multislot.c's PATCH_MAXTASKS_FOR_BUG_1770 workaround is removed; that test now drives its scenario entirely through unmodified real ingest. Sibling gaps deliberately NOT fixed by this feature, tracked separately: bug-1772 (snapshot-restore resreq_fp gap), bug-1773 (resource-request update never taking effect for scheduling), bug-1774 (make_sched_job()'s dead-code resreq_fp gap).",
"tags": [
"scheduler",
"resreq-parsing",