From 281a026d407291217e362d2ea61eb073aca2a167 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sun, 23 Aug 2026 09:25:38 +0000 Subject: [PATCH] feat(scheduler-dispatch-commit): scheduler processes dispatch-fail notify, returns job to PEND (task 2.2) Adds a case for NotifyData_data_jobDispatchFail to sched_handle_notify()'s switch, fixing bug-1625 -- the notify was previously silently discarded (falling to default: -EINVAL), leaving job->accounts/job->allocs stale forever on any dispatch failure. No classification logic here: the controller has already decided PEND is correct before sending the notify; this reuses the same XCE_SCHED_OP_JSTATE_CHANGE enqueue path the adjacent schedExitJob/schedFailJob cases already use, and relies on core_events.c's existing free-on-non-RUN branch to release the stale accounts/allocs. Independently reviewed and approved: full scheduler suite passes (51/52, one pre-existing unrelated flake -- bug-1560). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Cxa1PmqAB5PyXfJaNvj8TN --- .../specs/scheduler-dispatch-commit/tasks.md | 2 +- src/scheduler/src/handler.c | 18 +++++ src/scheduler/testsuite/test_handlers.c | 67 +++++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) diff --git a/.kiro/specs/scheduler-dispatch-commit/tasks.md b/.kiro/specs/scheduler-dispatch-commit/tasks.md index 40f11e3a..7eed5b5d 100644 --- a/.kiro/specs/scheduler-dispatch-commit/tasks.md +++ b/.kiro/specs/scheduler-dispatch-commit/tasks.md @@ -31,7 +31,7 @@ - _Requirements: 1.1, 1.2, 1.3_ - _Boundary: Scheduler Dispatch Commit_ -- [ ] 2.2 (P) Scheduler processes a reported dispatch failure by returning the job to a pending state +- [x] 2.2 (P) Scheduler processes a reported dispatch failure by returning the job to a pending state - When the scheduler is told a job's dispatch failed, it returns that job to a pending state instead of silently ignoring the report as it does today - Returning a job to pending correctly releases the resource-allocation data that was tentatively committed for it, reusing the scheduler's existing behavior for any other transition away from a running state - Observable completion: a job whose dispatch failure is reported to the scheduler is again eligible for the scheduler's own job-selection process in a subsequent round, with no dangling resource-allocation data left behind diff --git a/src/scheduler/src/handler.c b/src/scheduler/src/handler.c index 927cbfd3..88ceaf11 100644 --- a/src/scheduler/src/handler.c +++ b/src/scheduler/src/handler.c @@ -515,6 +515,24 @@ int sched_handle_notify(xce_scheduler_t* sched, xce_notify_data_t* notify) rc = sched_job_state_change(sched, notify->data.sched_fail_job, JOB_STAT_EXIT); break; + case NotifyData_data_jobDispatchFail: + /* scheduler-dispatch-commit task 2.2 (the actual fix for bug-1625): + * previously fell to `default: -EINVAL`, silently discarding the + * controller's existing dispatch-failure report and leaving + * job->accounts/job->allocs stale forever. The controller has + * already decided this resolution before sending the notify -- no + * classification logic belongs here, just return the job to + * pending through the same enqueue path schedExitJob/schedFailJob + * above already use. Freeing the stale accounts/allocs is handled + * automatically by core_events.c's existing free-on-non-RUN branch + * once this enqueued change is applied. */ + if (notify->data.dispatchfail == NULL) { + rc = -EINVAL; + break; + } + rc = sched_job_state_change(sched, notify->data.dispatchfail->jobid, + JOB_STAT_PEND); + break; case NotifyData_data_schedUpdateJob: rc = sched_handle_sched_update_job(sched, notify->data.sched_update_job); diff --git a/src/scheduler/testsuite/test_handlers.c b/src/scheduler/testsuite/test_handlers.c index e8c98426..3beb2757 100644 --- a/src/scheduler/testsuite/test_handlers.c +++ b/src/scheduler/testsuite/test_handlers.c @@ -91,6 +91,33 @@ static int pop_op(xce_scheduler_t* sched, xce_sched_op_t* op_out) return 1; } +/* Dequeue one pending request and assert it is an XCE_SCHED_OP_JSTATE_CHANGE, + * returning its jobid/state via jobid_out/state_out (both optional). Frees + * the request shell. Returns 1 if an item was present, 0 if the queue was + * empty (scheduler-dispatch-commit task 2.2: verifies the jobDispatchFail + * notify enqueues the same op the adjacent schedExitJob/schedFailJob cases + * already use). */ +static int pop_jstate_change(xce_scheduler_t* sched, uint64_t* jobid_out, + uint32_t* state_out) +{ + data_item_t* it = dataq_deq(sched->core.pend_updates[0], NULL); + xce_sched_update_request_t* req; + + if (it == NULL) { + return 0; + } + req = ITPTR_TO_UPDATE_REQ(it); + TEST_ASSERT_EQUAL_INT(XCE_SCHED_OP_JSTATE_CHANGE, req->op); + if (jobid_out != NULL) { + *jobid_out = req->u.job_state_change.jobid; + } + if (state_out != NULL) { + *state_out = req->u.job_state_change.state; + } + free(req); + return 1; +} + /* ── 9.3: notify single-item handlers (NULL -> -EINVAL, valid -> enqueue) ── */ static void test_handle_update_job_enqueues_job_fields(void) @@ -199,6 +226,45 @@ static void test_handle_resvals_full_skips_empty_enqueues_nonempty(void) free(one[0]); } +/* ── scheduler-dispatch-commit task 2.2: sched_handle_notify's new + * jobDispatchFail case (bug-1625's actual fix) ── */ + +static void test_handle_notify_job_dispatch_fail_enqueues_pend(void) +{ + xce_scheduler_t sched; + xce_notify_data_t notify; + job_failinfo_t info = { .jobid = 99, .stage = 0, .code = -5, + .detail = NULL }; + uint64_t jobid = 0; + uint32_t state = (uint32_t)-1; + fixture_init(&sched); + + memset(¬ify, 0, sizeof(notify)); + notify.event = NotifyData_data_jobDispatchFail; + notify.data.dispatchfail = &info; + + /* Previously fell to `default: rc = -EINVAL;` and enqueued nothing -- + * this is the actual observable fix for bug-1625: the notify is now + * processed instead of silently discarded. */ + TEST_ASSERT_EQUAL_INT(0, sched_handle_notify(&sched, ¬ify)); + TEST_ASSERT_EQUAL_INT(1, pop_jstate_change(&sched, &jobid, &state)); + TEST_ASSERT_EQUAL_UINT64(99, jobid); + TEST_ASSERT_EQUAL_INT(JOB_STAT_PEND, state); + + /* No classification logic here: the notify's meaning is exclusively + * "return this job to pending" regardless of the reported failure + * code -- verify a different `code` value still resolves to PEND. */ + fixture_init(&sched); + info.jobid = 123; + info.code = 0; + jobid = 0; + state = (uint32_t)-1; + TEST_ASSERT_EQUAL_INT(0, sched_handle_notify(&sched, ¬ify)); + TEST_ASSERT_EQUAL_INT(1, pop_jstate_change(&sched, &jobid, &state)); + TEST_ASSERT_EQUAL_UINT64(123, jobid); + TEST_ASSERT_EQUAL_INT(JOB_STAT_PEND, state); +} + int main(int argc, char* argv[]) { (void)argc; @@ -210,5 +276,6 @@ int main(int argc, char* argv[]) RUN_TEST(test_handle_update_resvals_skips_empty_enqueues_nonempty, 0); RUN_TEST(test_handle_resources_skips_empty_enqueues_nonempty, 0); RUN_TEST(test_handle_resvals_full_skips_empty_enqueues_nonempty, 0); + RUN_TEST(test_handle_notify_job_dispatch_fail_enqueues_pend, 0); return UNITY_END(); }