diff --git a/src/scheduler/src/core_events.c b/src/scheduler/src/core_events.c index dc37ce68..2f1d1f37 100644 --- a/src/scheduler/src/core_events.c +++ b/src/scheduler/src/core_events.c @@ -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; } diff --git a/src/scheduler/src/schedobj.c b/src/scheduler/src/schedobj.c index 3a7fe5d8..b8cc143b 100644 --- a/src/scheduler/src/schedobj.c +++ b/src/scheduler/src/schedobj.c @@ -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; } diff --git a/src/scheduler/testsuite/test_maxtasks_wiring_ingest.c b/src/scheduler/testsuite/test_maxtasks_wiring_ingest.c index 569fa366..346adcb8 100644 --- a/src/scheduler/testsuite/test_maxtasks_wiring_ingest.c +++ b/src/scheduler/testsuite/test_maxtasks_wiring_ingest.c @@ -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); } /* ---------------------------------------------------------------------- */