From 7ea117dafa43575ca92ff2a05c2ee6b7f11acc74 Mon Sep 17 00:00:00 2001 From: "lei.chen" Date: Tue, 25 Aug 2026 12:53:20 +0000 Subject: [PATCH] merge-fix: port our scheduler/worker/apiserver fixes onto scheduler-framework merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After merging origin/scheduler-framework (scheduler-core), re-apply and reconcile our fixes that Rongsong's branch lacked or implemented differently: - handler.c: keep the code<0 dispatch-fail check (bug-1512: a code-0 success ack must NOT re-PEND, or dispatched jobs spawn duplicates). Use sched_job_state_change (Rongsong's enqueue mechanism) instead of sched_requeue_pend_job so the merged test framework (pop_jstate_change) stays valid. Drop Rongsong's duplicate unconditional re-PEND case. - scheduler.c: port sched_requeue_pend_job (bug-1494 class) adapted to sched_get_job(&sched->core, ...) signature. - types.h: re-add xce_sched_queue_t.dispatchwindow (our time-window PEND reason field) dropped when taking Rongsong's types.h. - models/jobs.go: re-add JobSubmitResponse.JobIDHuman (human job id). - worker controller_connector.c: our bug-1509 resend_job_status on rejoin success was already ported; add test stub. - tests: adapt dispatch-fail test (code=0 → no re-PEND); fix sched_get_job signature in white-box stubs. Build passes (nix build .#xce). Unit + live-cluster verification pending. Co-Authored-By: Claude --- src/frontend/apiserver/models/jobs.go | 1 + src/scheduler/include/sched/types.h | 5 +++ src/scheduler/include/scheduler.h | 3 ++ src/scheduler/src/handler.c | 27 ++++----------- src/scheduler/src/scheduler.c | 34 +++++++++++++++++++ src/scheduler/testsuite/test_handlers.c | 12 +++---- .../testsuite/test_job_array_expand.c | 2 +- .../testsuite/test_job_array_wire_e2e.c | 2 +- .../testsuite/test_controller_connector.c | 8 +++++ 9 files changed, 65 insertions(+), 29 deletions(-) diff --git a/src/frontend/apiserver/models/jobs.go b/src/frontend/apiserver/models/jobs.go index a2e3f902..050d2e94 100644 --- a/src/frontend/apiserver/models/jobs.go +++ b/src/frontend/apiserver/models/jobs.go @@ -703,6 +703,7 @@ type ShellLimit struct { type JobSubmitResponse struct { JobID int64 `json:"jobid"` + JobIDHuman string `json:"job_id_human"` // cluster-apiserver-timestamp-seq, for xce_test/xcecli cross-reference QueueName string `json:"queue_name"` Message string `json:"message"` } diff --git a/src/scheduler/include/sched/types.h b/src/scheduler/include/sched/types.h index 7f793603..f57ca46e 100644 --- a/src/scheduler/include/sched/types.h +++ b/src/scheduler/include/sched/types.h @@ -80,6 +80,11 @@ typedef struct { int32_t accept_interval; int32_t sched_delay; char* resreq; + /* Time-window dispatch config (PEND-reason #17/E11): when non-NULL, jobs + * in this queue may only be dispatched inside the window; outside it the + * job carries a time-window PEND reason. Owned by the queue, freed in + * kill_sched_queue (schedobj.c). */ + xce_timewindow_config_t* dispatchwindow; /* Req 2 AC 3 (scheduler-core spec, task 1.3): owning project name, * establishing the queue->project relationship. Scaffolding only -- * xce_queue_t (the raw config struct) has no project field yet, so diff --git a/src/scheduler/include/scheduler.h b/src/scheduler/include/scheduler.h index cec42f48..7022253c 100644 --- a/src/scheduler/include/scheduler.h +++ b/src/scheduler/include/scheduler.h @@ -271,6 +271,9 @@ extern int sched_apply_job_rusage(xce_scheduler_t*, xce_job_rusages_t*); extern int sched_commit_decision(xce_scheduler_t*, xce_decision_t*); extern int sched_publish_pend_reason(xce_scheduler_t*, xce_preason_set_t*); extern int sched_publish_dispatch(xce_scheduler_t*, xce_job_dispatch_t*); +/* return a job to PEND after a real dispatch failure (bug-1494 class); + * free dispatch bookkeeping and re-enqueue for re-election. */ +extern int sched_requeue_pend_job(xce_scheduler_t*, uint64_t jobid); extern void sched_worker_release_host_ctx(xce_sched_worker_t*); extern int sched_meet_host_selection(xce_scheduler_t*, xce_sched_worker_t*, xce_select_t*); diff --git a/src/scheduler/src/handler.c b/src/scheduler/src/handler.c index 7598432f..ee2cf973 100644 --- a/src/scheduler/src/handler.c +++ b/src/scheduler/src/handler.c @@ -515,24 +515,6 @@ 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); @@ -555,8 +537,11 @@ int sched_handle_notify(xce_scheduler_t* sched, xce_notify_data_t* notify) * on the success ack returns every dispatched job to the PEND set, * so the next session re-elects and re-dispatches it while the * original instance keeps running, spawning duplicate instances - * (multi-worker over-dispatch). On success the job is already RUN - * (sched_commit_decision marks it at commit); just consume the ack. */ + * (multi-worker over-dispatch, bug-1512). On success the job is + * already RUN (committed at dispatch); just consume the ack. + * Returns the job to PEND via sched_job_state_change (the same + * enqueue path schedExitJob/schedFailJob use); core_events.c's + * free-on-non-RUN branch clears the stale accounts/allocs. */ job_failinfo_t* fail = notify->data.dispatchfail; char jobidString[128]; @@ -575,7 +560,7 @@ int sched_handle_notify(xce_scheduler_t* sched, xce_notify_data_t* notify) fmt_jobid(jobidString, sizeof(jobidString), fail->jobid); WARN("%s: job %s dispatch failed (code %d) — returning to PEND", __func__, jobidString, fail->code); - rc = sched_requeue_pend_job(sched, fail->jobid); + rc = sched_job_state_change(sched, fail->jobid, JOB_STAT_PEND); if (rc >= 0) notify->data.dispatchfail = NULL; break; diff --git a/src/scheduler/src/scheduler.c b/src/scheduler/src/scheduler.c index 6f41d05c..e442c2d8 100644 --- a/src/scheduler/src/scheduler.c +++ b/src/scheduler/src/scheduler.c @@ -986,6 +986,40 @@ static int sched_job_state_change(xce_scheduler_t* sched, uint64_t jobid, return rc; } + +/* + * sched_requeue_pend_job — return a job to PEND after its dispatch failed at + * the controller (jobDispatchFail notify). The dispatch decision set the + * job's state to RUN and attached allocs/accounts; the controller rejected + * the dispatch (e.g. the chosen worker went UNAVAIL/down between the + * scheduler's election and the controller's dispatch_a_job_ check), and + * without this the job stays RUN in the scheduler's view forever — it is + * never re-elected and is stuck PEND cluster-wide even after the worker + * recovers (scheduler-side of bug-1494 class). Free the dispatch + * bookkeeping, reset to PEND, and re-enqueue so the next election + * re-dispatches it. + */ +int sched_requeue_pend_job(xce_scheduler_t* sched, uint64_t jobid) +{ + xce_sched_job_t* job = sched_get_job(&sched->core, jobid); + + if (job == NULL) { + return -ENOENT; + } + + if (job->accounts != NULL) { + free_job_account(job->accounts); + job->accounts = NULL; + } + if (job->allocs != NULL) { + free_JobAlloc_ptr(&(job->allocs)); + } + job->state = JOB_STAT_PEND; + + /* fcfs_update_job (job_hook) enqueues only PEND jobs, with dedup. */ + return plugin_job_update(sched, job); +} + static int sched_update_workers(xce_scheduler_t* sched, int nworkers, xce_worker_t** workers) { diff --git a/src/scheduler/testsuite/test_handlers.c b/src/scheduler/testsuite/test_handlers.c index 3beb2757..7e60be42 100644 --- a/src/scheduler/testsuite/test_handlers.c +++ b/src/scheduler/testsuite/test_handlers.c @@ -251,18 +251,18 @@ static void test_handle_notify_job_dispatch_fail_enqueues_pend(void) 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. */ + /* A code of 0 is the controller's SUCCESS ack (the job was dispatched + * and is already RUN, committed at dispatch) — it must NOT be returned + * to PEND (bug-1512: re-PENDing the success ack re-elects and + * re-dispatches a job whose original instance keeps running, spawning + * duplicate instances). The ack is consumed as a no-op. */ 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); + TEST_ASSERT_EQUAL_INT(0, pop_jstate_change(&sched, &jobid, &state)); } int main(int argc, char* argv[]) diff --git a/src/scheduler/testsuite/test_job_array_expand.c b/src/scheduler/testsuite/test_job_array_expand.c index 503237f0..8bfbce95 100644 --- a/src/scheduler/testsuite/test_job_array_expand.c +++ b/src/scheduler/testsuite/test_job_array_expand.c @@ -75,7 +75,7 @@ xce_sched_job_t* sched_get_job(xce_sched_core_t* core, uint64_t jobid) * case), backed by this TU's real kvmgr sched_get_job. */ int sched_requeue_pend_job(xce_scheduler_t* sched, uint64_t jobid) { - xce_sched_job_t* job = sched_get_job(sched, jobid); + xce_sched_job_t* job = sched_get_job(&sched->core, jobid); if (job == NULL) { return -ENOENT; } diff --git a/src/scheduler/testsuite/test_job_array_wire_e2e.c b/src/scheduler/testsuite/test_job_array_wire_e2e.c index 4fcb9c54..13132692 100644 --- a/src/scheduler/testsuite/test_job_array_wire_e2e.c +++ b/src/scheduler/testsuite/test_job_array_wire_e2e.c @@ -124,7 +124,7 @@ xce_sched_job_t* sched_get_job(xce_sched_core_t* core, uint64_t jobid) * case), backed by this TU's real kvmgr sched_get_job. */ int sched_requeue_pend_job(xce_scheduler_t* sched, uint64_t jobid) { - xce_sched_job_t* job = sched_get_job(sched, jobid); + xce_sched_job_t* job = sched_get_job(&sched->core, jobid); if (job == NULL) { return -ENOENT; } diff --git a/src/worker/testsuite/test_controller_connector.c b/src/worker/testsuite/test_controller_connector.c index 7f009d8b..cb567388 100644 --- a/src/worker/testsuite/test_controller_connector.c +++ b/src/worker/testsuite/test_controller_connector.c @@ -134,6 +134,14 @@ bool ipc_master_send_job_ckpt(uint64_t job_id, int kill, const char* method) void xcecd_process_command(void* arg) { (void)arg; } +/* bug-1509 resend on rejoin (checking_connection's rejoin success path) — + * this test exercises rejoin, not the resend; a stub that reports success. */ +bool resend_job_status(xcecd_worker_t* worker_list) +{ + (void)worker_list; + return true; +} + /* ════════════════════════════════════════════════════════════════════════ * Fixtures * ════════════════════════════════════════════════════════════════════════ */