diff --git a/src/controller/persist.c b/src/controller/persist.c index b4ae59a0..33bab9bf 100644 --- a/src/controller/persist.c +++ b/src/controller/persist.c @@ -576,7 +576,23 @@ static void* fsync_worker(void* arg) * pending but made no progress (after == before) still waits out the * normal wal_sync_ms cadence before retrying — same bounded-latency * budget the idle path already uses, no new tunable needed. */ - if (pending && after == before && + /* Re-evaluate the level AFTER the flush, not with the pre-flush + * `pending` snapshot: `after == before` alone cannot distinguish a + * genuine sustained fdatasync failure (pending work that made no + * progress — the spin this block exists to throttle) from a flush that + * was a no-op because another path advanced durable_lsn to the tip + * mid-flush (e.g. persist_stop's final sync, or a concurrent + * persist_wal_sync_append). In the no-op case durable is caught up, so + * there is nothing to retry and waiting wal_sync_ms is pure delay — + * and that spurious delay sits directly in the fsync_worker's + * teardown window, where it races persist_stop()/commit_engine_fini() + * (bug-1515: p2_integration hang). Only wait when the level is STILL + * pending after the flush (real failure). */ + bool pending_after; + pthread_mutex_lock(&p->lock); + pending_after = flusher_work_pending_locked(p); + pthread_mutex_unlock(&p->lock); + if (pending_after && after == before && !atomic_load_explicit(&p->fsync_stop, memory_order_acquire)) { long ms = (long)p->config.wal_sync_ms; struct timespec dl; @@ -5668,6 +5684,15 @@ int persist_recover(persist_t* p) * own walk drains fully from last_applied_lsn+1 through the current * committed_lsn, so this is a complete, ordered catch-up, not a partial * one; a harmless no-op if nothing queued. */ + + /* bug-1509 B3: after restart, RUN jobs' state is uncertain (worker + * connectivity unknown) — mark them UNKWN+TRANSIENT in-memory only. + * No WAL append: persist_recover runs before the controller knows it + * will be primary, and a standby must never append. Idempotent. */ + if (p->cluster) { + mark_all_run_jobs_unknown_in_memory(p->cluster); + } + atomic_store_explicit(&p->recovery_complete, true, memory_order_release); /* n_vcpus==0 is this codebase's own established "not yet initialized" * sentinel for vtaskmgr_t (mirrors cluster_init_phase2's own diff --git a/src/controller/testsuite/test_dispatch_commit.c b/src/controller/testsuite/test_dispatch_commit.c index c60dc691..e50ffd61 100644 --- a/src/controller/testsuite/test_dispatch_commit.c +++ b/src/controller/testsuite/test_dispatch_commit.c @@ -679,10 +679,13 @@ static void test_dc_success_appends_wal_job_dispatch_durably(void) unlock(c2.jobmgr); TEST_ASSERT_NOT_NULL_MESSAGE(j2, "restart replay must reconstruct the job via WAL_JOB_SUBMIT"); - TEST_ASSERT_EQUAL_UINT32_MESSAGE(JOB_STAT_RUN, j2->job.state, + TEST_ASSERT_TRUE_MESSAGE(JOB_IS_RUN(j2->job.state), "restart replay must recover the job as RUNNING from the " "WAL_JOB_DISPATCH fact appended by dispatch_a_job_()'s real " "success path -- no WAL_JOB_START anywhere in this log"); + TEST_ASSERT_TRUE_MESSAGE((j2->job.state & JOB_STAT_UNKWN) != 0, + "bug-1509 B3: a RUN job recovered on restart is marked UNKWN " + "(in-memory) awaiting worker reconnect/resend reconciliation"); /* bug-1678: restart replay must recover not just STATE but the * resource allocation itself, or EXEC_HOST is permanently blank for diff --git a/src/controller/testsuite/test_wal_job_dispatch_fact.c b/src/controller/testsuite/test_wal_job_dispatch_fact.c index 6c9819db..a2a55185 100644 --- a/src/controller/testsuite/test_wal_job_dispatch_fact.c +++ b/src/controller/testsuite/test_wal_job_dispatch_fact.c @@ -434,10 +434,14 @@ test_wjd_restart_replay_recovers_running_without_start_fact(void) job_t* j = wjd_find_job(71003); TEST_ASSERT_NOT_NULL_MESSAGE(j, "restart replay must construct the job via WAL_JOB_SUBMIT"); - TEST_ASSERT_EQUAL_UINT32_MESSAGE(JOB_STAT_RUN, j->job.state, + TEST_ASSERT_TRUE_MESSAGE(JOB_IS_RUN(j->job.state), "restart replay must recover the job as RUNNING from the " "WAL_JOB_DISPATCH fact alone, with no WAL_JOB_START present " "in the log at all"); + TEST_ASSERT_TRUE_MESSAGE((j->job.state & JOB_STAT_UNKWN) != 0, + "bug-1509 B3: a RUN job recovered on restart is marked UNKWN " + "(in-memory) awaiting the worker reconnect/resend to reconcile " + "the true state"); wjd_teardown(); wjd_teardown_persist(p2); @@ -529,7 +533,12 @@ test_wjd_restart_replay_recovers_running_and_allocs_with_dispatch_fact(void) job_t* j = wjd_find_job(71006); TEST_ASSERT_NOT_NULL_MESSAGE(j, "restart replay must construct the job via WAL_JOB_SUBMIT"); - TEST_ASSERT_EQUAL_UINT32(JOB_STAT_RUN, j->job.state); + TEST_ASSERT_TRUE_MESSAGE(JOB_IS_RUN(j->job.state), + "restart replay must recover the job as RUNNING from the " + "WAL_JOB_DISPATCH fact's state"); + TEST_ASSERT_TRUE_MESSAGE((j->job.state & JOB_STAT_UNKWN) != 0, + "bug-1509 B3: a RUN job recovered on restart is marked UNKWN " + "(in-memory) awaiting worker reconnect/resend reconciliation"); TEST_ASSERT_NOT_NULL_MESSAGE(j->job.allocs, "restart replay must recover job.allocs from the " "WAL_JOB_DISPATCH fact's trailing JobAlloc bytes (bug-1678), " diff --git a/src/frontend/apiserver/backend/clib.go b/src/frontend/apiserver/backend/clib.go index 3ae3d48f..6480df12 100644 --- a/src/frontend/apiserver/backend/clib.go +++ b/src/frontend/apiserver/backend/clib.go @@ -664,7 +664,13 @@ func SubmitJob(job *models.JobInfo, arraySize int) error { func ControlJob(jobid uint64, action int, content string) error { var ctrl C.xce_jobctrl_t ctrl.jobid = C.uint64_t(jobid) - ctrl.action = C.int(action) + // ctrl.action is the capnp JobControl union DISCRIMINANT (0=siginfo, + // 1=ckptoption), NOT the models.JOB_* constant. bug-1490: setting it to + // JOB_STOP/RESUME/KILL/CHKPNT (1/2/3/8) made the controller's job_ctrl() + // select the wrong union branch (or none) and reject every control with + // -EINVAL, so hold/release/cancel never reached the job. (Re-applied on + // the merge branch — Rongsong's clib.go had reverted it to action.) + ctrl.action = C.int(jobCtrlDiscriminant(action)) // For signal-based actions set signo; for checkpoint set ckpt options. // xce_jobctrl_t.u is a C union — we use a static C helper to set the diff --git a/src/scheduler/testsuite/meson.build b/src/scheduler/testsuite/meson.build index 755b2333..5ff72ab4 100644 --- a/src/scheduler/testsuite/meson.build +++ b/src/scheduler/testsuite/meson.build @@ -14,7 +14,11 @@ scheduler_test_src = [ scheduler_test_deps = [libxce_deps, librtti_dep, unity_dep, libxce_dep, libedit_dep] -incdir = include_directories('../include', '../../include', '.', '../src', '../../lib') +# unity.pc's Cflags only adds -I${includedir}/unity, so resolves +# but (used by every suite) does not — add the parent +# includedir as well (same fix as src/lib/meson.build). +incdir = include_directories('../include', '../../include', '.', '../src', '../../lib', + unity_dep.get_variable('includedir')) scheduler_test = executable('sched_test', scheduler_test_src, diff --git a/src/scheduler/testsuite/test_handlers.c b/src/scheduler/testsuite/test_handlers.c index 7e60be42..c4616525 100644 --- a/src/scheduler/testsuite/test_handlers.c +++ b/src/scheduler/testsuite/test_handlers.c @@ -257,10 +257,11 @@ static void test_handle_notify_job_dispatch_fail_enqueues_pend(void) * 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; + info.jobid = 123; + info.code = 0; + jobid = 0; + state = (uint32_t)-1; + notify.data.dispatchfail = &info; /* first notify NULLed it on success */ TEST_ASSERT_EQUAL_INT(0, sched_handle_notify(&sched, ¬ify)); TEST_ASSERT_EQUAL_INT(0, pop_jstate_change(&sched, &jobid, &state)); } diff --git a/src/worker/testsuite/meson.build b/src/worker/testsuite/meson.build index 449e145b..3a3a400d 100644 --- a/src/worker/testsuite/meson.build +++ b/src/worker/testsuite/meson.build @@ -6,8 +6,12 @@ # for why controller_connector.c is not also listed in xcecd_srcs here (it # would double-define every symbol in that file). +# unity.pc's Cflags only adds -I${includedir}/unity, so resolves +# but (used by every suite) does not — add the parent +# includedir as well (same fix as src/lib/meson.build). worker_test_incdir = include_directories('../include', '../../include', '.', - '../src', '../../lib') + '../src', '../../lib', + unity_dep.get_variable('includedir')) worker_test_deps = [xcecd_worker_deps, unity_dep] diff --git a/src/worker/testsuite/test_controller_connector.c b/src/worker/testsuite/test_controller_connector.c index cb567388..81d07b6c 100644 --- a/src/worker/testsuite/test_controller_connector.c +++ b/src/worker/testsuite/test_controller_connector.c @@ -424,6 +424,11 @@ static void test_checking_connection_skips_rejoin_when_not_pending(void) controller_connector_ctx_t fake_cc = { 0 }; fake_cc.running = true; cc_ctx = &fake_cc; + /* checking_connection()'s rejoin-success path reports job state via + * resend_job_status(*cc_ctx->worker_list); give the connector a valid + * (here: empty) list pointer so the resend deref is not a NULL crash. */ + xcecd_worker_t* fake_workers = NULL; + fake_cc.worker_list = (void**)&fake_workers; xcecli_ctx = NULL; /* xcecli_hello fails fast */ needs_rejoin = false; @@ -446,6 +451,11 @@ static void test_checking_connection_rejoin_failure_keeps_flag_set(void) controller_connector_ctx_t fake_cc = { 0 }; fake_cc.running = true; cc_ctx = &fake_cc; + /* checking_connection()'s rejoin-success path reports job state via + * resend_job_status(*cc_ctx->worker_list); give the connector a valid + * (here: empty) list pointer so the resend deref is not a NULL crash. */ + xcecd_worker_t* fake_workers = NULL; + fake_cc.worker_list = (void**)&fake_workers; xcecli_ctx = NULL; needs_rejoin = true; @@ -470,6 +480,11 @@ static void test_checking_connection_rejoin_success_clears_flag(void) controller_connector_ctx_t fake_cc = { 0 }; fake_cc.running = true; cc_ctx = &fake_cc; + /* checking_connection()'s rejoin-success path reports job state via + * resend_job_status(*cc_ctx->worker_list); give the connector a valid + * (here: empty) list pointer so the resend deref is not a NULL crash. */ + xcecd_worker_t* fake_workers = NULL; + fake_cc.worker_list = (void**)&fake_workers; xcecli_ctx = NULL; needs_rejoin = true; @@ -497,6 +512,11 @@ test_checking_connection_rejoin_success_with_nonzero_index_clears_flag(void) controller_connector_ctx_t fake_cc = { 0 }; fake_cc.running = true; cc_ctx = &fake_cc; + /* checking_connection()'s rejoin-success path reports job state via + * resend_job_status(*cc_ctx->worker_list); give the connector a valid + * (here: empty) list pointer so the resend deref is not a NULL crash. */ + xcecd_worker_t* fake_workers = NULL; + fake_cc.worker_list = (void**)&fake_workers; xcecli_ctx = NULL; needs_rejoin = true; @@ -530,6 +550,11 @@ test_checking_connection_rejoin_retried_on_next_probe_after_failure(void) controller_connector_ctx_t fake_cc = { 0 }; fake_cc.running = true; cc_ctx = &fake_cc; + /* checking_connection()'s rejoin-success path reports job state via + * resend_job_status(*cc_ctx->worker_list); give the connector a valid + * (here: empty) list pointer so the resend deref is not a NULL crash. */ + xcecd_worker_t* fake_workers = NULL; + fake_cc.worker_list = (void**)&fake_workers; xcecli_ctx = NULL; needs_rejoin = true;