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

refactor(scheduler): one schedobj builder for ingest and snapshot restore (bug-1767)

Browse source 
snapshot_load.c carried its own field-for-field copy of core_events.c's
schedobj builders (primary/compound/alter + hlimits killer), kept separate
by a "never share builders" convention. The copy drifted from the original
three times, each a field-completeness bug in restored jobs (bug-1772's
all-zero resreq_fp among them). The convention's reasons apply to the
job/queue/worker builders, not to schedobj, which is never wire-captured.

- core_build_schedobj() becomes the exported xce_core_build_schedobj(obj,
  user, queue, project, resreq) (core.h), taking identity strings instead
  of a wire job; core_build_job() and snapshot_load.c both call it.
- snapshot_load.c's five copied functions are removed.
- test_sim_startup.c #includes snapshot_load.c without core_events.c; it
  now also #includes core_events.c, plus the same
  sched_reconcile_job_resource_usage link stub test_driver_translate.c
  uses.
- test_fresh_ingest_and_snapshot_restore_derive_identical_... now compares
  the whole schedobj (tag, user/queue/project, configver, glimits, owner,
  resreq binding, resreq_fp), guarding against future drift. No RED phase:
  there is no live gap left to catch; this removes the duplication that
  kept producing them.

Scheduler suite 87/87; full build links.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-09-27 03:14:53 +00:00
parent 6103150084
commit 1dfdf42cc9
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
5 changed files with 70 additions and 158 deletions
Download patch file Download diff file Expand all files Collapse all files

10
src/scheduler/include/core.h
View file

@ -434,6 +434,16 @@ extern int sched_charge_account(xce_sched_core_t*, xce_job_account_t*);
extern int sched_uncharge_account(xce_sched_core_t*, xce_job_account_t*);
extern int sched_commit_job_alloc(xce_sched_core_t*, xce_sched_job_t*);
/*
* xce_core_build_schedobj() - fill a calloc'd xce_sched_obj_t for a job with
* the given identity and parsed resreq (defined in core_events.c). The only
* schedobj builder: fresh ingest and snapshot restore (snapshot_load.c) both
* call it, so they cannot drift apart (bug-1767).
*/
extern void xce_core_build_schedobj(xce_sched_obj_t*, const char* user,
const char* queue, const char* project,
xce_resreq_t*);
/*
* sched_check_host_satisfy()/sched_check_shared_satisfy() - relocated here
* from scheduler.h (scheduler-driver-equivalence fix, bug-1836), following

19
src/scheduler/src/core_events.c
View file

@ -207,12 +207,17 @@ static void core_hlimits_list_killer(void* item)
free(l);
}
static void core_build_schedobj(xce_sched_obj_t* obj, xce_job_t* j,
xce_resreq_t* resreq)
/* bug-1767: the ONE schedobj builder -- fresh ingest (core_build_job below)
* and snapshot restore (snapshot_load.c) both use it, so the two can no
* longer drift apart field by field (bug-1772 was such a drift). Takes the
* identity strings directly rather than a wire xce_job_t. See core.h. */
void xce_core_build_schedobj(xce_sched_obj_t* obj, const char* user,
const char* queue, const char* project,
xce_resreq_t* resreq)
{
obj->user = makeString(j->submit->user);
obj->queue = makeString(j->submit->queue);
obj->project = makeString(j->submit->project_name);
obj->user = makeString((char*)user);
obj->queue = makeString((char*)queue);
obj->project = makeString((char*)project);
xlist_init(&(obj->glimits), 8);
zarray_init(&(obj->hlimits), 32, core_hlimits_list_killer);
@ -333,7 +338,9 @@ static xce_sched_job_t* core_build_job(xce_sched_core_t* core, xce_job_t* j)
FREEUP(job);
return NULL;
}
core_build_schedobj(job->schedobj, j, job->resreq);
xce_core_build_schedobj(job->schedobj, j->submit->user,
j->submit->queue, j->submit->project_name,
job->resreq);
job->mintasks = j->submit->mintasks;
job->maxtasks = j->submit->maxtasks;

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

