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

feat(scheduler-snapshot-job-allocation): restore allocs/accounts on load

Browse source 
Task 2.2: xce_sched_core_load_snapshot() now restores job->allocs via
dup_job_alloc() (never aliasing the wire job's own copy, since it
belongs to a const, potentially-shared snapshot struct) and rebuilds
job->accounts via the existing make_job_account(), positioned right
after task 1.2's schedobj construction and before sched_add_job().
Independent of resreq parse success -- not nested inside that guard.
Absent allocation data is the normal case: no WARN, no error.

New tests prove a full cross-core round trip (save core_a, load into
independent core_b): restored allocs/accounts content-match the
source, with three-way pointer independence (restored vs source-core
vs saved-wire) proving the deep-copy guarantee end to end. A companion
test confirms a job with no allocation loads cleanly.

Note: this task's implementation was completed directly after two
implementer subagent dispatches hit transient API interruptions; one
auto-resumed and reported a false READY_FOR_REVIEW (claiming 9/9
passing tests while the actual production code was never written,
only the test file). Caught by fresh-evidence verification before
trusting the report -- confirmed via an independent build+test run
showing a genuine failure, then fixed and re-verified by an
independent reviewer who reproduced RED-to-GREEN itself.

Requirements: 1.1, 1.2, 1.3, 1.4, 3.5
This commit is contained in:
Rongsong Shen 2026-08-27 14:30:29 +00:00
parent 6fa2d970f1
commit 182cb55574
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
3 changed files with 256 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

2
.kiro/specs/scheduler-snapshot-job-allocation/tasks.md
View file

