feat(scheduler-dispatch-commit): controller resolves retryable dispatch failures with bounded escalation (task 3.3)
The four retryable-classified failure sites in dispatch_a_job_() (worker lookup, worker null, worker not ready, RPC failure) now resolve instead of just returning an error code: lock(j, RTTI_LOCK_WRITE) -> genuinely classify via xce_classify_dispatch_fail() -> increment dispatch_fail_count -> within XCE_DISPATCH_FAIL_RETRY_BOUND (3) resolves retryable (nothing further under the lock), past it resolves terminal via mark_job_fail() exactly like task 3.2's sites -> unlock(j) -> dereference(j) (closing bug-1627's 4-site reference leak) -> outside the lock, notify_dispatch() fires only on the retryable outcome. Each site's natural error code is captured into a stable local before any intervening call, since errno isn't guaranteed stable once lock()/ERROR()/free_launchinfo() sit between the original failure and the eventual return. handle_jobdispatch() is intentionally untouched -- it still unconditionally calls notify_dispatch() itself for every outcome, so the retryable path now temporarily double-notifies until task 4.1 removes that caller-side call. Expected, not a defect. New test_dispatch_fail_retry.c (4 tests, including a bounded-escalation test driving 4 consecutive RPC failures on the same job: 3rd retries, 4th resolves EXIT) plus a fixed stale assertion in test_dispatch_commit.c and additive, read-only notify-capture accessors in test_commit_barrier_reply.c (the binary's sole notify() stub host). Independently reviewed and approved, including explicit scrutiny of the cross-file test accessor addition and the stale-assertion fix -- both confirmed legitimate, not scope creep or regression-hiding. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cxa1PmqAB5PyXfJaNvj8TN
This commit is contained in:
parent
f756c4b09f
commit
05c45ef3c8
7 changed files with 880 additions and 12 deletions
|
|
@ -23,6 +23,19 @@ static int record_job_finish_fact(cluster_t*, job_t*, xce_jobfinish_t*);
|
|||
static int record_job_fail_fact(cluster_t*, job_t*, job_failinfo_t*);
|
||||
static int mark_job_fail(cluster_t*, job_t*, job_failinfo_t*);
|
||||
static int mark_job_update(cluster_t*, job_t*, int);
|
||||
/* scheduler-dispatch-commit task 3.3: dispatch_a_job_()'s four retryable
|
||||
* failure sites (job.c, below) need to notify the scheduler of a retryable
|
||||
* failure -- notify_dispatch() is defined AFTER dispatch_a_job_() in this
|
||||
* file with no forward declaration until now. Mirrors mark_job_fail's own
|
||||
* forward declaration immediately above; body/signature unmodified. */
|
||||
static void notify_dispatch(cluster_t*, endpoint_t*, uint64_t, int);
|
||||
|
||||
/* scheduler-dispatch-commit task 3.3 (Req 4.3): default bounded-retry limit
|
||||
* for a job's consecutive retryable dispatch failures with no intervening
|
||||
* successful dispatch -- design.md's own stated default (3 consecutive
|
||||
* failures). The failure that pushes job.dispatch_fail_count past this
|
||||
* bound resolves the job to terminal instead of retrying again. */
|
||||
#define XCE_DISPATCH_FAIL_RETRY_BOUND 3
|
||||
|
||||
/* A job is terminal once it carries a DONE or EXIT bit. Terminal jobs reject
|
||||
* backward (terminal -> non-terminal) transitions and treat re-applied
|
||||
|
|
@ -1025,27 +1038,202 @@ static int dispatch_a_job_(cluster_t* c, endpoint_t* ep, xce_job_alloc_t* alloc)
|
|||
}
|
||||
unlock(c->workermgr);
|
||||
if (rc < 0) {
|
||||
/* scheduler-dispatch-commit task 3.3 (Req 4.1, 4.3, 4.4, 4.5):
|
||||
* resolves this retryable (transient/environmental) dispatch
|
||||
* failure -- closes bug-1627's leak site #1 (no dereference(j)
|
||||
* existed on this path before this task). rc already holds the
|
||||
* stable natural error code from key_find() above; no further call
|
||||
* touches it before it is returned below. Nothing was ever
|
||||
* committed to j->job.allocs on this path (task 3.1 gates that
|
||||
* commit on a successful xce_startjob_req(), never reached here),
|
||||
* so Req 4.4/4.5's "release any tentative allocation data" is
|
||||
* satisfied vacuously by construction. */
|
||||
xce_dispatch_fail_class_t dfclass;
|
||||
job_failinfo_t failinfo = { .jobid = alloc->jobid };
|
||||
bool resolved_retryable = false;
|
||||
uint32_t failcount;
|
||||
|
||||
lock(j, RTTI_LOCK_WRITE);
|
||||
|
||||
dfclass = xce_classify_dispatch_fail(
|
||||
XCE_DISPATCH_FAIL_SITE_WORKER_LOOKUP);
|
||||
switch (dfclass) {
|
||||
case XCE_DISPATCH_FAIL_RETRYABLE:
|
||||
failcount = ++j->dispatch_fail_count;
|
||||
if (failcount <= XCE_DISPATCH_FAIL_RETRY_BOUND) {
|
||||
resolved_retryable = true;
|
||||
} else {
|
||||
failinfo.code = rc;
|
||||
failinfo.detail = "dispatch failed: worker lookup failed "
|
||||
"repeatedly, retry bound exceeded";
|
||||
mark_job_fail(c, j, &failinfo);
|
||||
}
|
||||
break;
|
||||
case XCE_DISPATCH_FAIL_TERMINAL:
|
||||
default:
|
||||
/* Impossible per today's classification table (confirmed:
|
||||
* xce_classify_dispatch_fail() returns RETRYABLE for this site
|
||||
* unconditionally) -- fail loud and fall through to the same
|
||||
* terminal resolution as a safe fallback, mirroring
|
||||
* xce_classify_dispatch_fail()'s own default: convention. */
|
||||
ERROR("dispatch_a_job_: XCE_DISPATCH_FAIL_SITE_WORKER_LOOKUP "
|
||||
"classified TERMINAL (%d) -- impossible per the current "
|
||||
"classification table; resolving terminal as a safe "
|
||||
"fallback", (int)dfclass);
|
||||
#ifdef XCE_TESTING
|
||||
assert(0
|
||||
&& "XCE_DISPATCH_FAIL_SITE_WORKER_LOOKUP classified "
|
||||
"terminal -- update the classification table or "
|
||||
"this call site");
|
||||
#endif
|
||||
failinfo.code = rc;
|
||||
failinfo.detail = "dispatch failed: worker lookup failed";
|
||||
mark_job_fail(c, j, &failinfo);
|
||||
break;
|
||||
}
|
||||
|
||||
unlock(j);
|
||||
|
||||
dereference(j);
|
||||
|
||||
if (resolved_retryable) {
|
||||
notify_dispatch(c, ep, alloc->jobid, rc);
|
||||
}
|
||||
|
||||
return rc;
|
||||
}
|
||||
|
||||
if (w == NULL) {
|
||||
int dfrc;
|
||||
xce_dispatch_fail_class_t dfclass;
|
||||
job_failinfo_t failinfo = { .jobid = alloc->jobid };
|
||||
bool resolved_retryable = false;
|
||||
uint32_t failcount;
|
||||
|
||||
/* scheduler-dispatch-commit task 3.3: capture errno FIRST, before
|
||||
* any other call (free_launchinfo() included) -- errno is not
|
||||
* guaranteed stable across subsequent calls. */
|
||||
dfrc = -errno;
|
||||
|
||||
free_launchinfo(&launchinfo);
|
||||
return -errno;
|
||||
|
||||
/* Req 4.1, 4.3, 4.4, 4.5: resolves this retryable (transient/
|
||||
* environmental) dispatch failure -- closes bug-1627's leak site
|
||||
* #2 (no dereference(j) existed on this path before this task). */
|
||||
lock(j, RTTI_LOCK_WRITE);
|
||||
|
||||
dfclass = xce_classify_dispatch_fail(
|
||||
XCE_DISPATCH_FAIL_SITE_WORKER_NULL);
|
||||
switch (dfclass) {
|
||||
case XCE_DISPATCH_FAIL_RETRYABLE:
|
||||
failcount = ++j->dispatch_fail_count;
|
||||
if (failcount <= XCE_DISPATCH_FAIL_RETRY_BOUND) {
|
||||
resolved_retryable = true;
|
||||
} else {
|
||||
failinfo.code = dfrc;
|
||||
failinfo.detail = "dispatch failed: worker not found "
|
||||
"repeatedly, retry bound exceeded";
|
||||
mark_job_fail(c, j, &failinfo);
|
||||
}
|
||||
break;
|
||||
case XCE_DISPATCH_FAIL_TERMINAL:
|
||||
default:
|
||||
ERROR("dispatch_a_job_: XCE_DISPATCH_FAIL_SITE_WORKER_NULL "
|
||||
"classified TERMINAL (%d) -- impossible per the current "
|
||||
"classification table; resolving terminal as a safe "
|
||||
"fallback", (int)dfclass);
|
||||
#ifdef XCE_TESTING
|
||||
assert(0
|
||||
&& "XCE_DISPATCH_FAIL_SITE_WORKER_NULL classified "
|
||||
"terminal -- update the classification table or "
|
||||
"this call site");
|
||||
#endif
|
||||
failinfo.code = dfrc;
|
||||
failinfo.detail = "dispatch failed: worker not found";
|
||||
mark_job_fail(c, j, &failinfo);
|
||||
break;
|
||||
}
|
||||
|
||||
unlock(j);
|
||||
|
||||
dereference(j);
|
||||
|
||||
if (resolved_retryable) {
|
||||
notify_dispatch(c, ep, alloc->jobid, dfrc);
|
||||
}
|
||||
|
||||
return dfrc;
|
||||
}
|
||||
|
||||
lock(w, RTTI_LOCK_READ);
|
||||
if ((w->ep == NULL) || (w->host.status != 0)) {
|
||||
int dfrc;
|
||||
xce_dispatch_fail_class_t dfclass;
|
||||
job_failinfo_t failinfo = { .jobid = alloc->jobid };
|
||||
bool resolved_retryable = false;
|
||||
uint32_t failcount;
|
||||
|
||||
ERROR("worker %s is not in correct state, status %x ep %p",
|
||||
w->host.name, w->host.status, w->ep);
|
||||
free_launchinfo(&launchinfo);
|
||||
|
||||
SET_ERROR(E_XCE_BAD_WORKER);
|
||||
/* scheduler-dispatch-commit task 3.3: capture errno immediately
|
||||
* after SET_ERROR(), before any further call -- errno is not
|
||||
* guaranteed stable across unlock(w)/dereference(w)/lock(j)/etc. */
|
||||
dfrc = -errno;
|
||||
|
||||
unlock(w);
|
||||
|
||||
dereference(w);
|
||||
|
||||
return -errno;
|
||||
/* Req 4.1, 4.3, 4.4, 4.5: resolves this retryable (transient/
|
||||
* environmental) dispatch failure -- closes bug-1627's leak site
|
||||
* #3 (no dereference(j) existed on this path before this task). */
|
||||
lock(j, RTTI_LOCK_WRITE);
|
||||
|
||||
dfclass = xce_classify_dispatch_fail(
|
||||
XCE_DISPATCH_FAIL_SITE_WORKER_NOT_READY);
|
||||
switch (dfclass) {
|
||||
case XCE_DISPATCH_FAIL_RETRYABLE:
|
||||
failcount = ++j->dispatch_fail_count;
|
||||
if (failcount <= XCE_DISPATCH_FAIL_RETRY_BOUND) {
|
||||
resolved_retryable = true;
|
||||
} else {
|
||||
failinfo.code = dfrc;
|
||||
failinfo.detail = "dispatch failed: worker not ready "
|
||||
"repeatedly, retry bound exceeded";
|
||||
mark_job_fail(c, j, &failinfo);
|
||||
}
|
||||
break;
|
||||
case XCE_DISPATCH_FAIL_TERMINAL:
|
||||
default:
|
||||
ERROR("dispatch_a_job_: "
|
||||
"XCE_DISPATCH_FAIL_SITE_WORKER_NOT_READY classified "
|
||||
"TERMINAL (%d) -- impossible per the current "
|
||||
"classification table; resolving terminal as a safe "
|
||||
"fallback", (int)dfclass);
|
||||
#ifdef XCE_TESTING
|
||||
assert(0
|
||||
&& "XCE_DISPATCH_FAIL_SITE_WORKER_NOT_READY classified "
|
||||
"terminal -- update the classification table or "
|
||||
"this call site");
|
||||
#endif
|
||||
failinfo.code = dfrc;
|
||||
failinfo.detail = "dispatch failed: worker not ready";
|
||||
mark_job_fail(c, j, &failinfo);
|
||||
break;
|
||||
}
|
||||
|
||||
unlock(j);
|
||||
|
||||
dereference(j);
|
||||
|
||||
if (resolved_retryable) {
|
||||
notify_dispatch(c, ep, alloc->jobid, dfrc);
|
||||
}
|
||||
|
||||
return dfrc;
|
||||
}
|
||||
|
||||
/* FIXME: we may consider to optimize when a job has enter a queue? */
|
||||
|
|
@ -1183,8 +1371,67 @@ static int dispatch_a_job_(cluster_t* c, endpoint_t* ep, xce_job_alloc_t* alloc)
|
|||
|
||||
unlock(j);
|
||||
} else {
|
||||
xce_dispatch_fail_class_t dfclass;
|
||||
job_failinfo_t failinfo = { .jobid = alloc->jobid };
|
||||
bool resolved_retryable = false;
|
||||
uint32_t failcount;
|
||||
|
||||
SET_ERROR(-rc);
|
||||
PERROR("xce_startjob_req");
|
||||
|
||||
/* scheduler-dispatch-commit task 3.3 (Req 4.1, 4.3, 4.4, 4.5):
|
||||
* resolves this retryable (transient/environmental) dispatch
|
||||
* failure -- closes bug-1627's leak site #4 (no dereference(j)
|
||||
* existed on this path before this task). rc already holds the
|
||||
* stable, captured return of xce_startjob_req() -- no call between
|
||||
* that assignment and here has touched it. */
|
||||
lock(j, RTTI_LOCK_WRITE);
|
||||
|
||||
dfclass = xce_classify_dispatch_fail(
|
||||
XCE_DISPATCH_FAIL_SITE_RPC_FAILURE);
|
||||
switch (dfclass) {
|
||||
case XCE_DISPATCH_FAIL_RETRYABLE:
|
||||
failcount = ++j->dispatch_fail_count;
|
||||
if (failcount <= XCE_DISPATCH_FAIL_RETRY_BOUND) {
|
||||
resolved_retryable = true;
|
||||
} else {
|
||||
failinfo.code = rc;
|
||||
failinfo.detail = "dispatch failed: xce_startjob_req "
|
||||
"failed repeatedly, retry bound "
|
||||
"exceeded";
|
||||
mark_job_fail(c, j, &failinfo);
|
||||
}
|
||||
break;
|
||||
case XCE_DISPATCH_FAIL_TERMINAL:
|
||||
default:
|
||||
/* Impossible per today's classification table (confirmed:
|
||||
* xce_classify_dispatch_fail() returns RETRYABLE for this site
|
||||
* unconditionally) -- fail loud and fall through to the same
|
||||
* terminal resolution as a safe fallback, mirroring
|
||||
* xce_classify_dispatch_fail()'s own default: convention. */
|
||||
ERROR("dispatch_a_job_: XCE_DISPATCH_FAIL_SITE_RPC_FAILURE "
|
||||
"classified TERMINAL (%d) -- impossible per the current "
|
||||
"classification table; resolving terminal as a safe "
|
||||
"fallback", (int)dfclass);
|
||||
#ifdef XCE_TESTING
|
||||
assert(0
|
||||
&& "XCE_DISPATCH_FAIL_SITE_RPC_FAILURE classified "
|
||||
"terminal -- update the classification table or "
|
||||
"this call site");
|
||||
#endif
|
||||
failinfo.code = rc;
|
||||
failinfo.detail = "dispatch failed: xce_startjob_req failed";
|
||||
mark_job_fail(c, j, &failinfo);
|
||||
break;
|
||||
}
|
||||
|
||||
unlock(j);
|
||||
|
||||
dereference(j);
|
||||
|
||||
if (resolved_retryable) {
|
||||
notify_dispatch(c, ep, alloc->jobid, rc);
|
||||
}
|
||||
}
|
||||
|
||||
return rc;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue