feat(worker-rejoin-resync): fail-fast disconnected-worker dispatch (task 2.3)
Split dispatch_a_job_()'s combined "(w->ep == NULL) || (w->host.status != 0)" check into two branches, checked in order: w->ep == NULL now resolves directly via record_job_reconcile_pending_fact() (task 2.1) -- no dispatch_fail_count increment, no mark_job_fail(), never reaching xce_classify_dispatch_fail() at all -- before the code ever tests w->host.status != 0, which keeps its existing retry-then-terminal-fail behavior completely unchanged (byte-identical body, confirmed against HEAD). Completes Task 2 (all controller job-table reconciliation outcomes: 2.1 reconcile-to-pending, 2.2 reconcile-assign, 2.3 this split). RED phase caught a genuine pre-existing test bug: test_worker_notready_ resolves_retryable was seeding a NULL-ep worker while claiming to test the host.status!=0 retry path -- it never actually exercised that path. Corrected to use a connected-but-busy worker, with self-asserting TEST_ASSERT_NOT_NULL(w->ep)/TEST_ASSERT_TRUE(status!=0) guards so this can't silently drift again. 2 new tests, including a full JOB_TRANSITION_APPLIED path proof (WAL LSN advance, hook-fires-exactly-once per bug-1679, allocs released per bug-1631) through this exact call site, both using correctly-timed canary references on both the job and worker objects to catch any over-release under ASAN. Full controller suite: 1227 tests, 0 failures, zero regressions across every other dispatch_a_job_ failure site. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
This commit is contained in:
parent
ecfb4c942f
commit
ea0658ecf4
3 changed files with 390 additions and 8 deletions
|
|
@ -1510,7 +1510,66 @@ static int dispatch_a_job_(cluster_t* c, endpoint_t* ep, xce_job_alloc_t* alloc)
|
|||
}
|
||||
|
||||
lock(w, RTTI_LOCK_READ);
|
||||
if ((w->ep == NULL) || (w->host.status != 0)) {
|
||||
if (w->ep == NULL) {
|
||||
/* worker-rejoin-resync task 2.3 (Req 5.1, 5.4): a dispatch attempt
|
||||
* to a worker the controller is not currently connected to (w->ep
|
||||
* == NULL) is resolved here, BEFORE the w->host.status != 0 check
|
||||
* below ever reaches xce_classify_dispatch_fail() at all --
|
||||
* deliberately a bypass of that retry-then-terminal-fail machinery,
|
||||
* not a third value added to xce_dispatch_fail_class_t (design.md's
|
||||
* "Controller: job.c" Components section: that enum's whole
|
||||
* contract is a two-way TERMINAL/RETRYABLE choice every existing
|
||||
* call site's switch already exhaustively handles). Retrying the
|
||||
* dispatch itself doesn't fit "the worker isn't even connected,"
|
||||
* and this case must stay recoverable-via-reschedule rather than
|
||||
* ever resolve to mark_job_fail()'s terminal outcome -- so this
|
||||
* branch calls neither xce_classify_dispatch_fail() nor
|
||||
* mark_job_fail(), and never touches j->dispatch_fail_count. */
|
||||
int dfrc;
|
||||
int result;
|
||||
|
||||
ERROR("worker %s is not connected (ep NULL), status %x",
|
||||
w->host.name, w->host.status);
|
||||
free_launchinfo(&launchinfo);
|
||||
|
||||
SET_ERROR(E_XCE_BAD_WORKER);
|
||||
/* Capture errno immediately after SET_ERROR(), before any further
|
||||
* call -- errno is not guaranteed stable across unlock(w)/
|
||||
* dereference(w)/lock(j)/etc., mirroring the WORKER_NOT_READY
|
||||
* branch's own discipline below. */
|
||||
dfrc = -errno;
|
||||
|
||||
unlock(w);
|
||||
|
||||
dereference(w);
|
||||
|
||||
/* Req 5.4: resolves via record_job_reconcile_pending_fact() (task
|
||||
* 2.1) directly -- eager mutation + allocs release (bug-1631) under
|
||||
* this write lock, durable append inside that call, then (only on
|
||||
* an actual APPLIED transition) this caller's own post-unlock
|
||||
* persist_notify_job_updated_already_persisted() call, exactly as
|
||||
* that function's own contract requires (cluster.h doc comment).
|
||||
* Closes the same bug-1627 reference-leak class as the WORKER_NOT_
|
||||
* READY branch below -- j's reference (held since the jobmgr lookup
|
||||
* at the top of this function) is released via dereference(j) on
|
||||
* every exit from this branch. */
|
||||
lock(j, RTTI_LOCK_WRITE);
|
||||
|
||||
result = record_job_reconcile_pending_fact(c, j);
|
||||
|
||||
unlock(j);
|
||||
|
||||
if (result == (int)JOB_TRANSITION_APPLIED) {
|
||||
persist_notify_job_updated_already_persisted(c, j);
|
||||
}
|
||||
/* JOB_TRANSITION_REJECTED_BACKWARD/NOOP_DUPLICATE: nothing changed
|
||||
* (job already terminal, or already PEND) -- no notify needed,
|
||||
* matching record_job_reconcile_pending_fact's own contract. */
|
||||
|
||||
dereference(j);
|
||||
|
||||
return dfrc;
|
||||
} else if (w->host.status != 0) {
|
||||
int dfrc;
|
||||
xce_dispatch_fail_class_t dfclass;
|
||||
job_failinfo_t failinfo = { .jobid = alloc->jobid };
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue