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

fix(scheduler): a job's resource-request update takes effect for scheduling (bug-1773)

Browse source 
JOB_UPDATE (core_events.c) and the legacy sched_update_existing_job()
(schedobj.c) replaced job->resreq, but the allocator reads job->schedobj,
whose primary->resreq still pointed into the old parsed request -- so an
update never changed scheduling, for any field. The old primary was also
leaked (FREEUP only freed the container).

- core_bind_primary_resreq(): the resreq pointer + fingerprint computation
  factored out of core_build_primary_schedobj(), reused on update.
- core_rebind_schedobj_resreq(): primary->primary re-points in place and
  recomputes the fingerprint, keeping the schedobj's runtime state; a kind
  change rebuilds the kind-specific part (the old part is left alone, as
  nothing frees these today -- never a dangling pointer).
- JOB_UPDATE now applies the job's ingest-time task count to the new
  resreq, as core_build_job() does, so resreq-maxtasks-wiring Req 1.4 (an
  update never changes the dispatched task count) still holds -- it used to
  hold only because the update never reached the schedobj at all.
- schedobj.c's legacy path gets the same re-binding.

test_resource_request_update_never_changes_dispatched_task_count asserted
the old behaviour (schedobj keeps the ingest-time object); it now asserts
the schedobj uses the updated resreq and fingerprint while the task count
stays 4/7 (RED before). Scheduler suite 87/87.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-09-27 03:10:09 +00:00
parent 08e6c5e817
commit 41c3694d06
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
3 changed files with 97 additions and 32 deletions
Download patch file Download diff file Expand all files Collapse all files

70
src/scheduler/src/core_events.c
View file

@ -108,6 +108,23 @@ extern xce_val_t* dup_val(const xce_val_t*);
* (make_sched_obj()'s `xce_scheduler_t* sched` parameter is dead in its own
* body -- verified, never dereferenced -- so this port drops it rather than
* threading an unused core-boundary violation through). */
/* Point a primary schedobj at `resreq` and compute its resreq fingerprint
* (Req 7.1, scheduler-allocation spec): every later unsatisfiable-resreq
* cache lookup for this job reuses this value instead of recomputing it.
* Called at ingest and again when JOB_UPDATE replaces the resreq
* (bug-1773). */
static void core_bind_primary_resreq(xce_primary_schedobj_t* primary,
xce_primary_resreq_t* resreq)
{
primary->resreq = resreq;
primary->resreq_fp.min_tasks = resreq->minTasks;
primary->resreq_fp.max_tasks = resreq->maxTasks;
primary->resreq_fp.select_node_id =
(resreq->select != NULL && resreq->select->root != NULL)
? resreq->select->root->node_id : 0;
primary->resreq_fp.rusage_hash = resreq->rusage_hash;
}
static void core_build_primary_schedobj(xce_sched_obj_t* obj,
xce_primary_resreq_t* resreq)
{
@ -119,20 +136,9 @@ static void core_build_primary_schedobj(xce_sched_obj_t* obj,
return;
}
primary->resreq = resreq;
primary->reason = NULL;
primary->owner = obj;
/* Req 7.1 (scheduler-allocation spec): compute the resreq fingerprint
* once, here, at real job-ingest time -- the only construction site
* for xce_primary_schedobj_t. Every later unsatisfiable-resreq cache
* lookup for this job reuses this value instead of recomputing it. */
primary->resreq_fp.min_tasks = resreq->minTasks;
primary->resreq_fp.max_tasks = resreq->maxTasks;
primary->resreq_fp.select_node_id =
(resreq->select != NULL && resreq->select->root != NULL)
? resreq->select->root->node_id : 0;
primary->resreq_fp.rusage_hash = resreq->rusage_hash;
core_bind_primary_resreq(primary, resreq);
obj->u.primary = primary;
}
@ -226,6 +232,36 @@ static void core_build_schedobj(xce_sched_obj_t* obj, xce_job_t* j,
obj->configver = 0;
}
/* bug-1773: JOB_UPDATE replaces job->resreq, but the allocator reads
* job->schedobj, which still pointed into the old resreq -- the update never
* took effect for scheduling. Re-bind the schedobj's resreq-specific part,
* keeping its runtime state (user/queue/project, limits, configver). A
* primary->primary update (the usual case) is re-pointed in place. When the
* resreq kind changes, the kind-specific part is rebuilt; the old part is
* left alone, as nothing in the scheduler ever frees these (they may still
* be referenced) -- a small leak on a rare path, never a dangling pointer. */
static void core_rebind_schedobj_resreq(xce_sched_obj_t* obj,
xce_resreq_t* resreq)
{
if ((obj->tag == XCE_RESREQ_PRIMARY) && (resreq->tag == XCE_RESREQ_PRIMARY)
&& (obj->u.primary != NULL)) {
core_bind_primary_resreq(obj->u.primary, resreq->u.primary);
return;
}
switch (resreq->tag) {
case XCE_RESREQ_PRIMARY:
core_build_primary_schedobj(obj, resreq->u.primary);
break;
case XCE_RESREQ_COMPOUND:
core_build_compound_schedobj(obj, resreq->u.compound);
break;
case XCE_RESREQ_ALTER:
core_build_alter_schedobj(obj, resreq->u.alter);
break;
}
}
static xce_sched_job_t* core_build_job(xce_sched_core_t* core, xce_job_t* j)
{
xce_sched_job_t* job;
@ -1332,6 +1368,16 @@ int xce_core_push_events(xce_sched_core_t* core, xce_core_event_t* events,
break;
}
/* bug-1773: the dispatched task count stays the
* ingest-time one (resreq-maxtasks-wiring Req 1.4),
* applied as core_build_job() does, and the schedobj
* the allocator reads is re-bound to the new resreq. */
apply_job_task_count(newreq, job->mintasks,
job->maxtasks);
if (job->schedobj != NULL) {
core_rebind_schedobj_resreq(job->schedobj, newreq);
}
FREEUP(job->resreq);
job->resreq = newreq;
}

