lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

test(resreq-maxtasks-wiring): retire multislot test's bug-1770 workaround

Browse source 
test_parjob_e2e_multislot.c's standing "bug-1770" assertion now
expects the correct, real primary->minTasks/maxTasks (1/JOB_MAXTASKS)
instead of the buggy always-1 value it used to document. The
PATCH_MAXTASKS_FOR_BUG_1770 block that manually overwrote the
already-ingested resreq's task-count fields is removed entirely --
the full multi-host allocation scenario (2-host commit, per-host
slots) now runs end to end through unmodified real ingest. Header
comment updated to record that bug-1770 is fixed by this feature
rather than describing a live gap.

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 07:49:38 +00:00
parent c36014c072
commit d1849c94d8
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
2 changed files with 44 additions and 65 deletions
Download patch file Download diff file Expand all files Collapse all files

2
.kiro/specs/resreq-maxtasks-wiring/tasks.md
View file

@ -49,7 +49,7 @@
- _Boundary: test_maxtasks_wiring_ingest.c, test_maxtasks_wiring_sim_driver_parity.c (see Implementation Notes)_
- _Depends: 2.1, 2.2_
- [ ] 3.3 (P) Retire the multi-host allocator characterization test's manual workaround
- [x] 3.3 (P) Retire the multi-host allocator characterization test's manual workaround
- Replace the existing multi-host allocator test's manual task-count patch with real ingest of an equivalent multi-task job, exercising the same scenario end to end through the now-fixed wiring
- Observable: the updated test passes while driving its scenario entirely through unmodified real ingest, with the workaround removed
- _Requirements: 2.1, 2.2_

107
src/scheduler/testsuite/test_parjob_e2e_multislot.c
View file

