feat(scheduler-snapshot-job-allocation): construct schedobj for restored jobs
Task 1.2: snapshot_load.c gains a local, static field-for-field port of core_events.c's core_build_schedobj()/core_build_primary_schedobj()/ core_build_compound_schedobj()/core_build_alter_schedobj() (the bug-1569 fix for the JOBS_PUSH path). Every job restored from a snapshot whose resreq parsed successfully now gets a valid, non-NULL job->schedobj, closing the same NULL-deref crash class for the snapshot-load path. Guarded on job->resreq != NULL so the pre-existing, unrelated resreq-parse-failure path is untouched (still leaves schedobj NULL, same as today -- not fixed here, per design.md's documented boundary). New tests prove non-NULL schedobj with correct tag/union member for primary, compound, and alter resource requests, plus a regression test confirming the parse-failure path's behavior is unchanged. Requirements: 2.1, 2.2, 2.3, 2.4
This commit is contained in:
parent
097e33ae34
commit
22353849f6
3 changed files with 335 additions and 1 deletions
|
|
@ -9,7 +9,7 @@
|
|||
- _Requirements: 1.1, 1.4_
|
||||
- _Boundary: xcealloc (job allocation deep-copy)_
|
||||
|
||||
- [ ] 1.2 (P) Construct a valid scheduling representation for every job restored from a snapshot
|
||||
- [x] 1.2 (P) Construct a valid scheduling representation for every job restored from a snapshot
|
||||
- Write a failing test first (TDD red) asserting that a job reconstructed from a snapshot has a non-NULL, correctly-shaped internal scheduling representation for a primary, a compound, and an alternative resource request.
|
||||
- Implement construction using only information already available by the time a restored job's resource request has been parsed (its queue, user, project, and resource request) — no new snapshot data is captured or required.
|
||||
- Skip construction (leaving today's behavior unchanged) when a restored job's resource request itself failed to parse — do not attempt to fix that separate, pre-existing failure path as part of this task.
|
||||
|
|
|
|||
|
|
@ -51,6 +51,121 @@
|
|||
* 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;
|
||||
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
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, NULL);
|
||||
|
||||
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;
|
||||
}
|
||||
|
||||
/* ------------------------------------------------------------------------
|
||||
* xce_sched_core_load_snapshot
|
||||
* ---------------------------------------------------------------------- */
|
||||
|
|
@ -331,6 +446,21 @@ int xce_sched_core_load_snapshot(xce_sched_core_t* core,
|
|||
}
|
||||
}
|
||||
|
||||
/* scheduler-snapshot-job-allocation task 1.2 (Req 2 AC 1-4):
|
||||
* construct job->schedobj for every restored job whose resreq
|
||||
* parsed successfully -- guarded on job->resreq != NULL so the
|
||||
* pre-existing, unrelated resreq-parse-failure path above
|
||||
* (which already leaves job->resreq NULL) is tolerated, not
|
||||
* fixed, here (design.md "Load-Side Job Reconstruction", Known
|
||||
* limitation). */
|
||||
if (job->resreq != NULL) {
|
||||
job->schedobj
|
||||
= (xce_sched_obj_t*)calloc(1, sizeof(*(job->schedobj)));
|
||||
if (job->schedobj != NULL) {
|
||||
snapshot_build_schedobj(job->schedobj, wj, job->resreq);
|
||||
}
|
||||
}
|
||||
|
||||
sched_add_job(core, job);
|
||||
|
||||
if (picker != NULL) {
|
||||
|
|
|
|||
|
|
@ -357,6 +357,21 @@ static xce_job_t* make_job(uint64_t jobid, uint32_t state, char* queue,
|
|||
return j;
|
||||
}
|
||||
|
||||
/* make_job_with_resreq - same fixture as make_job() above, but with a
|
||||
* caller-supplied resreq string, so task 1.2's schedobj tests can build
|
||||
* PRIMARY/COMPOUND/ALTER-tagged jobs (and a deliberately-unparseable one)
|
||||
* without duplicating make_job()'s own field-population logic. */
|
||||
static xce_job_t* make_job_with_resreq(uint64_t jobid, uint32_t state,
|
||||
char* queue, char* user, char* project,
|
||||
char* resreq)
|
||||
{
|
||||
xce_job_t* j = make_job(jobid, state, queue, user, project);
|
||||
|
||||
j->submit->resreq = resreq;
|
||||
|
||||
return j;
|
||||
}
|
||||
|
||||
#define MAX_RECORDED_DECISIONS 8
|
||||
|
||||
typedef struct {
|
||||
|
|
@ -975,6 +990,191 @@ test_snapshot_save_then_load_produces_behaviorally_equivalent_core(void)
|
|||
TEST_ASSERT_EQUAL_UINT64(203, driver_ctx_a.seen[2]->u.reason->jobid);
|
||||
}
|
||||
|
||||
/* ---------------------------------------------------------------------- */
|
||||
/* scheduler-snapshot-job-allocation task 1.2: every job restored from a */
|
||||
/* snapshot must get a non-NULL, correctly-shaped job->schedobj -- the */
|
||||
/* same bug-1569 failure shape (core_events.c's own header comment), */
|
||||
/* still open on THIS load path. Covers a PRIMARY, a COMPOUND, and an */
|
||||
/* ALTER resource request (design.md "Load-Side Job Reconstruction"). */
|
||||
/* ---------------------------------------------------------------------- */
|
||||
|
||||
static void
|
||||
test_snapshot_load_builds_schedobj_for_primary_compound_and_alter_resreq(void)
|
||||
{
|
||||
xce_sched_core_t core;
|
||||
sched_snapshot_data_t in;
|
||||
xce_jobs_data_t jobs_data;
|
||||
xce_queue_t wire_queue_normal;
|
||||
xce_queue_t wire_queue_batch;
|
||||
xce_queue_t* queues[2];
|
||||
xce_job_t* job_primary;
|
||||
xce_job_t* job_compound;
|
||||
xce_job_t* job_alter;
|
||||
xce_job_t* jobs[3];
|
||||
xce_sched_job_t* restored_primary;
|
||||
xce_sched_job_t* restored_compound;
|
||||
xce_sched_job_t* restored_alter;
|
||||
int rc;
|
||||
|
||||
memset(&g_stub_state_a, 0, sizeof(g_stub_state_a));
|
||||
fixture_init(&core, &g_stub_plugin_a);
|
||||
|
||||
/* sched_get_queue() is NOT get-or-create (unlike sched_get_user()/
|
||||
* sched_get_project()) -- both queues this test's jobs reference must
|
||||
* be present in the snapshot input, same as the cross-core test above. */
|
||||
memset(&wire_queue_normal, 0, sizeof(wire_queue_normal));
|
||||
wire_queue_normal.queue = "normal";
|
||||
wire_queue_normal.priority = 1;
|
||||
wire_queue_normal.status = QUEUE_STAT_OPEN;
|
||||
|
||||
memset(&wire_queue_batch, 0, sizeof(wire_queue_batch));
|
||||
wire_queue_batch.queue = "batch";
|
||||
wire_queue_batch.priority = 1;
|
||||
wire_queue_batch.status = QUEUE_STAT_OPEN;
|
||||
|
||||
queues[0] = &wire_queue_normal;
|
||||
queues[1] = &wire_queue_batch;
|
||||
|
||||
job_primary = make_job_with_resreq(401, JOB_STAT_PEND, "normal", "alice",
|
||||
"proj1", "type == any");
|
||||
/* COMPOUND: '+' between two '{...}' terms (test_resreq.c's own
|
||||
* test_resreq2() fixture -- proven, real COMPOUND-tagged input). */
|
||||
job_compound
|
||||
= make_job_with_resreq(402, JOB_STAT_PEND, "normal", "bob", "proj1",
|
||||
"2*{rusage[mem=100:duration=20:decay=1]} + "
|
||||
"4*{rusage[mem=80:duration=30:decay=1]}");
|
||||
/* ALTER: '||' between two '{compound}' terms (test_resreq.c's own
|
||||
* test_resreq3() fixture -- proven, real ALTER-tagged input). */
|
||||
job_alter
|
||||
= make_job_with_resreq(403, JOB_STAT_PEND, "batch", "carol", "proj2",
|
||||
"{2*{rusage[mem=100:duration=20:decay=1]} + "
|
||||
"4*{rusage[mem=80:duration=30:decay=1]}} || "
|
||||
"{3*{rusage[mem=150:duration=15:decay=1]} + "
|
||||
"6*{rusage[mem=120:duration=20:decay=1]}}");
|
||||
|
||||
jobs[0] = job_primary;
|
||||
jobs[1] = job_compound;
|
||||
jobs[2] = job_alter;
|
||||
|
||||
memset(&jobs_data, 0, sizeof(jobs_data));
|
||||
jobs_data.count = 3;
|
||||
jobs_data.jobs = jobs;
|
||||
|
||||
memset(&in, 0, sizeof(in));
|
||||
in.jobs = &jobs_data;
|
||||
in.queues = queues;
|
||||
in.n_queues = 2;
|
||||
|
||||
rc = xce_sched_core_load_snapshot(&core, &in);
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
|
||||
restored_primary = sched_get_job(&core, 401);
|
||||
restored_compound = sched_get_job(&core, 402);
|
||||
restored_alter = sched_get_job(&core, 403);
|
||||
TEST_ASSERT_NOT_NULL(restored_primary);
|
||||
TEST_ASSERT_NOT_NULL(restored_compound);
|
||||
TEST_ASSERT_NOT_NULL(restored_alter);
|
||||
|
||||
/* (a) PRIMARY */
|
||||
TEST_ASSERT_NOT_NULL(restored_primary->resreq);
|
||||
TEST_ASSERT_NOT_NULL(restored_primary->schedobj);
|
||||
TEST_ASSERT_EQUAL_INT(XCE_RESREQ_PRIMARY, restored_primary->schedobj->tag);
|
||||
TEST_ASSERT_NOT_NULL(restored_primary->schedobj->u.primary);
|
||||
|
||||
/* (b) COMPOUND */
|
||||
TEST_ASSERT_NOT_NULL(restored_compound->resreq);
|
||||
TEST_ASSERT_NOT_NULL(restored_compound->schedobj);
|
||||
TEST_ASSERT_EQUAL_INT(XCE_RESREQ_COMPOUND,
|
||||
restored_compound->schedobj->tag);
|
||||
TEST_ASSERT_NOT_NULL(restored_compound->schedobj->u.compound);
|
||||
|
||||
/* (c) ALTER */
|
||||
TEST_ASSERT_NOT_NULL(restored_alter->resreq);
|
||||
TEST_ASSERT_NOT_NULL(restored_alter->schedobj);
|
||||
TEST_ASSERT_EQUAL_INT(XCE_RESREQ_ALTER, restored_alter->schedobj->tag);
|
||||
TEST_ASSERT_TRUE(restored_alter->schedobj->u.alters.count > 0);
|
||||
|
||||
/* (d) the restored schedobj's user/queue/project string values match
|
||||
* the job's own (restored via sched_get_user/queue/project), for all
|
||||
* three -- proving snapshot_build_schedobj() reads the same wire
|
||||
* fields core_build_schedobj() would have. */
|
||||
TEST_ASSERT_EQUAL_STRING(restored_primary->user->name,
|
||||
restored_primary->schedobj->user);
|
||||
TEST_ASSERT_EQUAL_STRING(restored_primary->queue->name,
|
||||
restored_primary->schedobj->queue);
|
||||
TEST_ASSERT_EQUAL_STRING(restored_primary->project->name,
|
||||
restored_primary->schedobj->project);
|
||||
|
||||
TEST_ASSERT_EQUAL_STRING(restored_compound->user->name,
|
||||
restored_compound->schedobj->user);
|
||||
TEST_ASSERT_EQUAL_STRING(restored_compound->queue->name,
|
||||
restored_compound->schedobj->queue);
|
||||
TEST_ASSERT_EQUAL_STRING(restored_compound->project->name,
|
||||
restored_compound->schedobj->project);
|
||||
|
||||
TEST_ASSERT_EQUAL_STRING(restored_alter->user->name,
|
||||
restored_alter->schedobj->user);
|
||||
TEST_ASSERT_EQUAL_STRING(restored_alter->queue->name,
|
||||
restored_alter->schedobj->queue);
|
||||
TEST_ASSERT_EQUAL_STRING(restored_alter->project->name,
|
||||
restored_alter->schedobj->project);
|
||||
}
|
||||
|
||||
/* Regression-safety test (NOT a new feature): a job whose resreq fails to
|
||||
* parse on load is a pre-existing, out-of-boundary gap (design.md "Known
|
||||
* limitation") that task 1.2 must NOT try to fix -- it must only make sure
|
||||
* schedobj construction is skipped rather than crashing on a NULL resreq.
|
||||
* Proves job->schedobj stays NULL, identical to today's behavior. */
|
||||
static void
|
||||
test_snapshot_load_leaves_schedobj_null_when_resreq_fails_to_parse(void)
|
||||
{
|
||||
xce_sched_core_t core;
|
||||
sched_snapshot_data_t in;
|
||||
xce_jobs_data_t jobs_data;
|
||||
xce_job_t* bad_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);
|
||||
|
||||
/* An unbalanced/unterminated compound-resreq brace: the top-level PEG
|
||||
* grammar (xce_resreq.peg) requires a full input match through EOF; a
|
||||
* lone unterminated '{' cannot complete as alter/compound/primary and
|
||||
* leaves trailing unconsumed input, so parse_resreq() genuinely fails
|
||||
* (unlike plain non-brace garbage, which the grammar's permissive
|
||||
* primary_resreq rule accepts as a literal select expression and only
|
||||
* fails later, silently, inside parse_resreq_primary() without
|
||||
* propagating an error -- confirmed empirically while writing this
|
||||
* test). */
|
||||
bad_job = make_job_with_resreq(404, JOB_STAT_PEND, "normal", "alice",
|
||||
"proj1", "{");
|
||||
|
||||
jobs[0] = bad_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;
|
||||
|
||||
rc = xce_sched_core_load_snapshot(&core, &in);
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
|
||||
restored = sched_get_job(&core, 404);
|
||||
TEST_ASSERT_NOT_NULL(restored);
|
||||
|
||||
/* pre-existing, unrelated gap: resreq parse failure leaves job->resreq
|
||||
* NULL (FREEUP'd in xce_sched_core_load_snapshot's own job loop) --
|
||||
* unchanged by this feature. */
|
||||
TEST_ASSERT_NULL(restored->resreq);
|
||||
/* THIS feature's guard: schedobj construction is skipped, not crashed
|
||||
* into, when resreq is NULL. */
|
||||
TEST_ASSERT_NULL(restored->schedobj);
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
UNITY_BEGIN();
|
||||
|
|
@ -984,5 +1184,9 @@ int main(void)
|
|||
test_snapshot_save_and_load_reject_during_genuinely_active_session);
|
||||
RUN_TEST(
|
||||
test_snapshot_save_then_load_produces_behaviorally_equivalent_core);
|
||||
RUN_TEST(
|
||||
test_snapshot_load_builds_schedobj_for_primary_compound_and_alter_resreq);
|
||||
RUN_TEST(
|
||||
test_snapshot_load_leaves_schedobj_null_when_resreq_fails_to_parse);
|
||||
return UNITY_END();
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue