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

fix(scheduler): re-derive worker/shared resource used-totals on snapshot load

Browse source 
xce_sched_core_load_snapshot() left every restored worker's resInUse
(and core->sharedInUse) at zero -- zarray_init() in the workers-
restoration block clears them, and nothing re-summed against the
restored job allocations. A caller inspecting worker/resource state
between a snapshot load and the first scheduling session (e.g. the
REPL's `info worker` before the first `step`) saw a misleadingly empty
"used" total for jobs already known to be RUNNING. Added one
sched_account_jobs(core) call after workers/jobs are restored -- a
correct idempotent-from-zero re-derivation using the same
sched_charge_account() chain the next real session would use anyway.
Must not be called again without an intervening reset (accumulates,
does not assign) -- verified no such double-call exists in this change.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019QxaSzHRB7pmYFp4cVnwkA
This commit is contained in:
Rongsong Shen 2026-08-29 11:53:32 +00:00
parent 42fdeb6669
commit 14f95b06fe
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
5 changed files with 309 additions and 48 deletions
Download patch file Download diff file Expand all files Collapse all files

231
src/scheduler/testsuite/test_snapshot_save_load.c
View file

@ -1666,8 +1666,20 @@ test_snapshot_load_leaves_allocs_and_accounts_null_for_job_without_allocation(
* #include scheduler.c itself). Calls the REAL sched_charge_account()
* (resource.c, #include'd above), so this genuinely exercises the same
* accounting chain live dispatch and sim_reconcile_unaccounted_jobs() both
* rely on downstream. */
static int test_sched_account_jobs(xce_sched_core_t* core)
* rely on downstream.
*
* Named (and exported, not `static`) as the REAL symbol -- not
* `test_sched_account_jobs` -- because xce_sched_core_load_snapshot()
* (snapshot_load.c, #include'd above) now calls sched_account_jobs()
* itself (scheduler-snapshot-job-allocation task 4.6, Req 1.2/1.3: worker/
* shared resource "used" totals correct immediately on load, not only
* after a caller manually re-derives them). This TU's own white-box
* #include of snapshot_load.c means that call resolves to THIS
* definition, exactly like every other real/stub function this file
* already provides for a function snapshot_load.c or core.c reference
* (kill_sched_*, sched_resolve_job_picker, etc.) -- same convention, not a
* new one. */
int sched_account_jobs(xce_sched_core_t* core)
{
iterator_t it;
int rc = 0;
@ -1914,12 +1926,16 @@ test_snapshot_round_trip_mixed_jobs_shared_pool_resinuse_and_teardown_independen
/* ===================================================================
* Phase 3: the actual proof point (Req 1.2/1.3) -- call the REAL
* sched_account_jobs() chain (test_sched_account_jobs() above, which
* calls the REAL sched_charge_account(), resource.c) on core_a BEFORE
* saving, capturing what "resInUse/sharedInUse, reported the normal
* way" looks like for the source instance.
* sched_account_jobs() chain (this file's own real reimplementation
* above, which calls the REAL sched_charge_account(), resource.c) on
* core_a BEFORE saving, capturing what "resInUse/sharedInUse, reported
* the normal way" looks like for the source instance. Manual here
* because core_a is the SOURCE core, built via direct field pokes
* (never loaded from a snapshot), so nothing calls this for it
* automatically the way xce_sched_core_load_snapshot() now does for
* core_b below (Phase 5/8).
* =================================================================== */
rc = test_sched_account_jobs(&core_a);
rc = sched_account_jobs(&core_a);
TEST_ASSERT_EQUAL_INT(0, rc);
worker_a_a = sched_get_worker(&core_a, "hostA");
@ -2030,16 +2046,19 @@ test_snapshot_round_trip_mixed_jobs_shared_pool_resinuse_and_teardown_independen
TEST_ASSERT_TRUE(restored_running->allocs != snap.jobs->jobs[0]->allocs);
/* ===================================================================
* Phase 8: the actual proof (Req 1.2/1.3) -- call the SAME real
* accounting chain on core_b (the RESTORED instance) and confirm its
* resInUse/sharedInUse totals EXACTLY match what the identical chain
* produced on core_a (Phase 3) before saving -- proving the restored
* allocation is "included in in-use totals immediately" and "reported
* identically to ... normal dispatch" (Req 1.2, 1.3), not merely that
* job->accounts' own fields happen to look right.
* Phase 8: the actual proof (Req 1.2/1.3) -- confirm core_b's
* resInUse/sharedInUse totals EXACTLY match what the identical
* accounting chain produced on core_a (Phase 3) before saving --
* proving the restored allocation is "included in in-use totals
* immediately" and "reported identically to ... normal dispatch" (Req
* 1.2, 1.3), not merely that job->accounts' own fields happen to look
* right. NO manual sched_account_jobs(&core_b) call here (task 4.6,
* unlike this test's pre-4.6 form): xce_sched_core_load_snapshot()
* itself (Phase 5 above) now calls it internally, exactly once, right
* after restoring job->accounts -- calling it again here would
* double-charge (sched_charge_account() accumulates, it does not
* assign) and silently corrupt every assertion below.
* =================================================================== */
rc = test_sched_account_jobs(&core_b);
TEST_ASSERT_EQUAL_INT(0, rc);
worker_a_b = sched_get_worker(&core_b, "hostA");
worker_b_b = sched_get_worker(&core_b, "hostB");
@ -2124,6 +2143,160 @@ test_snapshot_round_trip_mixed_jobs_shared_pool_resinuse_and_teardown_independen
TEST_ASSERT_EQUAL_FLOAT(1.0f, restored_shared_lic);
}
/* ---------------------------------------------------------------------- */
/* scheduler-snapshot-job-allocation task 4.6 (Req 1.2/1.3, "correct */
/* immediately on load"): the direct, minimal proof that */
/* xce_sched_core_load_snapshot() itself -- with NO further call from the */
/* caller -- leaves a restored RUNNING job's worker resInUse already */
/* correct. Found live (2026-08-28): an operator running */
/* `xce_scheduler -i -s <replay>` saw `info worker all` report `used=0.00` */
/* for a worker with jobs already RUNNING in the loaded snapshot, correct */
/* only after the first `step` -- because, before this task, ONLY the */
/* round-boundary sched_account_jobs() call (scheduler-sim-decision-parity */
/* spec, bug-1613's fix, sim_loop.c) ever re-derived resInUse; nothing ran */
/* at load time itself. The round-trip test above already covers this */
/* incidentally (its Phase 8 no longer needs a manual sched_account_jobs() */
/* call and still passes) -- this test names the specific fix directly, */
/* with the leanest fixture that can prove it (one resource, one worker, */
/* one running job), rather than relying on a much larger test's side */
/* effect. */
/* ---------------------------------------------------------------------- */
static void
test_snapshot_load_populates_worker_resinuse_immediately_without_a_session(
void)
{
xce_sched_core_t core;
sched_snapshot_data_t snap;
xce_core_event_t events[3];
xce_conf4sched_t config;
xce_resource_t res_slots;
xce_resource_t* resources[1];
xce_queue_t wire_queue;
xce_queue_t* queues[1];
xce_worker_t wire_worker;
xce_worker_t* workers[1];
xce_val_t worker_slots_val;
xce_name_val_t worker_slots_nv;
xce_name_val_t* worker_resvals[1];
xce_job_t* wire_job;
xce_job_t* jobs[1];
xce_sched_job_t* job;
xce_alloc_t* a_slots;
xce_host_alloc_t* h0;
xce_host_alloc_t** hallocs;
xce_job_alloc_t* jalloc;
xce_sched_worker_t* worker;
xce_resource_t* slots_res;
float resinuse_slots = -1.0f;
int rc;
memset(&g_stub_state_a, 0, sizeof(g_stub_state_a));
fixture_init(&core, &g_stub_plugin_a);
memset(&res_slots, 0, sizeof(res_slots));
res_slots.name = "slots";
resources[0] = &res_slots;
memset(&wire_queue, 0, sizeof(wire_queue));
wire_queue.queue = "normal";
wire_queue.priority = 1;
wire_queue.status = QUEUE_STAT_OPEN;
queues[0] = &wire_queue;
memset(&config, 0, sizeof(config));
config.n_resources = 1;
config.resources = resources;
config.n_queues = 1;
config.queues = queues;
memset(&worker_slots_val, 0, sizeof(worker_slots_val));
worker_slots_val.which = XCE_VAL_NUMERIC;
worker_slots_val.u.ivalue32 = 16;
memset(&worker_slots_nv, 0, sizeof(worker_slots_nv));
worker_slots_nv.name = "slots";
worker_slots_nv.val = &worker_slots_val;
worker_resvals[0] = &worker_slots_nv;
memset(&wire_worker, 0, sizeof(wire_worker));
wire_worker.name = "hostA";
wire_worker.status = 0; /* available */
wire_worker.nresvals_to_sched = 1;
wire_worker.resvals_to_sched = worker_resvals;
workers[0] = &wire_worker;
wire_job = make_job(901, JOB_STAT_PEND, "normal", "alice", "proj1");
jobs[0] = wire_job;
memset(events, 0, sizeof(events));
events[0].type = XCE_CORE_EVENT_CONFIG_PUSH;
events[0].u.config_push.config = &config;
events[1].type = XCE_CORE_EVENT_WORKERS_PUSH;
events[1].u.workers_push.nworkers = 1;
events[1].u.workers_push.workers = workers;
events[2].type = XCE_CORE_EVENT_JOBS_PUSH;
events[2].u.jobs_push.njobs = 1;
events[2].u.jobs_push.jobs = jobs;
rc = xce_core_push_events(&core, events, 3);
TEST_ASSERT_EQUAL_INT(0, rc);
job = sched_get_job(&core, 901);
TEST_ASSERT_NOT_NULL(job);
a_slots = make_alloc((char*)"slots", NULL, 0, 4.0f, 0);
TEST_ASSERT_NOT_NULL(a_slots);
h0 = make_host_alloc((char*)"hostA", 1, make_allocs(1, a_slots));
TEST_ASSERT_NOT_NULL(h0);
hallocs = make_host_allocs(1, h0);
TEST_ASSERT_NOT_NULL(hallocs);
jalloc = make_job_alloc(901, 1, hallocs);
TEST_ASSERT_NOT_NULL(jalloc);
job->state = JOB_STAT_RUN;
job->allocs = jalloc;
job->accounts = make_job_account(job->allocs);
TEST_ASSERT_NOT_NULL(job->accounts);
memset(&snap, 0, sizeof(snap));
rc = xce_sched_core_save_snapshot(&core, &snap);
TEST_ASSERT_EQUAL_INT(0, rc);
/* Reload into the SAME core -- xce_sched_core_load_snapshot() only
* ADDS state (sched_add_job/sched_add_worker on fresh kvmgr entries),
* it does not clear what is already there first, so a second load onto
* an already-populated core would double the very jobs/workers this
* test needs to stay singular; a fresh core (matching every other test
* in this file's own "load into a separate instance" convention) keeps
* the fixture unambiguous. */
{
xce_sched_core_t core_b;
memset(&g_stub_state_b, 0, sizeof(g_stub_state_b));
fixture_init(&core_b, &g_stub_plugin_b);
rc = xce_sched_core_load_snapshot(&core_b, &snap);
TEST_ASSERT_EQUAL_INT(0, rc);
/* The actual proof: NO sched_account_jobs()/manual accounting call
* anywhere in this test -- xce_sched_core_load_snapshot() alone
* must already have charged this restored RUNNING job's
* allocation onto its worker's resInUse. */
worker = sched_get_worker(&core_b, "hostA");
TEST_ASSERT_NOT_NULL(worker);
rc = key_find(core_b.resmgr, (void*)"slots", (void**)&slots_res);
TEST_ASSERT_GREATER_THAN_INT(0, rc);
TEST_ASSERT_NOT_NULL(slots_res);
get_resvals(&(worker->resInUse), 1, &(slots_res->idx),
&resinuse_slots);
TEST_ASSERT_EQUAL_FLOAT(4.0f, resinuse_slots);
}
}
/* ---------------------------------------------------------------------- */
/* scheduler-snapshot-job-allocation task 3.2 (Req 3.5): the backward- */
/* compatibility case made explicit for the scenario Req 3.5's own text and */
@ -2156,8 +2329,9 @@ test_snapshot_round_trip_mixed_jobs_shared_pool_resinuse_and_teardown_independen
/* (d) job->allocs and job->accounts are both NULL -- clean degrade, not */
/* a fabricated allocation or a crash; */
/* (e) with a REAL worker present, the REAL sched_account_jobs() chain */
/* (test_sched_account_jobs() above, task 3.1's own real */
/* reimplementation) correctly contributes ZERO from this job to the */
/* (this file's own reimplementation above, task 3.1, now called */
/* automatically by xce_sched_core_load_snapshot() itself per task */
/* 4.6) correctly contributes ZERO from this job to the */
/* worker's resInUse -- the concrete "used=0, identical to today's */
/* behavior" claim Req 3.5 and the originally-reported "info worker" */
/* symptom are both actually about, not merely that job->accounts is */
@ -2266,11 +2440,17 @@ test_snapshot_load_leaves_running_job_without_allocation_at_zero_resinuse(void)
TEST_ASSERT_NULL(restored->allocs);
TEST_ASSERT_NULL(restored->accounts);
/* (e): the REAL accounting chain, run against the restored core, charges
* NOTHING for this job (job->accounts == NULL skips it, same guard live
* dispatch's own sched_account_jobs() uses) -- the worker's resInUse for
* "slots" reads back as exactly 0, matching today's pre-feature
* behavior, not an uninitialized/undefined value. */
/* (e): the REAL accounting chain -- xce_sched_core_load_snapshot()
* itself already ran it once above (task 4.6) -- charges NOTHING for
* this job (job->accounts == NULL skips it, same guard live dispatch's
* own sched_account_jobs() uses) -- the worker's resInUse for "slots"
* reads back as exactly 0, matching today's pre-feature behavior, not
* an uninitialized/undefined value. No second manual call here: with
* nothing charged either way for this NULL-accounts job, a redundant
* call would be harmless for THIS assertion specifically, but is
* omitted anyway to match this file's one-call-per-load convention
* (see Phase 8 of the round-trip test above, where a second call would
* NOT be harmless). */
worker = sched_get_worker(&core, "hostA");
TEST_ASSERT_NOT_NULL(worker);
@ -2278,9 +2458,6 @@ test_snapshot_load_leaves_running_job_without_allocation_at_zero_resinuse(void)
TEST_ASSERT_GREATER_THAN_INT(0, rc);
TEST_ASSERT_NOT_NULL(slots_res);
rc = test_sched_account_jobs(&core);
TEST_ASSERT_EQUAL_INT(0, rc);
get_resvals(&(worker->resInUse), 1, &(slots_res->idx), &resinuse_slots);
TEST_ASSERT_EQUAL_FLOAT(0.0f, resinuse_slots);
}
@ -2307,6 +2484,8 @@ int main(void)
test_snapshot_load_leaves_allocs_and_accounts_null_for_job_without_allocation);
RUN_TEST(
test_snapshot_round_trip_mixed_jobs_shared_pool_resinuse_and_teardown_independence);
RUN_TEST(
test_snapshot_load_populates_worker_resinuse_immediately_without_a_session);
RUN_TEST(
test_snapshot_load_leaves_running_job_without_allocation_at_zero_resinuse);
return UNITY_END();