18
src/scheduler/src/schedobj.c
View file

@ -687,6 +687,24 @@ int sched_update_existing_job(xce_scheduler_t* sched, xce_job_t* updated)
return -EINVAL;
}
/* bug-1773: re-bind the schedobj the allocator reads to the new
* resreq (mirrors core_events.c's JOB_UPDATE fix). */
if (job->schedobj != NULL) {
xce_sched_obj_t* obj = job->schedobj;
if ((obj->tag == XCE_RESREQ_PRIMARY)
&& (newreq->tag == XCE_RESREQ_PRIMARY)
&& (obj->u.primary != NULL)) {
obj->u.primary->resreq = newreq->u.primary;
} else if (newreq->tag == XCE_RESREQ_PRIMARY) {
make_primary_schedobj(obj, newreq->u.primary);
} else if (newreq->tag == XCE_RESREQ_COMPOUND) {
make_compound_schedobj(obj, newreq->u.compound);
} else if (newreq->tag == XCE_RESREQ_ALTER) {
make_alter_schedobj(obj, newreq->u.alter);
}
}
free(job->resreq);
job->resreq = newreq;
}

41
src/scheduler/testsuite/test_maxtasks_wiring_ingest.c
View file

@ -954,34 +954,33 @@ test_resource_request_update_never_changes_dispatched_task_count(void)
TEST_ASSERT_NOT_NULL(sjob);
/* Sanity: the update DID genuinely replace job->resreq (not a silent
* no-op) -- job->resreq is now a fresh object showing parse_resreq_
* primary()'s own bare 1/1 default (JOB_UPDATE never calls apply_job_
* task_count(), design.md), confirming the assertions below are a
* meaningful immunity proof, not a vacuous one from an update that
* never took effect. */
* no-op). */
TEST_ASSERT_NOT_NULL(sjob->resreq);
TEST_ASSERT_EQUAL_INT(XCE_RESREQ_PRIMARY, sjob->resreq->tag);
TEST_ASSERT_FALSE_MESSAGE(
sjob->resreq->u.primary == ingest_time_primary,
"fixture: JOB_UPDATE must have replaced job->resreq with a "
"genuinely NEW object, not left the old one in place, for this "
"test's immunity assertion to be meaningful");
TEST_ASSERT_EQUAL_INT_MESSAGE(
1, sjob->resreq->u.primary->minTasks,
"fixture: job->resreq (freshly reparsed by JOB_UPDATE) must show "
"parse_resreq_primary()'s own bare default, confirming JOB_UPDATE "
"never calls apply_job_task_count()");
"genuinely NEW object");
/* ── The Req 1.4 proof: job->schedobj->u.primary->resreq (what the
* real allocator dispatch reads, scheduler.c:315) is the EXACT SAME
* object, with the EXACT SAME ingest-time task count, unaffected by
* the update -- because JOB_UPDATE never reassigns job->schedobj. ──── */
/* ── bug-1773: the allocator reads job->schedobj->u.primary->resreq
* (scheduler.c), so the update must re-point it at the NEW resreq --
* previously it kept the ingest-time object and the update never took
* effect for scheduling, for any field. ─────────────────────────────── */
TEST_ASSERT_NOT_NULL(sjob->schedobj);
TEST_ASSERT_EQUAL_INT(XCE_RESREQ_PRIMARY, sjob->schedobj->tag);
TEST_ASSERT_TRUE_MESSAGE(
sjob->schedobj->u.primary->resreq == ingest_time_primary,
"job->schedobj->u.primary->resreq must remain the EXACT SAME "
"object ingest originally built -- JOB_UPDATE must never rebuild "
"job->schedobj");
sjob->schedobj->u.primary->resreq == sjob->resreq->u.primary,
"bug-1773: job->schedobj must use the updated resreq, not the "
"ingest-time one");
TEST_ASSERT_EQUAL_UINT64_MESSAGE(
sjob->resreq->u.primary->rusage_hash,
sjob->schedobj->u.primary->resreq_fp.rusage_hash,
"bug-1773: the schedobj's resreq fingerprint must be recomputed "
"from the updated resreq");
/* ── Req 1.4 still holds: the dispatched task count stays the
* ingest-time value, not the update's own different mintasks/maxtasks
* nor parse_resreq_primary()'s bare 1/1 default. ─────────────────── */
TEST_ASSERT_EQUAL_INT_MESSAGE(
4, sjob->schedobj->u.primary->resreq->minTasks,
"Req 1.4: the job's dispatched minTasks must remain the "
@ -992,6 +991,8 @@ test_resource_request_update_never_changes_dispatched_task_count(void)
"Req 1.4: the job's dispatched maxTasks must remain the "
"ingest-time value (7), unaffected by the update's own different "
"maxtasks (1)");
TEST_ASSERT_EQUAL_INT(4, sjob->schedobj->u.primary->resreq_fp.min_tasks);
TEST_ASSERT_EQUAL_INT(7, sjob->schedobj->u.primary->resreq_fp.max_tasks);
}
/* ---------------------------------------------------------------------- */