diff --git a/src/scheduler/include/core.h b/src/scheduler/include/core.h index 75477c29..69b2a166 100644 --- a/src/scheduler/include/core.h +++ b/src/scheduler/include/core.h @@ -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 diff --git a/src/scheduler/src/core_events.c b/src/scheduler/src/core_events.c index 2f1d1f37..f21c9dea 100644 --- a/src/scheduler/src/core_events.c +++ b/src/scheduler/src/core_events.c @@ -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; diff --git a/src/scheduler/src/snapshot_load.c b/src/scheduler/src/snapshot_load.c index faaa91db..bce7b6a7 100644 --- a/src/scheduler/src/snapshot_load.c +++ b/src/scheduler/src/snapshot_load.c @@ -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); } } diff --git a/src/scheduler/testsuite/test_maxtasks_wiring_ingest.c b/src/scheduler/testsuite/test_maxtasks_wiring_ingest.c index 346adcb8..7c548a4f 100644 --- a/src/scheduler/testsuite/test_maxtasks_wiring_ingest.c +++ b/src/scheduler/testsuite/test_maxtasks_wiring_ingest.c @@ -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"); + } } /* ---------------------------------------------------------------------- */ diff --git a/src/scheduler/testsuite/test_sim_startup.c b/src/scheduler/testsuite/test_sim_startup.c index 0a7b9b52..c55a38d5 100644 --- a/src/scheduler/testsuite/test_sim_startup.c +++ b/src/scheduler/testsuite/test_sim_startup.c @@ -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; }