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

fix(scheduler): refuse a stale JOBS_PUSH resync claim that would destroy a real allocation

Browse source 
A resync redelivery (on_scheduler_join()'s full push, fired on every
scheduler reconnect) can race a job's own very-recent dispatch
confirmation and carry a stale, non-terminal state for it. The
duplicate-jobid merge in core_events.c trusted this claim
unconditionally, freeing the job's already-captured allocation and
regressing its state -- live-confirmed via a production scheduler.log
showing a resync WARN landing 16ms after the job's own RUN
confirmation, and matching a live-reported simulator symptom (a RUN
job silently missing from info worker's allocation listing).

Refuse a non-terminal claim when the existing job already has a real
allocation on record, mirroring the existing phantom-RUN guard's own
reasoning in the opposite direction; a genuinely terminal claim
(DONE/EXIT) still applies and releases the allocation correctly, same
as before.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLSNJhNoULvVcUhXEFafQC
This commit is contained in:
Rongsong Shen 2026-09-15 09:01:22 +00:00
parent 89a3dec5d9
commit 6c201bb3e8
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
2 changed files with 173 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -930,6 +930,42 @@ int xce_core_push_events(xce_sched_core_t* core, xce_core_event_t* events,
picker, existing, job->state);
}
}
} else if ((existing->allocs != NULL)
&& !JOB_IS_DONE(job->state)
&& !JOB_IS_EXIT(job->state)) {
/* bug-1812 ("stale resync regression"): symmetric
* to bug-1739's own phantom-RUN guard above, but
* for the descent direction. `existing` is
* currently RUN with a REAL, already-captured
* allocation -- this resync redelivery claims
* some OTHER, non-terminal state (e.g. PEND) with
* no allocation of its own to justify it.
* on_scheduler_join()'s full resync
* (xcesched_push_jobs(), controller/sched.c)
* fires on every scheduler reconnect, built from
* collect_jobs_committed() -- a committed-data
* snapshot that can race the scheduler's own very
* recent, self-initiated dispatch (live-confirmed
* via a production scheduler.log: a JOBS_PUSH
* resync WARN landed just 16ms after this exact
* job's own schedStartJob confirmation). Trusting
* this claim would destroy a real allocation the
* scheduler's own resource/slot bookkeeping
* already correctly has -- the same phantom-RUN-
* shaped data loss bug-1739 already guards
* against, just reached from the opposite state
* transition. A genuinely TERMINAL claim
* (DONE/EXIT) is NOT covered by this guard: that
* is legitimate "job finished while this
* scheduler was disconnected" information
* (bug-1734's own motivating case) and must
* still release the allocation and apply. */
WARN("%s JOBS_PUSH: refusing resync state 0x%x "
"for jobid %s -- existing job is RUN with a "
"real allocation on record (existing state "
"0x%x kept)",
__func__, job->state, jobidStr,
existing->state);
} else {
if (existing->accounts != NULL) {
free_job_account(existing->accounts);

137
src/scheduler/testsuite/test_core_events.c
View file

@ -739,6 +739,141 @@ test_jobs_push_duplicate_jobid_run_state_without_allocation_is_refused(void)
TEST_ASSERT_NULL(tracked->allocs);
}
/* bug-1812 ("stale resync regression"): symmetric to bug-1739's own
* "phantom RUN" guard just above (refusing to FABRICATE an allocation-less
* RUN claim), but for the descent direction -- a JOBS_PUSH resync
* redelivery must not DESTROY a job's REAL, already-captured allocation by
* trusting an unsupported non-RUN, non-terminal claim over it.
* Live-confirmed via a real production scheduler.log: on_scheduler_join()'s
* full resync (xcesched_push_jobs(), controller/sched.c) fires on every
* scheduler reconnect, built from collect_jobs_committed() -- a
* committed-data snapshot that can race the scheduler's own very recent,
* self-initiated dispatch. The log showed a JOBS_PUSH resync WARN landing
* just 16ms after this exact job's own schedStartJob confirmation
* (sched_job_state_change -> state 2), repeating in bursts on later
* reconnects -- each collision wiping the job's real allocation while its
* state stayed RUN, exactly matching the simulator's live-reported symptom
* ("info jobs" shows RUN, "info worker"'s assigned-jobs listing -- which
* walks job->allocs -- omits it). Asserts a job tracked RUN with a real
* allocation, redelivered via resync as PEND with no allocation, KEEPS its
* RUN state and real allocation -- the resync's unsupported claim must be
* refused, not trusted. */
static void
test_jobs_push_duplicate_jobid_run_with_allocation_survives_stale_nonrun_resync(
void)
{
xce_sched_core_t core;
xce_core_event_t event;
xce_job_t* job1;
xce_job_t* stale_resync_job;
xce_sched_job_t* tracked;
xce_alloc_t* a_slots;
xce_host_alloc_t* h0;
xce_host_alloc_t* hallocs[1];
xce_job_alloc_t* jalloc;
int rc;
fixture_init(&core, &g_full_plugin);
job1 = make_job(65, JOB_STAT_RUN, "normal", "alice", "proj1");
memset(&event, 0, sizeof(event));
event.type = XCE_CORE_EVENT_JOBS_PUSH;
event.u.jobs_push.njobs = 1;
event.u.jobs_push.jobs = &job1;
rc = xce_core_push_events(&core, &event, 1);
TEST_ASSERT_EQUAL_INT(0, rc);
tracked = sched_get_job(&core, 65);
TEST_ASSERT_NOT_NULL(tracked);
/* Give it a real, captured allocation -- mirrors what a genuine
* dispatch (scheduler.c's sched_commit_decision()) would already have
* set by the time a stale resync arrives. */
a_slots = make_alloc((char*)"slots", NULL, 0, 1.0f, 0);
h0 = make_host_alloc((char*)"hostA", 1, make_allocs(1, a_slots));
hallocs[0] = h0;
jalloc = make_job_alloc(65, 1, make_host_allocs(1, hallocs[0]));
tracked->allocs = jalloc;
tracked->accounts = make_job_account(tracked->allocs);
/* A stale on_scheduler_join() resync redelivery -- the controller's
* committed-data snapshot hasn't yet caught up to this job's own very
* recent dispatch, so it still reports PEND with no allocation. */
stale_resync_job = make_job(65, JOB_STAT_PEND, "normal", "alice", "proj1");
memset(&event, 0, sizeof(event));
event.type = XCE_CORE_EVENT_JOBS_PUSH;
event.u.jobs_push.njobs = 1;
event.u.jobs_push.jobs = &stale_resync_job;
rc = xce_core_push_events(&core, &event, 1);
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_PTR(tracked, sched_get_job(&core, 65));
/* Must still be RUN with its real allocation intact -- the stale resync
* claim must not have clobbered it. */
TEST_ASSERT_EQUAL_UINT32(JOB_STAT_RUN, tracked->state);
TEST_ASSERT_EQUAL_PTR(jalloc, tracked->allocs);
TEST_ASSERT_NOT_NULL(tracked->accounts);
}
/* Companion: a resync claiming a genuinely TERMINAL state (DONE/EXIT) for a
* RUN-with-real-allocation job must still apply -- this is the legitimate
* "job finished while this scheduler was disconnected" case bug-1734's own
* fix exists for, and the new stale-resync guard above must not block it. */
static void
test_jobs_push_duplicate_jobid_terminal_resync_releases_real_allocation(void)
{
xce_sched_core_t core;
xce_core_event_t event;
xce_job_t* job1;
xce_job_t* finished_resync_job;
xce_sched_job_t* tracked;
xce_alloc_t* a_slots;
xce_host_alloc_t* h0;
xce_host_alloc_t* hallocs[1];
int rc;
fixture_init(&core, &g_full_plugin);
job1 = make_job(66, JOB_STAT_RUN, "normal", "alice", "proj1");
memset(&event, 0, sizeof(event));
event.type = XCE_CORE_EVENT_JOBS_PUSH;
event.u.jobs_push.njobs = 1;
event.u.jobs_push.jobs = &job1;
rc = xce_core_push_events(&core, &event, 1);
TEST_ASSERT_EQUAL_INT(0, rc);
tracked = sched_get_job(&core, 66);
TEST_ASSERT_NOT_NULL(tracked);
a_slots = make_alloc((char*)"slots", NULL, 0, 1.0f, 0);
h0 = make_host_alloc((char*)"hostA", 1, make_allocs(1, a_slots));
hallocs[0] = h0;
tracked->allocs = make_job_alloc(66, 1, make_host_allocs(1, hallocs[0]));
tracked->accounts = make_job_account(tracked->allocs);
finished_resync_job
= make_job(66, JOB_STAT_DONE, "normal", "alice", "proj1");
memset(&event, 0, sizeof(event));
event.type = XCE_CORE_EVENT_JOBS_PUSH;
event.u.jobs_push.njobs = 1;
event.u.jobs_push.jobs = &finished_resync_job;
rc = xce_core_push_events(&core, &event, 1);
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_PTR(tracked, sched_get_job(&core, 66));
TEST_ASSERT_EQUAL_UINT32(JOB_STAT_DONE, tracked->state);
TEST_ASSERT_NULL(tracked->allocs);
TEST_ASSERT_NULL(tracked->accounts);
}
/* bug-1738: the JOBS_PUSH duplicate-jobid merge must never regress an
* already-terminal job to a DIFFERENT terminal state either -- a resync can
* just as easily carry a stale snapshot as a genuinely newer one. Pushes
@ -1717,6 +1852,8 @@ int main(void)
RUN_TEST(test_jobs_push_duplicate_jobid_does_not_call_on_job_push_or_leak);
RUN_TEST(test_jobs_push_duplicate_jobid_with_newer_state_updates_existing_job);
RUN_TEST(test_jobs_push_duplicate_jobid_run_state_transfers_allocation);
RUN_TEST(test_jobs_push_duplicate_jobid_run_with_allocation_survives_stale_nonrun_resync);
RUN_TEST(test_jobs_push_duplicate_jobid_terminal_resync_releases_real_allocation);
RUN_TEST(test_jobs_push_duplicate_jobid_run_state_without_allocation_is_refused);
RUN_TEST(test_jobs_push_duplicate_jobid_does_not_regress_terminal_state);
RUN_TEST(test_job_state_change_does_not_regress_terminal_state);