@ -53,157 +53,12 @@
* core_session.c's own `extern void sched_worker_release_host_ctx(...)`. */
extern xce_val_t* dup_val(const xce_val_t*);
/*
* snapshot_build_primary_schedobj/snapshot_build_compound_schedobj/
* snapshot_build_alter_schedobj/snapshot_build_schedobj -
* scheduler-snapshot-job-allocation spec, task 1.2 (Req 2 AC 1-4): a LOCAL,
* STATIC, field-for-field port of core_build_primary_schedobj()/
* core_build_compound_schedobj()/core_build_alter_schedobj()/
* core_build_schedobj() (core_events.c) -- NOT a call into those functions,
* which are `static` to core_events.c and, per this project's existing,
* deliberate convention (see this file's own header comment and
* core_events.c's), snapshot_load.c never shares helpers with
* core_events.c's builders. This is the exact same bug-1569 failure shape
* core_events.c's own header comment documents (the allocator --
* sched_make_allocation() and plugins like limit.c -- dereferences
* job->schedobj unconditionally, no NULL check) applied to the one
* remaining code path that still leaves job->schedobj NULL for every
* restored job: this feature closes it here.
*
* Needs no new snapshot data: user/queue/project come from the wire job's
* own submit fields (wj->submit->user/queue/project_name, the same fields
* core_build_schedobj() reads from its own wire job), and resreq is the
* job's own already-parsed xce_resreq_t. glimits/hlimits start empty and
* configver starts at 0, matching core_build_schedobj() exactly; alloc/
* reason are left NULL (calloc'd obj is already zeroed) -- both are
* per-scheduling-round scratch state, never persisted job state (see
* research.md's own finding for this task).
*/
static void snapshot_build_primary_schedobj(xce_sched_obj_t* obj,
xce_primary_resreq_t* resreq)
{
xce_primary_schedobj_t* primary
= (xce_primary_schedobj_t*)calloc(1, sizeof(*primary));
obj->tag = XCE_RESREQ_PRIMARY;
if (primary == NULL) {
return;
}
primary->resreq = resreq;
primary->reason = NULL;
primary->owner = obj;
/* bug-1772 fix (xce-audit-invariants scheduler, dimension 4): this
* function's own header comment documents it as a field-for-field port
* of core_build_primary_schedobj() (core_events.c) -- it had ported
* every field except this one. Without it, `primary->resreq_fp` stays
* all-zero (calloc'd) for every job restored from a snapshot, and
* alloc_cache.c's build_key_() folds resreq_fp directly into the
* unsatisfiable-cache key alongside borrowed user/queue/project
* identity -- two genuinely different restored jobs sharing a
* user/queue/project would collapse to the identical zero fingerprint
* and silently share (and incorrectly reuse) each other's
* unsatisfiable-cache verdict. Same formula, same field order, as
* core_build_primary_schedobj()'s own Req 7.1 comment. */
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;
obj->u.primary = primary;
}
static void snapshot_build_compound_schedobj(xce_sched_obj_t* obj,
xce_compound_resreq_t* resreq)
{
xce_compound_schedobj_t* compound
= (xce_compound_schedobj_t*)calloc(1, sizeof(*compound));
obj->tag = XCE_RESREQ_COMPOUND;
if (compound == NULL) {
return;
}
compound->resreq = resreq;
compound->owner = obj;
obj->u.compound = compound;
}
static void snapshot_build_alter_schedobj(xce_sched_obj_t* obj,
xce_alter_resreq_t* resreq)
{
int i;
obj->tag = XCE_RESREQ_ALTER;
xlist_init(&(obj->u.alters), 4);
for (i = 0; i < resreq->compounds.count; i++) {
xce_compound_schedobj_t* cobj = NULL;
xce_compound_resreq_t* cresreq
= (xce_compound_resreq_t*)xlist_get(&(resreq->compounds), i);
if (cresreq == NULL) {
continue;
}
cobj = (xce_compound_schedobj_t*)calloc(1, sizeof(*cobj));
if (cobj == NULL) {
continue;
}
cobj->resreq = cresreq;
cobj->owner = obj;
xlist_store(&(obj->u.alters), cobj);
}
}
/* snapshot_hlimits_list_killer - obj->hlimits item killer (bug-1848), a
* local copy of schedobj.c's kill_hlimits_list() / core_events.c's
* core_hlimits_list_killer() per this file's "never share builder helpers"
* convention (header comment above) -- several test binaries link it
* without schedobj.c. Frees one per-worker xlist_t, never its vres
* entries (owned by the limit plugin). */
static void snapshot_hlimits_list_killer(void* item)
{
xlist_t* l = (xlist_t*)item;
if (l == NULL) {
return;
}
xlist_fini(l);
free(l);
}
static void snapshot_build_schedobj(xce_sched_obj_t* obj, xce_job_t* wj,
xce_resreq_t* resreq)
{
obj->user = makeString(wj->submit->user);
obj->queue = makeString(wj->submit->queue);
obj->project = makeString(wj->submit->project_name);
xlist_init(&(obj->glimits), 8);
zarray_init(&(obj->hlimits), 32, snapshot_hlimits_list_killer);
switch (resreq->tag) {
case XCE_RESREQ_PRIMARY:
snapshot_build_primary_schedobj(obj, resreq->u.primary);
break;
case XCE_RESREQ_COMPOUND:
snapshot_build_compound_schedobj(obj, resreq->u.compound);
break;
case XCE_RESREQ_ALTER:
snapshot_build_alter_schedobj(obj, resreq->u.alter);
break;
}
obj->configver = 0;
}
/* bug-1767: snapshot restore builds each job's schedobj with the same
* xce_core_build_schedobj() fresh ingest uses (core_events.c, core.h) -- it
* used to carry its own field-for-field copy here, which drifted three times
* (bug-1772's missing resreq_fp among them). Needs no snapshot data: the
* identity strings come from the wire job's submit fields and resreq is the
* job's own already-parsed request. */
/* ------------------------------------------------------------------------
* xce_sched_core_load_snapshot
@ -518,7 +373,10 @@ int xce_sched_core_load_snapshot(xce_sched_core_t* core,
job->schedobj
= (xce_sched_obj_t*)calloc(1, sizeof(*(job->schedobj)));
if (job->schedobj != NULL) {
snapshot_build_schedobj(job->schedobj, wj, job->resreq);
xce_core_build_schedobj(job->schedobj, wj->submit->user,
wj->submit->queue,
wj->submit->project_name,
job->resreq);
}
}

26
src/scheduler/testsuite/test_maxtasks_wiring_ingest.c
View file

@ -638,6 +638,32 @@ test_fresh_ingest_and_snapshot_restore_derive_identical_task_count_for_equivalen
fresh_primary->maxTasks, restored_primary->maxTasks,
"Req 5.6: snapshot-restore ingest must derive the IDENTICAL "
"standalone primary maxTasks an equivalent fresh submission would");
/* ── bug-1767: the whole schedobj the allocator reads must be built
* identically on both paths (snapshot_load.c used to carry its own
* hand-copied builder, which drifted three times, e.g. bug-1772's
* missing resreq_fp). ───────────────────────────────────────────── */
{
xce_sched_obj_t* fo = fresh_sjob->schedobj;
xce_sched_obj_t* ro = restored_sjob->schedobj;
TEST_ASSERT_NOT_NULL(fo);
TEST_ASSERT_NOT_NULL(ro);
TEST_ASSERT_EQUAL_INT(fo->tag, ro->tag);
TEST_ASSERT_EQUAL_STRING(fo->user, ro->user);
TEST_ASSERT_EQUAL_STRING(fo->queue, ro->queue);
TEST_ASSERT_EQUAL_STRING(fo->project, ro->project);
TEST_ASSERT_EQUAL_UINT32(fo->configver, ro->configver);
TEST_ASSERT_EQUAL_INT(fo->glimits.count, ro->glimits.count);
TEST_ASSERT_NOT_NULL(fo->u.primary);
TEST_ASSERT_NOT_NULL(ro->u.primary);
TEST_ASSERT_TRUE(ro->u.primary->resreq == restored_primary);
TEST_ASSERT_TRUE(ro->u.primary->owner == ro);
TEST_ASSERT_EQUAL_MEMORY_MESSAGE(&fo->u.primary->resreq_fp,
&ro->u.primary->resreq_fp, sizeof(fo->u.primary->resreq_fp),
"bug-1767: fresh and restored schedobj resreq fingerprints "
"must be identical");
}
}
/* ---------------------------------------------------------------------- */

11
src/scheduler/testsuite/test_sim_startup.c
View file

@ -62,12 +62,23 @@
* same convention as test_record_lifecycle.c). */
#include "../src/core.c"
#include "../src/core_clock.c"
#include "../src/core_events.c" /* bug-1767: xce_core_build_schedobj() */
#include "../src/recording.c"
#include "../src/sim.c"
#include "../src/snapshot.c"
#include "../src/snapshot_load.c"
#include "../src/snapshot_wire.c"
/* core_events.c (included above for xce_core_build_schedobj(), bug-1767)
* calls this from its JOB_RUSAGE handler; this test never drives events,
* so a link-satisfying stub, the same as test_driver_translate.c's. */
int sched_reconcile_job_resource_usage(xce_sched_core_t* c,
xce_job_account_t* a,
xce_job_rusages_t* r)
{
(void)c; (void)a; (void)r; return 0;
}
void kill_sched_job(void* arg) { (void)arg; }
void kill_sched_queue(void* arg) { (void)arg; }
void kill_sched_worker(void* arg) { (void)arg; }