@ -74,54 +74,41 @@
* cache-clear wiring (`_Depends: 5.1, 5.2, 4.3` on this task) as part of the
* same real call sequence production makes every round.
*
* ── bug-1770 (discovered by this file, NOT fixed here) ──────────────────
* ── bug-1770 (discovered by this file; FIXED by resreq-maxtasks-wiring) ──
* Confirmed by direct code read (resreq.c's parse_resreq_primary(),
* core_events.c's core_build_job()/core_build_primary_schedobj(),
* schedobj.c's make_sched_job()) and by a genuine RED-phase run of this
* very file: the REAL job-ingest path can NEVER produce a primary resreq
* with maxTasks > 1. parse_resreq_primary() unconditionally hardcodes
* very file: the REAL job-ingest path could NEVER produce a primary resreq
* with maxTasks > 1. parse_resreq_primary() unconditionally hardcoded
* `primary->minTasks = 1; primary->maxTasks = 1;` regardless of the
* resreq string's own content, and neither core_build_job() nor
* make_sched_job() ever copies the job's own submitted task count
* make_sched_job() ever copied the job's own submitted task count
* (job_submit_data_t.mintasks/maxtasks, correctly propagated onto the
* TOP-LEVEL xce_sched_job_t.mintasks/maxtasks by both functions) 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: parjob_allocator is
* UNREACHABLE from any real production job submission today, regardless of
* what task count a real job requests -- every job's primary resreq that
* parses successfully has maxTasks pinned to 1 by parse_resreq_primary(),
* so sched_make_primary_alloc_() always routes it to seqjob_allocator
* instead (scheduler.c:290-294). This is a genuine, real, pre-existing gap
* this end-to-end test is what surfaced it -- every prior unit test in this
* feature (test_parjob_combinatorial.c etc.) hand-constructs its
* xce_primary_resreq_t fixture directly, setting maxTasks by plain struct
* assignment, so none of them exercise the real ingest path closely enough
* to notice maxTasks never actually arrives that way.
* allocator.c's my_alloc() actually read. Net effect: parjob_allocator was
* UNREACHABLE from any real production job submission, regardless of what
* task count a real job requested -- every job's primary resreq that
* parsed successfully had maxTasks pinned to 1 by parse_resreq_primary(),
* so sched_make_primary_alloc_() always routed it to seqjob_allocator
* instead (scheduler.c:290-294). This end-to-end test is what surfaced the
* gap -- every prior unit test in this feature (test_parjob_combinatorial.c
* etc.) hand-constructs its xce_primary_resreq_t fixture directly, setting
* maxTasks by plain struct assignment, so none of them exercised the real
* ingest path closely enough to notice maxTasks never actually arrived
* that way. Logged as bug-1770 (.wolf/buglog.json).
*
* Deliberately NOT fixed as part of this task: wiring job_submit_data_t's
* task count into xce_primary_resreq_t requires a real design decision
* (parse_resreq_primary()'s signature would need the job's task count
* threaded in from BOTH of its two call sites, plus a decision about how a
* COMPOUND resreq's several nested primaries should each receive a single
* job-level task count) well outside this spec's Boundary Context (the
* iterator/allocator rework, not resreq-parsing/job-submission plumbing) --
* 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). Logged as bug-1770 (.wolf/buglog.json) for dedicated
* follow-up.
*
* This file's fixture compensates with ONE minimal, explicitly-labeled
* patch of the real, already-ingested xce_primary_resreq_t's task-count
* fields (see PATCH_MAXTASKS_FOR_BUG_1770 below) immediately after the real
* JOBS_PUSH ingest completes and before the round is driven -- everything
* else (cache lookup, parjob dispatch, plan iteration, host claiming,
* multi-host commit, decision production) runs through the unmodified real
* dispatch path. Removing that patch reproduces the RED-phase failure this
* file's own task-5.3 implementation captured (see the implementer's status
* report for the exact command/output): the job would route to seqjob_
* allocator instead of parjob_allocator, and decision->u.alloc->nhosts
* would be 1, not 2.
* Update: fixed for real by the resreq-maxtasks-wiring spec (see
* .kiro/specs/resreq-maxtasks-wiring/), which threads
* job_submit_data_t.mintasks/maxtasks through apply_job_task_count()
* (resreq.c) into the standalone primary's minTasks/maxTasks at both real
* ingest call sites (core_events.c's core_build_job() and
* snapshot_load.c's restore path). This file's fixture no longer needs any
* compensating patch: the real, already-ingested xce_primary_resreq_t's
* task-count fields now arrive correctly from unmodified real JOBS_PUSH
* ingest alone, and the standing assertion below has been flipped to
* expect that correct value instead of documenting the old gap.
*
* ── Fixture: forcing a genuine multi-host split ─────────────────────────
* Two real workers, each with exactly 2.0 free "slots" (WORKER_SLOTS),
@ -487,35 +474,27 @@ test_multihost_parallel_job_allocation_spans_two_hosts_via_real_dispatch(void)
* carries the real submitted task count through ingest... */
TEST_ASSERT_EQUAL_UINT32(JOB_MAXTASKS, sjob->maxtasks);
/* ...but the NESTED xce_primary_resreq_t.maxTasks the allocator
* dispatch actually reads does not -- bug-1770 (see file header). This
* assertion is the direct, standing proof of that gap: it will keep
* passing (documenting the bug) until bug-1770 is fixed for real. */
/* ...and the NESTED xce_primary_resreq_t.minTasks/maxTasks the
* allocator dispatch actually reads now correctly carries the job's
* real submitted task count too -- resreq-maxtasks-wiring's fix
* (apply_job_task_count(), called from core_build_job() before
* core_build_schedobj()/core_build_primary_schedobj()). This is the
* direct, standing proof that bug-1770 is fixed for real: no patch of
* any kind is applied to `primary` below -- these are the exact,
* unmodified values real JOBS_PUSH ingest produced. The job's fixture
* (make_push_job() above) submits mintasks=1, maxtasks=JOB_MAXTASKS. */
primary = sjob->schedobj->u.primary->resreq;
TEST_ASSERT_NOT_NULL(primary);
TEST_ASSERT_EQUAL_INT_MESSAGE(
1, primary->maxTasks,
"bug-1770: the real ingest path currently hardcodes primary->"
"maxTasks=1 regardless of the job's own submitted maxtasks -- if "
"this assertion ever fails because maxTasks now correctly arrives "
"as JOB_MAXTASKS, bug-1770 has been fixed for real and the "
"PATCH_MAXTASKS_FOR_BUG_1770 workaround below should be deleted");
/* PATCH_MAXTASKS_FOR_BUG_1770: compensate for the confirmed,
* out-of-boundary gap above by patching the REAL, already-ingested
* primary resreq's task-count fields directly (see file header for
* the full rationale for not fixing this in production code as part
* of this task). Everything downstream of this point -- cache lookup,
* parjob dispatch, plan iteration, host claiming, multi-host commit,
* decision production -- runs through the unmodified real dispatch
* path. resreq_fp is kept consistent too (same fields
* core_build_primary_schedobj() itself derives from primary->min/
* maxTasks at ingest time), though this single-dispatch test never
* exercises a second cache lookup that would depend on it. */
primary->minTasks = 1;
primary->maxTasks = JOB_MAXTASKS;
sjob->schedobj->u.primary->resreq_fp.min_tasks = 1;
sjob->schedobj->u.primary->resreq_fp.max_tasks = JOB_MAXTASKS;
1, primary->minTasks,
"resreq-maxtasks-wiring: primary->minTasks must arrive as the "
"job's real submitted mintasks (1) straight from real ingest, "
"with no test-side patch");
TEST_ASSERT_EQUAL_INT_MESSAGE(
JOB_MAXTASKS, primary->maxTasks,
"resreq-maxtasks-wiring: primary->maxTasks must arrive as the "
"job's real submitted maxtasks (JOB_MAXTASKS) straight from real "
"ingest, with no test-side patch -- this is the fix for bug-1770");
/* ── Real per-session begin (task 5.2's cache-clear wiring) ──────── */
rc = xce_core_begin_session(&(sched->core));