fix(scheduler): snapshots save a job's resreq text, not its debug dump (bug-1888)

build_wire_job() filled the saved job's submit->resreq with
format_resreq()'s human-readable dump ("Primary:\n  Tasks: ..."). The
loader's parser accepted that as a primary with neither select nor
rusage, so every job restored from a snapshot (a replay's start snapshot,
REPL snapshots) lost both.

A scheduler job now keeps the text its resreq was parsed from
(xce_sched_job_t.resreq_str, set at ingest, JOB_UPDATE and snapshot load,
freed at teardown) and the snapshot saves that; a job without it restores
as the loader's "type == any" default.

Test: test_snapshot_round_trip_keeps_job_resreq_text RED before (saved
text was the dump), GREEN after.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-09-29 10:26:21 +00:00
parent 3e873f0a66
commit 077b1213d3
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
7 changed files with 83 additions and 9 deletions

View file

@ -139,6 +139,10 @@ typedef struct xce_sched_job_ {
* Owned by the job: replaced on each JOB_RUSAGE update, freed in
* kill_sched_job. NULL until the first rusage report. */
xce_job_rusages_t* rusages;
/* bug-1888: the resreq text `resreq` was parsed from (after the
* "type == any" default), saved by snapshots so a restored job gets the
* same request back. Owned; NULL for a job built without one. */
char* resreq_str;
} xce_sched_job_t;
typedef struct {

View file

@ -275,6 +275,7 @@ static void core_job_free_(xce_sched_job_t* job)
}
free_JobAlloc_ptr(&(job->allocs));
free_job_account(job->accounts);
free(job->resreq_str);
free(job);
}

View file

@ -305,6 +305,9 @@ static xce_sched_job_t* core_build_job(xce_sched_core_t* core, xce_job_t* j)
resreq_str = "type == any";
}
rc = parse_resreq(job->resreq, resreq_str, core->resmgr, &errmsg);
if (rc >= 0) {
job->resreq_str = strdup(resreq_str);
}
}
if (rc < 0) {
@ -337,6 +340,7 @@ static xce_sched_job_t* core_build_job(xce_sched_core_t* core, xce_job_t* j)
if (job->schedobj == NULL) {
kill_resreq(job->resreq);
FREEUP(job->resreq);
FREEUP(job->resreq_str);
FREEUP(job);
return NULL;
}
@ -1061,6 +1065,7 @@ int xce_core_push_events(xce_sched_core_t* core, xce_core_event_t* events,
xce_core_free_schedobj(job->schedobj);
kill_resreq(job->resreq);
FREEUP(job->resreq);
FREEUP(job->resreq_str);
FREEUP(job);
continue;
}
@ -1390,6 +1395,8 @@ int xce_core_push_events(xce_sched_core_t* core, xce_core_event_t* events,
kill_resreq(job->resreq);
FREEUP(job->resreq);
job->resreq = newreq;
FREEUP(job->resreq_str);
job->resreq_str = strdup(updated->submit->resreq);
}
}

View file

@ -199,6 +199,9 @@ xce_sched_job_t* make_sched_job(void* arg, xce_job_t* j)
resreq_str = "type == any";
}
rc = parse_resreq(job->resreq, resreq_str, sched->core.resmgr, &errmsg);
if (rc >= 0) {
job->resreq_str = strdup(resreq_str);
}
}
if (rc < 0) {
ERROR("%s parse_resreq failed, %s", __func__, errmsg);
@ -720,6 +723,8 @@ int sched_update_existing_job(xce_scheduler_t* sched, xce_job_t* updated)
free(job->resreq);
job->resreq = newreq;
FREEUP(job->resreq_str);
job->resreq_str = strdup(updated->submit->resreq);
}
}

View file

@ -344,6 +344,7 @@ int xce_sched_core_load_snapshot(xce_sched_core_t* core,
FREEUP(errmsg);
FREEUP(job->resreq);
} else {
job->resreq_str = strdup(resreq_str);
/* resreq-maxtasks-wiring task 2.2 (bug-1770, Req 1.3):
* overwrite the just-parsed resreq's standalone
* primary minTasks/maxTasks with the job's real

View file

@ -158,15 +158,13 @@ xce_job_t* build_wire_job(xce_sched_job_t* job)
/* job->priority: no wire slot -- see snapshot.c header "Scope
* decisions". */
if (job->resreq != NULL) {
int rc = format_resreq(&resreq_str, job->resreq);
if (rc < 0) {
WARN("%s format_resreq failed for jobid %llu (rc=%d); resreq "
"will be empty for this restored job",
__func__, (unsigned long long)job->jobid, rc);
resreq_str = NULL;
}
/* bug-1888: the text the job's resreq was parsed from. This used to be
* format_resreq()'s human-readable dump ("Primary:\n Tasks: ..."),
* which the loader's parser accepted as a primary with neither select
* nor rusage, so every restored job lost both. A job without the text
* restores as the loader's "type == any" default. */
if (job->resreq_str != NULL) {
resreq_str = strdup(job->resreq_str);
}
j->submit->resreq = resreq_str;

View file

@ -2601,9 +2601,67 @@ test_snapshot_load_leaves_running_job_without_allocation_at_zero_resinuse(void)
TEST_ASSERT_EQUAL_FLOAT(0.0f, resinuse_slots);
}
/*
* bug-1888: a snapshot saves the text a job's resreq was parsed from, so
* the restored job gets the same request back. It saved format_resreq()'s
* human-readable dump instead, which the loader parsed as a primary with
* neither select nor rusage.
*/
static void test_snapshot_round_trip_keeps_job_resreq_text(void)
{
xce_sched_core_t core_a;
xce_sched_core_t core_b;
xce_queue_t wire_queue;
xce_job_t* wire_job;
xce_core_event_t events[2];
sched_snapshot_data_t snap;
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));
fixture_init(&core_a, &g_stub_plugin_a);
fixture_init(&core_b, &g_stub_plugin_b);
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_with_resreq(503, JOB_STAT_PEND, "normal", "alice",
"proj1", "rusage[mem=2]");
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);
memset(&snap, 0, sizeof(snap));
rc = xce_sched_core_save_snapshot(&core_a, &snap);
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_INT(1, snap.jobs->count);
TEST_ASSERT_EQUAL_STRING("rusage[mem=2]",
snap.jobs->jobs[0]->submit->resreq);
rc = xce_sched_core_load_snapshot(&core_b, &snap);
TEST_ASSERT_EQUAL_INT(0, rc);
restored = sched_get_job(&core_b, 503);
TEST_ASSERT_NOT_NULL(restored);
TEST_ASSERT_NOT_NULL(restored->resreq);
TEST_ASSERT_EQUAL_INT(XCE_RESREQ_PRIMARY, restored->resreq->tag);
TEST_ASSERT_EQUAL_INT_MESSAGE(
1, restored->resreq->u.primary->rusages.count,
"the restored job must keep its rusage");
}
int main(void)
{
UNITY_BEGIN();
RUN_TEST(test_snapshot_round_trip_keeps_job_resreq_text);
RUN_TEST(
test_snapshot_guards_reject_null_args_and_busy_session_without_mutating_state);
RUN_TEST(