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

fix(scheduler): populate job-wide total want for per-host resources during global phase (bug-1790)

Browse source 
Closes the gap bug-1789 explicitly carved out: a GLOBAL limit built on a
per-host resource other than "slots" (e.g. a future absolute global mem
cap) needs ctx->alloc_req.host to carry that resource's job-wide total
want during the global phase, but only "slots" ever got that treatment
(via seqjob/parjob_prepare_allocation()'s own unconditional store).

Adds calc_wanted_host_resval_job_totals() (resreq.c), sharing its loop
body with calc_wanted_global_resvals() via a new static helper
parameterized only by which XCE_RESF_GLOBAL direction to match -- it
computes the SAME job-wide-total formulas (PER_TASK: value*nslots, PER_JOB:
value, PER_HOST: value*nhosts) for non-global resources instead of global
ones. seqjob/parjob_prepare_allocation()'s global-phase branch now calls
it into ctx->alloc_req.host alongside the existing calc_wanted_global_
resvals() call into .shared.

Safety was traced, not assumed: plugin_prepare_allocation() only resets
.host at the top of a per-host call, never during the global phase, and
sched_fill_limit_requests() for glimits reads .host later in the SAME
global-phase invocation -- transferring the value into the limit's
permanent .shared record before any later per-host call ever resets
.host. An earlier candidate fix (reusing calc_wanted_host_resvals()
directly) was rejected during design: its PER_HOST formula and, for
parjob, its per-bucket nslots don't give a job-wide total.

New regression tests in test_seqjob_allocator.c/test_parjob_
combinatorial.c prove a non-"slots" per-host resource ("mem") now gets
correctly populated; parjob's test uses maxTasks=3 specifically to prove
the formula uses the job's whole nslots, not a bucket's share.

Full suite (xce-build.sh all asan --incremental): 92/94, both failures
pre-existing and unrelated (bug-1762, bug-1733's already-documented
xcealloc.c leak). Not live-verified -- no limits.yaml entry exercises
this path yet.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GLfB5uWRdmx2WiZSFC7Fm7
This commit is contained in:
Rongsong Shen 2026-09-08 02:57:51 +00:00
parent 028d8f6ac7
commit 9caf1424b9
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
7 changed files with 265 additions and 8 deletions
Download patch file Download diff file Expand all files Collapse all files

2
src/include/resreq.h
View file

@ -315,6 +315,8 @@ extern int calc_wanted_global_resvals(xvec_idval_t*, var, xlist_t*,
rusage_per_type_t, int, int);
extern int calc_wanted_host_resvals(xvec_idval_t*, var, xlist_t*,
rusage_per_type_t, int, int);
extern int calc_wanted_host_resval_job_totals(xvec_idval_t*, var, xlist_t*,
rusage_per_type_t, int, int);
/*
* Overwrite minTasks/maxTasks on resreq's own top-level primary with the

69
src/lib/resreq.c
View file

@ -731,9 +731,21 @@ int format_resreq(char** buf, xce_resreq_t* resreq)
return rc;
}
int calc_wanted_global_resvals(xvec_idval_t* resvals, var resmgr,
xlist_t* rusages, rusage_per_type_t perflag,
int nhosts, int nslots)
/*
* calc_wanted_job_total_resvals_ - shared body for calc_wanted_global_resvals()
* and calc_wanted_host_resval_job_totals(): computes each matching rusage
* entry's JOB-WIDE TOTAL want (PER_TASK: value*nslots, PER_JOB: value,
* PER_HOST: value*nhosts -- nslots/nhosts are the whole job's totals, not
* any one host's share). `match_global` selects which resources are
* "matching": nonzero processes only XCE_RESF_GLOBAL resources (the
* original calc_wanted_global_resvals() behavior), zero processes only
* non-GLOBAL (per-host) ones instead.
*/
static int calc_wanted_job_total_resvals_(xvec_idval_t* resvals, var resmgr,
xlist_t* rusages,
rusage_per_type_t perflag,
int nhosts, int nslots,
int match_global)
{
int i;
@ -760,8 +772,14 @@ int calc_wanted_global_resvals(xvec_idval_t* resvals, var resmgr,
continue;
}
if ((r->flags & XCE_RESF_GLOBAL) == 0) {
continue;
if (match_global) {
if ((r->flags & XCE_RESF_GLOBAL) == 0) {
continue;
}
} else {
if ((r->flags & XCE_RESF_GLOBAL) != 0) {
continue;
}
}
perflag1 = v->per_flag;
@ -790,6 +808,47 @@ int calc_wanted_global_resvals(xvec_idval_t* resvals, var resmgr,
return 0;
}
int calc_wanted_global_resvals(xvec_idval_t* resvals, var resmgr,
xlist_t* rusages, rusage_per_type_t perflag,
int nhosts, int nslots)
{
return calc_wanted_job_total_resvals_(resvals, resmgr, rusages, perflag,
nhosts, nslots, 1);
}
/*
* calc_wanted_host_resval_job_totals - bug-1790: like
* calc_wanted_global_resvals(), but for resources NOT flagged
* XCE_RESF_GLOBAL (per-host resources) -- computes each such resource's
* JOB-WIDE TOTAL want, not any one host's own share (calc_wanted_host_
* resvals(), below, computes that instead, for the real per-host capacity
* check). Exists so a GLOBAL limit built on top of a per-host resource
* (e.g. a future absolute global mem cap, "slots" already being the one
* bug-1789 fixed) can read a real aggregate instead of an always-missing
* value.
*
* seqjob_prepare_allocation()/parjob_prepare_allocation()'s global-phase
* branch (ctx->worker == NULL) calls this into ctx->alloc_req.host
* immediately alongside its existing calc_wanted_global_resvals() call.
* This is safe despite .host normally being per-host/per-bucket-varying
* data (see parjob_allocator.c's own per-bucket reset): plugin_prepare_
* allocation() (plugin.c) only resets .host at the START of a PER-HOST
* call (ctx->worker != NULL), never during the global phase, so the value
* written here survives long enough for sched_fill_limit_requests()
* (resource.c) to read it -- called later in the SAME global-phase
* invocation, for obj->glimits -- before any later per-host call wipes
* and correctly repopulates .host with that specific host's own share via
* calc_wanted_host_resvals().
*/
int calc_wanted_host_resval_job_totals(xvec_idval_t* resvals, var resmgr,
xlist_t* rusages,
rusage_per_type_t perflag, int nhosts,
int nslots)
{
return calc_wanted_job_total_resvals_(resvals, resmgr, rusages, perflag,
nhosts, nslots, 0);
}
int calc_wanted_host_resvals(xvec_idval_t* resvals, var resmgr,
xlist_t* rusages, rusage_per_type_t perflag,
int nhosts, int nslots)

9
src/scheduler/src/plugins/parjob_allocator.c
View file

@ -116,6 +116,15 @@ static int parjob_prepare_allocation(void* arg, xce_sched_ctx_t* ctx)
calc_wanted_global_resvals(&(ctx->alloc_req.shared), sched->core.resmgr,
&(primary->rusages), PER_TASK, ctx->nhosts,
ctx->nslots);
/* bug-1790: also compute this job's job-wide total want for any
* per-host resource in its own rusages (e.g. a future global limit
* built on top of "mem") -- see calc_wanted_host_resval_job_totals()'s
* own comment (resreq.c) for why writing into .host here, during the
* global phase, is safe. */
calc_wanted_host_resval_job_totals(&(ctx->alloc_req.host),
sched->core.resmgr,
&(primary->rusages), PER_TASK,
ctx->nhosts, ctx->nslots);
} else {
calc_wanted_host_resvals(&(ctx->alloc_req.host), sched->core.resmgr,
&(primary->rusages), PER_TASK, ctx->nhosts,

9
src/scheduler/src/plugins/seqjob_allocator.c
View file

@ -84,6 +84,15 @@ static int seqjob_prepare_allocation(void* arg, xce_sched_ctx_t* ctx)
ctx->nslots = 1;
calc_wanted_global_resvals(&(ctx->alloc_req.shared), sched->core.resmgr,
&(primary->rusages), PER_TASK, 1, 1);
/* bug-1790: also compute this job's job-wide total want for any
* per-host resource in its own rusages (e.g. a future global limit
* built on top of "mem") -- see calc_wanted_host_resval_job_totals()'s
* own comment (resreq.c) for why writing into .host here, during the
* global phase, is safe. */
calc_wanted_host_resval_job_totals(&(ctx->alloc_req.host),
sched->core.resmgr,
&(primary->rusages), PER_TASK, 1,
1);
} else {
calc_wanted_host_resvals(&(ctx->alloc_req.host), sched->core.resmgr,
&(primary->rusages), PER_TASK, 1, 1);

89
src/scheduler/testsuite/test_parjob_combinatorial.c
View file

@ -1134,6 +1134,93 @@ 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; }
@ -1153,5 +1240,7 @@ 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();
}

89
src/scheduler/testsuite/test_seqjob_allocator.c
View file

@ -361,11 +361,100 @@ test_seqjob_prepare_allocation_guards_against_maxtasks_gt_1_fanout(void)
sched_ctx_fini(&ctx);
}
/*
* bug-1790: a GLOBAL limit built on a per-host resource OTHER than "slots"
* (e.g. a hypothetical 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 -- see
* that function's own bug-1789 comment. Before this fix, only "slots" got
* this treatment (via seqjob_prepare_allocation()'s own unconditional
* store below); any OTHER per-host resource declared in the job's own
* rusages (here "mem") was silently never populated into .host during the
* global phase, since calc_wanted_host_resvals() (the thing that DOES
* compute a per-host resource's want) only ever ran in the per-host
* branch.
*/
static void
test_seqjob_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), same convention
* every real host-capacity resource uses -- the exact 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 = 1;
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 = seqjob_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 * 1) alongside the pre-existing
* unconditional "slots" entry -- both present, in staging order. */
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(2.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(1.0f, ((float*)ctx.alloc_req.host.vals.data)[1]);
xlist_fini(&(primary.rusages));
FREEUP(rv.res);
sched_ctx_fini(&ctx);
}
int main(void)
{
UNITY_BEGIN();
RUN_TEST(
test_my_alloc_allocates_single_matching_host_through_shared_iterators);
RUN_TEST(test_seqjob_prepare_allocation_guards_against_maxtasks_gt_1_fanout);
RUN_TEST(
test_seqjob_prepare_allocation_global_phase_stores_host_resource_job_total);
return UNITY_END();
}