fix(controller): persist a failed job's finish_time through WAL_JOB_FAIL (bug-1747)

The FAIL half of bug-1747: apply_job_fail_transition() stamps
finish_time = time(NULL) at APPLY time, and WAL_JOB_FAIL carried no time,
so a failed job's finish_time changed on every restart replay / standby
catch-up, and a worker-reported failure found on rejoin could never keep
the worker's own time.

- WAL_JOB_FAIL's trailer grows to int32_t code + uint64_t finish_time
  (seconds). Decoded by length: 16 B (pre-bug-1754), 20 B (code only,
  bug-1754) and 28 B (code + time) all apply as before for what they carry.
- record_job_fail_fact_at(c, job, failinfo, finish_time_ms), the fail-side
  sibling of record_job_finish_fact_at(): 0 = receipt time. on_job_fail()
  uses receipt time; resync's finishedFail branch passes the worker-
  reported finish_time_ms (Req 4.2 now holds for failures too).
- Both apply seams adopt the wire time after the transition.

4 new tests in test_wal_job_fail_fact.c (3 RED before the fix) and the
resync round-trip now asserts the worker-reported failure time (RED
before). Controller suite 1232/0. Factstream HA soak (2 rounds, 60 jobs)
PASS.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-09-26 09:33:55 +00:00
parent 21e72726f0
commit 1d6c04c28a
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
9 changed files with 349 additions and 68 deletions

View file

@ -2787,6 +2787,34 @@ int record_job_finish_fact_at(cluster_t* c, job_t* job,
return 0;
}
/* bug-1747: see cluster.h. The durable-append body record_job_fail_fact()
* used to hold, plus the failure time: `finish_time_ms` (MILLISECONDS, as
* the worker reports it -- see record_job_finish_fact_at) or, when 0,
* receipt time. It is written as SECONDS into WAL_JOB_FAIL's trailer so
* every apply of the fact -- live, restart replay, standby catch-up --
* yields the same job.finish_time instead of its own time(NULL). */
int record_job_fail_fact_at(cluster_t* c, job_t* job,
job_failinfo_t* failinfo, uint64_t finish_time_ms)
{
uint64_t finish_time = (finish_time_ms != 0)
? finish_time_ms / 1000
: (uint64_t)time(NULL);
xce_run_hooks(&(c->hooksys), XCE_CONTROLLER_HOOK_JOB, HOOK_BEFORE_EVENT,
HOOK_DETAIL_JOB_FAIL, c, failinfo, job);
/* Durable append only -- no mutation of the real `job`, no AFTER_EVENT
* firing here (Req 5.1, 5.3); apply_committed_entry() routes the
* STAGE_FLAG'd fact to apply_job_fail_transition() at apply time.
* bug-1754: failinfo->code rides along so apply time can record the
* job's exit code (apply_job_fail_exit_code). */
(void)persist_submit_job_fail_fact(&c->persist, job->job.jobid,
failinfo->stage, failinfo->code,
finish_time);
return 0;
}
int on_job_finish(cluster_t* c, xce_jobfinish_t* finish)
{
job_t* job = NULL;
@ -2916,21 +2944,7 @@ static int mark_job_fail(cluster_t* c, job_t* job, job_failinfo_t* failinfo)
static int record_job_fail_fact(cluster_t* c, job_t* job,
job_failinfo_t* failinfo)
{
xce_run_hooks(&(c->hooksys), XCE_CONTROLLER_HOOK_JOB, HOOK_BEFORE_EVENT,
HOOK_DETAIL_JOB_FAIL, c, failinfo, job);
/* Durable append only — no mutation of the real `job`, no AFTER_EVENT
* firing here (Req 5.1, 5.3). `state` is unused for this apply-time-
* recomputed shape (0 is written for forward-compatibility/diagnostics
* only); the WAL_JOB_FAIL_STAGE_FLAG bit in `_pad` tells
* apply_committed_entry() to route it to the real
* apply_job_fail_transition() core instead of an absolute overwrite.
* bug-1754: failinfo->code rides along as a trailer so apply time can
* record the job's exit code (apply_job_fail_exit_code). */
(void)persist_submit_job_fail_fact(&c->persist, job->job.jobid,
failinfo->stage, failinfo->code);
return 0;
return record_job_fail_fact_at(c, job, failinfo, 0);
}
/* Guarded state-transition core for an UPDATE, extracted from