@ -27,7 +27,7 @@
- _Boundary: snapshot save (allocation population)_
- _Note: not marked parallel with 2.2 — both tasks' tests land in the same test file (test_snapshot_save_load.c), a shared-file conflict even though the production code they touch is disjoint._
- [ ] 2.2 Restore a job's resource allocation and accounting on the load side
- [x] 2.2 Restore a job's resource allocation and accounting on the load side
- Write a failing test first (TDD red) asserting that loading a saved job that carries a resource allocation reconstructs both the job's allocation and its per-host/per-resource accounting.
- Restore the allocation via the deep-copy primitive from 1.1 (never sharing the saved representation's own copy), then rebuild the job's accounting from the restored allocation using the existing accounting constructor.
- Position this restoration immediately after the scheduling-representation construction from 1.2 within the same job-reconstruction step, before the job becomes visible to the rest of the scheduler.

21
src/scheduler/src/snapshot_load.c
View file

@ -8,6 +8,7 @@
#include "resobj.h"
#include "sched/types.h"
#include "utils.h"
#include "xcealloc.h"
#include "xstring.h"
/*
@ -461,6 +462,26 @@ int xce_sched_core_load_snapshot(xce_sched_core_t* core,
}
}
/* scheduler-snapshot-job-allocation task 2.2 (Req 1 AC 1-4, 3
* AC 5): restore job->allocs via a genuine deep copy -- never
* aliasing wj->allocs, which belongs to the caller's `in`
* (const sched_snapshot_data_t*) and may be loaded into more
* than one core (research.md "Deep-copy vs. move semantics").
* Independent of whether resreq parsed above -- an
* allocation has nothing to do with resreq parsing. Absent
* allocation data (old snapshot, or a job never dispatched)
* is the normal case: leave allocs/accounts NULL, no WARN. */
if (wj->allocs != NULL) {
job->allocs = dup_job_alloc(wj->allocs);
if (job->allocs != NULL) {
job->accounts = make_job_account(job->allocs);
} else {
WARN("%s dup_job_alloc failed for jobid %llu; this "
"job's allocation will not be restored",
__func__, (unsigned long long)wj->jobid);
}
}
sched_add_job(core, job);
if (picker != NULL) {

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

@ -1339,6 +1339,236 @@ test_snapshot_save_leaves_allocs_null_for_job_without_allocation(void)
TEST_ASSERT_NULL(saved_job->allocs);
}
/* ---------------------------------------------------------------------- */
/* scheduler-snapshot-job-allocation task 2.2: xce_sched_core_load_snapshot()
* (load side) must restore job->allocs via a genuine deep copy
* (dup_job_alloc()) whenever the wire job carries allocation data (Req 1.1-
* 1.4), and rebuild job->accounts from that restored allocation via the
* existing make_job_account() constructor -- so worker/shared resource "used"
* accounting is correct immediately on load, with no further replay
* activity required (Req 1.2, 1.3). Drives the REAL, full cross-core round
* trip -- save core_a's real state, load into a completely independent
* core_b -- the same "genuinely cross-core, not a field-level shortcut"
* convention this file's header comment establishes for the top-level
* save/load equivalence test above, since Req 1.4's independence guarantee
* can only be genuinely proven across two separately-alive cores.
* ---------------------------------------------------------------------- */
static void
test_snapshot_load_restores_allocs_and_accounts_for_running_job_across_cores(
void)
{
xce_sched_core_t core_a;
xce_sched_core_t core_b;
sched_snapshot_data_t snap;
xce_core_event_t events[2];
xce_queue_t wire_queue;
xce_job_t* wire_job;
xce_sched_job_t* job_a;
xce_alloc_t* a0;
xce_alloc_t* a1;
xce_alloc_t* a2;
xce_host_alloc_t* h0;
xce_host_alloc_t* h1;
xce_host_alloc_t** hallocs;
xce_job_alloc_t* jalloc;
xce_job_t* saved_job;
xce_sched_job_t* restored;
int rc;
memset(&g_stub_state_a, 0, sizeof(g_stub_state_a));
memset(&g_stub_state_b, 0, sizeof(g_stub_state_b));
/* ===================================================================
* Phase 1: build core_a with a RUNNING job carrying a multi-host,
* multi-resource allocation -- hostA gets TWO resources (slots, mem),
* hostB gets ONE (slots), so both "multi-host" and "multi-resource"
* are genuinely exercised, not just one alloc per host.
* =================================================================== */
fixture_init(&core_a, &g_stub_plugin_a);
memset(&wire_queue, 0, sizeof(wire_queue));
wire_queue.queue = "normal";
wire_queue.priority = 1;
wire_queue.status = QUEUE_STAT_OPEN;
wire_job = make_job(601, JOB_STAT_PEND, "normal", "alice", "proj1");
memset(events, 0, sizeof(events));
events[0].type = XCE_CORE_EVENT_QUEUE_UPDATE;
events[0].u.queue_update.queue = &wire_queue;
events[1].type = XCE_CORE_EVENT_JOBS_PUSH;
events[1].u.jobs_push.njobs = 1;
events[1].u.jobs_push.jobs = &wire_job;
rc = xce_core_push_events(&core_a, events, 2);
TEST_ASSERT_EQUAL_INT(0, rc);
job_a = sched_get_job(&core_a, 601);
TEST_ASSERT_NOT_NULL(job_a);
a0 = make_alloc((char*)"slots", NULL, 0, 2.0f, 0);
a1 = make_alloc((char*)"mem", NULL, 0, 100.0f, 0);
a2 = make_alloc((char*)"slots", NULL, 0, 3.0f, 0);
TEST_ASSERT_NOT_NULL(a0);
TEST_ASSERT_NOT_NULL(a1);
TEST_ASSERT_NOT_NULL(a2);
h0 = make_host_alloc((char*)"hostA", 2, make_allocs(2, a0, a1));
h1 = make_host_alloc((char*)"hostB", 1, make_allocs(1, a2));
TEST_ASSERT_NOT_NULL(h0);
TEST_ASSERT_NOT_NULL(h1);
hallocs = make_host_allocs(2, h0, h1);
TEST_ASSERT_NOT_NULL(hallocs);
jalloc = make_job_alloc(601, 2, hallocs);
TEST_ASSERT_NOT_NULL(jalloc);
job_a->state = JOB_STAT_RUN;
job_a->allocs = jalloc;
/* ===================================================================
* Phase 2: save core_a's snapshot.
* =================================================================== */
memset(&snap, 0, sizeof(snap));
rc = xce_sched_core_save_snapshot(&core_a, &snap);
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_NOT_NULL(snap.jobs);
TEST_ASSERT_EQUAL_INT(1, snap.jobs->count);
saved_job = snap.jobs->jobs[0];
TEST_ASSERT_NOT_NULL(saved_job);
TEST_ASSERT_NOT_NULL(saved_job->allocs);
/* ===================================================================
* Phase 3: load into core_b -- a completely separate, independently-
* alive instance.
* =================================================================== */
fixture_init(&core_b, &g_stub_plugin_b);
rc = xce_sched_core_load_snapshot(&core_b, &snap);
TEST_ASSERT_EQUAL_INT(0, rc);
restored = sched_get_job(&core_b, 601);
TEST_ASSERT_NOT_NULL(restored);
TEST_ASSERT_EQUAL_UINT32(JOB_STAT_RUN, restored->state);
/* ===================================================================
* Phase 4: job->allocs content matches the original, host-for-host,
* resource-for-resource.
* =================================================================== */
TEST_ASSERT_NOT_NULL(restored->allocs);
TEST_ASSERT_EQUAL_UINT64(601, restored->allocs->jobid);
TEST_ASSERT_EQUAL_INT(2, restored->allocs->nhosts);
TEST_ASSERT_NOT_NULL(restored->allocs->allocs[0]);
TEST_ASSERT_NOT_NULL(restored->allocs->allocs[1]);
TEST_ASSERT_EQUAL_STRING("hostA", restored->allocs->allocs[0]->host);
TEST_ASSERT_EQUAL_STRING("hostB", restored->allocs->allocs[1]->host);
TEST_ASSERT_EQUAL_INT(2, restored->allocs->allocs[0]->nallocs);
TEST_ASSERT_EQUAL_INT(1, restored->allocs->allocs[1]->nallocs);
TEST_ASSERT_EQUAL_STRING("slots",
restored->allocs->allocs[0]->allocs[0]->resname);
TEST_ASSERT_EQUAL_FLOAT(
2.0f, (float)restored->allocs->allocs[0]->allocs[0]->value);
TEST_ASSERT_EQUAL_STRING("mem",
restored->allocs->allocs[0]->allocs[1]->resname);
TEST_ASSERT_EQUAL_FLOAT(
100.0f, (float)restored->allocs->allocs[0]->allocs[1]->value);
TEST_ASSERT_EQUAL_STRING("slots",
restored->allocs->allocs[1]->allocs[0]->resname);
TEST_ASSERT_EQUAL_FLOAT(
3.0f, (float)restored->allocs->allocs[1]->allocs[0]->value);
/* ===================================================================
* Phase 5: job->accounts correctly reconstructed from the restored
* allocation via make_job_account() -- nhosts matches, and per-resource
* allocated/reserved values reflect the allocation (mirrors
* make_res_account()'s own reserved=allocated=alloc->value, used=0.0
* convention -- see xcealloc.c).
* =================================================================== */
TEST_ASSERT_NOT_NULL(restored->accounts);
TEST_ASSERT_EQUAL_INT(2, restored->accounts->nhosts);
TEST_ASSERT_NOT_NULL(restored->accounts->accounts[0]);
TEST_ASSERT_NOT_NULL(restored->accounts->accounts[1]);
TEST_ASSERT_EQUAL_INT(2, restored->accounts->accounts[0]->naccount);
TEST_ASSERT_EQUAL_INT(1, restored->accounts->accounts[1]->naccount);
TEST_ASSERT_EQUAL_FLOAT(
2.0f, (float)restored->accounts->accounts[0]->accounts[0]->allocated);
TEST_ASSERT_EQUAL_FLOAT(
2.0f, (float)restored->accounts->accounts[0]->accounts[0]->reserved);
TEST_ASSERT_EQUAL_FLOAT(
0.0f, (float)restored->accounts->accounts[0]->accounts[0]->used);
TEST_ASSERT_EQUAL_FLOAT(
100.0f, (float)restored->accounts->accounts[0]->accounts[1]->allocated);
TEST_ASSERT_EQUAL_FLOAT(
3.0f, (float)restored->accounts->accounts[1]->accounts[0]->allocated);
/* ===================================================================
* Phase 6: Req 1.4's independence guarantee, proven end-to-end across
* the full save->load round trip -- THREE genuinely distinct pointers:
* core_b's restored copy, core_a's own live copy, and the wire job's
* own (already-independent, task 2.1) copy. Freeing/mutating any one
* must never be able to corrupt either of the others.
* =================================================================== */
TEST_ASSERT_TRUE(restored->allocs != job_a->allocs);
TEST_ASSERT_TRUE(restored->allocs != saved_job->allocs);
TEST_ASSERT_TRUE(job_a->allocs != saved_job->allocs);
}
/* Companion test (Req 3.5's "absent allocation is the normal case, not an
* error" -- load-side half): a wire job with no allocation data at all
* (old snapshot file, or a job that was pending/never dispatched at save
* time) must load cleanly with job->allocs/job->accounts left at their
* calloc()-zeroed NULL default -- no crash, no WARN. */
static void
test_snapshot_load_leaves_allocs_and_accounts_null_for_job_without_allocation(
void)
{
xce_sched_core_t core;
sched_snapshot_data_t in;
xce_jobs_data_t jobs_data;
xce_queue_t wire_queue;
xce_queue_t* queues[1];
xce_job_t* wire_job;
xce_job_t* jobs[1];
xce_sched_job_t* restored;
int rc;
memset(&g_stub_state_a, 0, sizeof(g_stub_state_a));
fixture_init(&core, &g_stub_plugin_a);
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;
/* make_job() never sets ->allocs -- calloc()-zeroed NULL default,
* exactly the "old snapshot file" / "never dispatched" case Req 3.5
* covers. */
wire_job = make_job(602, JOB_STAT_PEND, "normal", "alice", "proj1");
jobs[0] = wire_job;
memset(&jobs_data, 0, sizeof(jobs_data));
jobs_data.count = 1;
jobs_data.jobs = jobs;
memset(&in, 0, sizeof(in));
in.jobs = &jobs_data;
in.queues = queues;
in.n_queues = 1;
rc = xce_sched_core_load_snapshot(&core, &in);
TEST_ASSERT_EQUAL_INT(0, rc);
restored = sched_get_job(&core, 602);
TEST_ASSERT_NOT_NULL(restored);
TEST_ASSERT_NULL(restored->allocs);
TEST_ASSERT_NULL(restored->accounts);
}
int main(void)
{
UNITY_BEGIN();
@ -1355,5 +1585,9 @@ int main(void)
RUN_TEST(
test_snapshot_save_populates_allocs_for_running_job_with_multi_host_allocation);
RUN_TEST(test_snapshot_save_leaves_allocs_null_for_job_without_allocation);
RUN_TEST(
test_snapshot_load_restores_allocs_and_accounts_for_running_job_across_cores);
RUN_TEST(
test_snapshot_load_leaves_allocs_and_accounts_null_for_job_without_allocation);
return UNITY_END();
}