refactor(scheduler): move job-total limit computation to the limit plugin (bug-1790)
bug-1790's fix worked but put limit-plugin-specific logic in the wrong place: seqjob_prepare_allocation()/parjob_prepare_allocation() (the allocator plugins) unconditionally pre-computed a job-wide total for every per-host resource in the job's rusages, "just in case" some global limit needed one. That's the allocator plugins' concern leaking into limit evaluation -- duplicated across both allocators (a third would need it too), and wasteful (computed for resources no limit actually references). Moved ownership to sched_fill_limit_requests() (resource.c), called once centrally from plugin_prepare_allocation() regardless of which allocator ran: - Removed calc_wanted_host_resval_job_totals() and its calls from both allocator plugins entirely. - Added calc_wanted_job_total_resval() (resreq.c): an on-demand, single-resource lookup by name (no resmgr/key_find needed -- the caller already has the resolved xce_resource_t), computing the same job-wide-total formula only for the one (scope, underlying-resource) combination that needs it -- a GLOBAL limit on a per-host resource. The other three combinations are unchanged, still reading straight out of alloc_req. - sched_fill_limit_requests() gains rusages/nhosts/nslots parameters back (a genuine need this time, not a re-added unused one). plugin.c's two call sites pass obj->u.primary->resreq->rusages (NULL for COMPOUND/ALTER, matching this codebase's existing scope convention for those tags) and ctx->nhosts/ctx->nslots. All 23 testsuite link-satisfying stubs mechanically updated. Removed the two allocator-level regression tests from the first pass (the behavior they proved no longer exists by design); rewrote test_fill_limit_requests_global_limit_on_host_resource_reads_host to exercise the new on-demand path directly, nslots=3 to prove the job's WHOLE total is used. Full xce-build.sh all asan --incremental: 94/94 Ok, zero regressions. Live re-verified against the same ug1_global_mem deploy/scenario: byte-identical scheduler trace to the pre-refactor result, confirming behavior-preserving. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GLfB5uWRdmx2WiZSFC7Fm7
This commit is contained in:
parent
b612f80bd6
commit
fa34b038f7
33 changed files with 308 additions and 350 deletions
|
|
@ -1134,93 +1134,6 @@ static void test_shared_resource_debited_once_regardless_of_host_count(void)
|
|||
sched_ctx_fini(&ctx);
|
||||
}
|
||||
|
||||
/*
|
||||
* bug-1790: same regression as test_seqjob_allocator.c's own sibling test,
|
||||
* for parjob's prepare_allocation hook -- a GLOBAL limit built on a
|
||||
* per-host resource OTHER than "slots" (e.g. a future global mem cap)
|
||||
* needs ctx->alloc_req.host to carry that resource's JOB-WIDE TOTAL want
|
||||
* during the global phase (ctx->worker == NULL) so sched_fill_limit_
|
||||
* requests() (resource.c) can read it. Uses maxTasks=3 (nslots=3 during
|
||||
* the global phase) specifically to prove the PER_TASK formula uses the
|
||||
* job's WHOLE nslots, not any one bucket's share (parjob_try_plan_() only
|
||||
* assigns a per-bucket nslots later, in the per-host phase -- unaffected
|
||||
* by this test, called directly with ctx.worker == NULL).
|
||||
*/
|
||||
static void
|
||||
test_parjob_prepare_allocation_global_phase_stores_host_resource_job_total(
|
||||
void)
|
||||
{
|
||||
xce_scheduler_t sched;
|
||||
xce_resource_t* slots_res;
|
||||
xce_resource_t* mem_res;
|
||||
xce_resource_t mem_res_in = { 0 };
|
||||
xce_sched_obj_t obj;
|
||||
xce_primary_schedobj_t primary_obj;
|
||||
xce_primary_resreq_t primary;
|
||||
xce_sched_ctx_t ctx;
|
||||
xce_sched_plugin_t plugin;
|
||||
rusage_value_t rv;
|
||||
int rc;
|
||||
|
||||
memset(&sched, 0, sizeof(sched));
|
||||
TEST_ASSERT_EQUAL_INT(0, xce_sched_core_init(&(sched.core)));
|
||||
|
||||
slots_res = register_slots_resource(sched.core.resmgr);
|
||||
TEST_ASSERT_NOT_NULL(slots_res);
|
||||
|
||||
/* "mem" -- per-host resource (NOT XCE_RESF_GLOBAL), the shape a future
|
||||
* GLOBAL limit on top of it (bug-1790) would need populated. */
|
||||
mem_res_in.name = strdup(SCHED_MEM_RESOURCE_NAME);
|
||||
mem_res = makeResource(sched.core.resmgr, &mem_res_in);
|
||||
TEST_ASSERT_NOT_NULL(mem_res);
|
||||
|
||||
memset(&primary, 0, sizeof(primary));
|
||||
primary.minTasks = 1;
|
||||
primary.maxTasks = 3; /* parjob's job, not seqjob's -- see nslots note above */
|
||||
primary.select = NULL;
|
||||
|
||||
rc = xlist_init(&(primary.rusages), 4);
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
|
||||
memset(&rv, 0, sizeof(rv));
|
||||
rv.res = strdup(SCHED_MEM_RESOURCE_NAME);
|
||||
rv.value = 2.0f;
|
||||
rv.per_flag = PER_TASK;
|
||||
rc = xlist_store(&(primary.rusages), &rv);
|
||||
TEST_ASSERT_TRUE(rc >= 0);
|
||||
|
||||
init_primary_obj(&obj, &primary_obj, &primary);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(0, sched_ctx_init(&ctx, &sched, &obj));
|
||||
|
||||
memset(&plugin, 0, sizeof(plugin));
|
||||
plugin.sched = &sched;
|
||||
|
||||
ctx.worker = NULL;
|
||||
rc = parjob_prepare_allocation(&plugin, &ctx);
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
|
||||
/* "mem" is not XCE_RESF_GLOBAL, so it's correctly excluded from
|
||||
* .shared -- only a genuinely global resource would land there. */
|
||||
TEST_ASSERT_EQUAL_INT(0, ctx.alloc_req.shared.count);
|
||||
|
||||
/* THE regression assertion: .host now carries "mem"'s job-wide total
|
||||
* (PER_TASK: value * nslots == 2.0 * 3, the job's WHOLE maxTasks, not
|
||||
* any one bucket's share) alongside the pre-existing unconditional
|
||||
* "slots" entry (also job-wide here: ctx->nslots itself, per the
|
||||
* existing xvec_idval_store() call below in this same branch). */
|
||||
TEST_ASSERT_EQUAL_INT(2, ctx.alloc_req.host.count);
|
||||
TEST_ASSERT_EQUAL_INT(mem_res->idx, ((int*)ctx.alloc_req.host.idxs.data)[0]);
|
||||
TEST_ASSERT_EQUAL_FLOAT(6.0f, ((float*)ctx.alloc_req.host.vals.data)[0]);
|
||||
TEST_ASSERT_EQUAL_INT(slots_res->idx,
|
||||
((int*)ctx.alloc_req.host.idxs.data)[1]);
|
||||
TEST_ASSERT_EQUAL_FLOAT(3.0f, ((float*)ctx.alloc_req.host.vals.data)[1]);
|
||||
|
||||
xlist_fini(&(primary.rusages));
|
||||
FREEUP(rv.res);
|
||||
sched_ctx_fini(&ctx);
|
||||
}
|
||||
|
||||
void setUp(void) { return; }
|
||||
|
||||
void tearDown(void) { return; }
|
||||
|
|
@ -1240,7 +1153,5 @@ int main(void)
|
|||
test_mid_plan_bucket_failure_rolls_back_before_later_plan_succeeds);
|
||||
RUN_TEST(test_returns_zero_when_every_plan_is_exhausted);
|
||||
RUN_TEST(test_shared_resource_debited_once_regardless_of_host_count);
|
||||
RUN_TEST(
|
||||
test_parjob_prepare_allocation_global_phase_stores_host_resource_job_total);
|
||||
return UNITY_END();
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue