From 077b1213d34e2ad92a7e1d810efff0daf51d6f59 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Tue, 29 Sep 2026 10:26:21 +0000 Subject: [PATCH] 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) --- src/scheduler/include/sched/types.h | 4 ++ src/scheduler/src/core.c | 1 + src/scheduler/src/core_events.c | 7 +++ src/scheduler/src/schedobj.c | 5 ++ src/scheduler/src/snapshot_load.c | 1 + src/scheduler/src/snapshot_wire.c | 16 +++-- .../testsuite/test_snapshot_save_load.c | 58 +++++++++++++++++++ 7 files changed, 83 insertions(+), 9 deletions(-) diff --git a/src/scheduler/include/sched/types.h b/src/scheduler/include/sched/types.h index 25d3f880..919c00ae 100644 --- a/src/scheduler/include/sched/types.h +++ b/src/scheduler/include/sched/types.h @@ -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 { diff --git a/src/scheduler/src/core.c b/src/scheduler/src/core.c index f249b83a..ae4d1808 100644 --- a/src/scheduler/src/core.c +++ b/src/scheduler/src/core.c @@ -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); } diff --git a/src/scheduler/src/core_events.c b/src/scheduler/src/core_events.c index c720afc2..c4c31235 100644 --- a/src/scheduler/src/core_events.c +++ b/src/scheduler/src/core_events.c @@ -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); } } diff --git a/src/scheduler/src/schedobj.c b/src/scheduler/src/schedobj.c index 96d62592..a021715c 100644 --- a/src/scheduler/src/schedobj.c +++ b/src/scheduler/src/schedobj.c @@ -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); } } diff --git a/src/scheduler/src/snapshot_load.c b/src/scheduler/src/snapshot_load.c index bce7b6a7..7f89dd8c 100644 --- a/src/scheduler/src/snapshot_load.c +++ b/src/scheduler/src/snapshot_load.c @@ -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 diff --git a/src/scheduler/src/snapshot_wire.c b/src/scheduler/src/snapshot_wire.c index 5eda3aa4..ba2b5ee7 100644 --- a/src/scheduler/src/snapshot_wire.c +++ b/src/scheduler/src/snapshot_wire.c @@ -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; diff --git a/src/scheduler/testsuite/test_snapshot_save_load.c b/src/scheduler/testsuite/test_snapshot_save_load.c index e9ecc7cf..18cf570d 100644 --- a/src/scheduler/testsuite/test_snapshot_save_load.c +++ b/src/scheduler/testsuite/test_snapshot_save_load.c @@ -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(