lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(controller): persist a failed job's exit code through WAL_JOB_FAIL (bug-1754)

Browse source 
A worker reports every nonzero job exit as a FAIL (xcecd_job_finished ->
xcecd_notify_job_fail(jobid, stage, exit_code)), not a FINISH. The deferred
WAL_JOB_FAIL fact carried only {jobid, STAGE_FLAG|stage} and no apply seam
touched job.exit_info, so exit_info stayed NULL for every failed job, live
and after replay, and HOOK_DETAIL_JOB_FAIL subscribers saw code 0.

- persist_submit_job_fail_fact(): header + int32_t code trailer, used by
  record_job_fail_fact(). Old entries (no trailer) decode as before.
- apply_job_fail_exit_code(): records the code in exit_info for pre-exit
  stages only; a POST_EXEC code is the script's and leaves exit_info alone.
  Kept out of apply_job_fail_transition() so mark_job_fail()'s internal
  -errno failures (not replayed with a code) stay live/replay consistent.
- Both apply seams adopt the code. apply_wal_entry()'s WAL_JOB_FAIL case
  also stops assigning the wire state 0 for STAGE_FLAG entries and runs the
  real transition, as apply_committed_entry() already does.

New test_wal_job_fail_fact.c (5 tests, 3 RED before the fix); the two
test_worker_resync.c assertions that pinned exit_info == NULL now assert the
worker-reported code. Controller suite: 1228 tests, 0 failures.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-09-26 03:32:54 +00:00
parent 1ba643b8be
commit 6876e4f6f2
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
10 changed files with 543 additions and 43 deletions
Download patch file Download diff file Expand all files Collapse all files

6
src/controller/include/cluster.h
View file

@ -171,6 +171,12 @@ extern job_transition_result_t apply_job_finish_transition(job_t*,
xce_jobfinish_t*);
extern job_transition_result_t apply_job_fail_transition(job_t*,
job_failinfo_t*);
/* bug-1754: adopt a worker-reported FAIL's code into job.exit_info. Only a
* pre-exit (non-POST_EXEC) failure's code is the job's own exit code; a
* POST_EXEC/Q_POST_EXEC code belongs to the post-exec script and leaves
* exit_info untouched. Called by both WAL_JOB_FAIL apply seams (persist.c)
* when the fact carries a code; caller holds `job` under RTTI_LOCK_WRITE. */
extern void apply_job_fail_exit_code(job_t*, const job_failinfo_t*);
extern job_transition_result_t apply_job_update_transition(job_t*, int);
/* Guarded state-transition core for a postdone fact (raft-apply-decoupling

27
src/controller/job.c
View file

@ -2852,6 +2852,23 @@ job_transition_result_t apply_job_fail_transition(job_t* job,
return JOB_TRANSITION_APPLIED;
}
/* bug-1754: see cluster.h. Kept out of apply_job_fail_transition() on
* purpose -- that core is also driven by mark_job_fail()'s controller-
* internal failures, whose code is a -errno and whose WAL fact (_pad == 0,
* absolute state) never carries it, so recording it there would make the
* live job disagree with its replay. */
void apply_job_fail_exit_code(job_t* job, const job_failinfo_t* failinfo)
{
if (failinfo->stage == JOB_EXEC_STAGE_POST_EXEC
|| failinfo->stage == JOB_EXEC_STAGE_Q_POST_EXEC)
return;
if (job->job.exit_info == NULL)
job->job.exit_info = (job_exitinfo_t*)calloc(1, sizeof(job_exitinfo_t));
if (job->job.exit_info != NULL)
job->job.exit_info->exitcode = failinfo->code;
}
/* Used only by job_ctrl's SIGKILL/SIGTERM not-started-job branch (a client
* command path, do_jobcontrol_op — out of raft-apply-decoupling task 4.1's
* boundary) — mutate-then-notify, unchanged. record_job_fail_fact below is
@ -2907,11 +2924,11 @@ static int record_job_fail_fact(cluster_t* c, job_t* job,
* 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. */
(void)persist_submit_job_fact_ex(&c->persist, WAL_JOB_FAIL,
job->job.jobid, 0,
WAL_JOB_FAIL_STAGE_FLAG |
failinfo->stage);
* 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;
}

1
src/controller/meson.build
View file

@ -153,6 +153,7 @@ controller_unit_test_src = [
'testsuite/test_wal_job_dispatch_fact.c',
'testsuite/test_wal_job_start_fact.c',
'testsuite/test_wal_job_finish_fact.c',
'testsuite/test_wal_job_fail_fact.c',
'testsuite/test_wal_job_rusage_fact.c',
'testsuite/test_wal_job_ckpt_fact.c',
'testsuite/test_wal_job_preason_fact.c',

72
src/controller/persist.c
View file

@ -4113,6 +4113,46 @@ int apply_wal_entry(persist_t* p, uint32_t type, uint64_t lsn,
break;
case WAL_JOB_FAIL:
/* bug-1754: a deferred fail-fact (WAL_JOB_FAIL_STAGE_FLAG) carries
* state 0 on the wire -- recompute via the real transition core,
* mirroring apply_committed_entry(), and adopt the optional int32_t
* code trailer into exit_info. _pad == 0 is the legacy absolute-
* state shape and falls through to restore_job_state(). */
if (payload_len >= sizeof(wal_jobstate_t)
&& (((const wal_jobstate_t*)payload)->_pad
& WAL_JOB_FAIL_STAGE_FLAG)) {
const wal_jobstate_t* js = (const wal_jobstate_t*)payload;
int32_t wire_code = 0;
bool have_code = payload_len
>= sizeof(wal_jobstate_t) + sizeof(wire_code);
job_t* j = NULL;
if (have_code)
memcpy(&wire_code,
(const uint8_t*)payload + sizeof(wal_jobstate_t),
sizeof(wire_code));
lock(p->cluster->jobmgr, RTTI_LOCK_READ);
key_find(p->cluster->jobmgr, (void*)js->jobid, (void**)&j);
unlock(p->cluster->jobmgr);
if (j != NULL) {
job_failinfo_t failinfo = {
.jobid = js->jobid,
.stage = js->_pad & ~WAL_JOB_FAIL_STAGE_FLAG,
.code = (int)wire_code,
.detail = NULL,
};
lock(j, RTTI_LOCK_WRITE);
if (apply_job_fail_transition(j, &failinfo)
== JOB_TRANSITION_APPLIED
&& have_code)
apply_job_fail_exit_code(j, &failinfo);
unlock(j);
}
break;
}
/* fall through */
case WAL_JOB_STOP:
case WAL_JOB_RESUME:
case WAL_JOB_UPDATE:
@ -5006,14 +5046,27 @@ int apply_committed_entry(persist_t* p, uint32_t wal_type, uint64_t lsn,
job_failinfo_t failinfo;
bool have_failinfo = false;
if (js->_pad & WAL_JOB_FAIL_STAGE_FLAG) {
int32_t wire_code = 0;
bool have_code = len >= sizeof(wal_jobstate_t)
+ sizeof(wire_code);
/* bug-1754: persist_submit_job_fail_fact()'s int32_t code
* trailer; absent on entries written before the fix. */
if (have_code)
memcpy(&wire_code,
(const uint8_t*)payload + sizeof(wal_jobstate_t),
sizeof(wire_code));
failinfo = (job_failinfo_t){
.jobid = js->jobid,
.stage = js->_pad & ~WAL_JOB_FAIL_STAGE_FLAG,
.code = 0,
.code = (int)wire_code,
.detail = NULL,
};
have_failinfo = true;
result = apply_job_fail_transition(j, &failinfo);
if (have_code && result == JOB_TRANSITION_APPLIED)
apply_job_fail_exit_code(j, &failinfo);
} else {
result = apply_job_update_transition(j, (int)js->state);
}
@ -8062,6 +8115,23 @@ int persist_submit_job_finish_fact(persist_t* p, uint64_t jobid, int state,
payload, (uint32_t)sizeof(payload));
}
/* bug-1754: see persist.h's doc comment. Same native fixed-size trailer
* style as persist_submit_job_finish_fact() above. */
int persist_submit_job_fail_fact(persist_t* p, uint64_t jobid, uint32_t stage,
int code)
{
wal_jobstate_t js = { .jobid = jobid, .state = 0,
._pad = WAL_JOB_FAIL_STAGE_FLAG | stage };
int32_t c32 = (int32_t)code;
uint8_t payload[sizeof(js) + sizeof(c32)];
memcpy(payload, &js, sizeof(js));
memcpy(payload + sizeof(js), &c32, sizeof(c32));
return wal_hook_append_classified(p, WAL_JOB_FAIL, WAL_MODE_RETAIN,
payload, (uint32_t)sizeof(payload));
}
/* bug-1679: see persist.h's doc comment for the full rationale. Mirrors
* apply_committed_entry()'s own identical Hazard-1-guarded hook fire
* (its shared WAL_JOB_START/.../DISPATCH case, right after its

5
src/controller/resync.c
View file

@ -642,9 +642,8 @@ static void resync_apply_running_entry(cluster_t* c, worker_t* w,
* finishedFail: still routed through the existing, UNMODIFIED on_job_fail()
* /record_job_fail_fact() -- Requirement 4.2 remains unsatisfiable for this
* outcome via any existing path in this codebase, not a reconciliation-
* specific gap: WAL_JOB_FAIL's payload (persist_submit_job_fact_ex(...,
* WAL_JOB_FAIL, jobid, 0, WAL_JOB_FAIL_STAGE_FLAG|stage)) carries no
* finish-time slot at any layer, and apply_job_fail_transition() (job.c)
* specific gap: WAL_JOB_FAIL's payload (persist_submit_job_fail_fact():
* stage plus, since bug-1754, the exit code) carries no finish-time slot, and apply_job_fail_transition() (job.c)
* unconditionally sets job->job.finish_time = time(NULL) at APPLY time
* (not even receipt time) for EVERY fail-reporting caller in this system,
* confirmed by direct inspection -- a pre-existing, systemic gap that

2
src/controller/testsuite/controller_test.c
View file

@ -84,6 +84,7 @@ extern void test_replay_apply_seam(void);
extern void test_wal_job_dispatch_fact(void);
extern void test_wal_job_start_fact(void);
extern void test_wal_job_finish_fact(void);
extern void test_wal_job_fail_fact(void);
extern void test_wal_job_rusage_fact(void);
extern void test_wal_job_ckpt_fact(void);
extern void test_wal_job_preason_fact(void);
@ -269,6 +270,7 @@ int main(int argc, char* argv[])
TEST(test_wal_job_dispatch_fact);
TEST(test_wal_job_start_fact);
TEST(test_wal_job_finish_fact);
TEST(test_wal_job_fail_fact);
TEST(test_wal_job_rusage_fact);
TEST(test_wal_job_ckpt_fact);
TEST(test_wal_job_preason_fact);

408
src/controller/testsuite/test_wal_job_fail_fact.c Normal file
View file

@ -0,0 +1,408 @@
/* test_wal_job_fail_fact.c — bug-1754 regression suite
*
* A worker reports EVERY nonzero job exit as a FAIL fact, not a FINISH
* (xcecd_job_finished -> xcecd_notify_job_fail(jobid, stage, exit_code),
* worker/src/controller_connector.c), so job_failinfo_t.code is the job's
* real exit code for a MAIN-stage failure. WAL_JOB_FAIL's deferred-apply
* payload used to be a bare wal_jobstate_t{jobid, 0, STAGE_FLAG|stage} with
* no code slot, and apply_job_fail_transition() never touched exit_info --
* so job.exit_info stayed NULL for every failed job, live and after replay.
*
* Proves WAL_JOB_FAIL now carries a trailing int32_t code and that BOTH of
* persist.c's apply seams adopt it into job.exit_info:
*
* - apply_committed_entry() (live-commit apply / boot replay dispatch)
* - apply_wal_entry() (standby catch-up / replicated-apply dispatch)
*
* that a POST_EXEC-stage failure (the post-exec SCRIPT's code, not the
* job's) leaves the job's own exit_info untouched, that an old-format entry
* (no trailer) still decodes with exit_info left NULL, and that a real
* on_job_fail() call survives a real WAL write + persist_recover() restart
* with job.exit_info intact.
*
* Fixtures mirror test_wal_job_finish_fact.c (bug-1689) exactly.
*/
#include <rtti.h>
#include <capnp_c.h>
#include <errno.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unity/unity.h>
#include "cluster.h"
#include "controller.capnp.h"
#include "hook.h"
#include "jobj.h"
#include "persist.h"
#include "wal_types.h"
#include "xce/cluster_def.h"
#include "xce/types.h"
#include "xcealloc.h"
#include "xstring.h"
static int g_wjfl_jobobj_ready;
static cluster_t g_wjfl_c;
static persist_t g_wjfl_p;
/* Captures the job_failinfo_t HOOK_DETAIL_JOB_FAIL carries to its AFTER_EVENT
* subscribers (sched_job_fail_fun / apiserver_job_fail_fun read the same
* (cluster_t*, job_failinfo_t*, job_t*) shape). */
static int g_wjfl_fail_hook_fires;
static int g_wjfl_captured_code;
static int wjfl_fail_spy_hook(int type, int situation, int detail, void* data,
va_list ap)
{
(void)type; (void)data;
if (situation != HOOK_AFTER_EVENT || detail != HOOK_DETAIL_JOB_FAIL)
return 0;
(void)va_arg(ap, cluster_t*);
job_failinfo_t* info = va_arg(ap, job_failinfo_t*);
g_wjfl_fail_hook_fires++;
if (info != NULL)
g_wjfl_captured_code = info->code;
return 0;
}
static void wjfl_setup(void)
{
kvmgr_params_t int_params = { .key_type = KVMGR_KEY_INT,
.nslots = 32,
.delta = 16 };
if (makeString((char*)"wjfl-probe") == NULL)
xstring_table_init(64);
if (!g_wjfl_jobobj_ready) {
TEST_ASSERT_EQUAL_INT(0, job_obj_init(32, 32));
g_wjfl_jobobj_ready = 1;
}
memset(&g_wjfl_c, 0, sizeof(g_wjfl_c));
TEST_ASSERT_EQUAL_INT(0,
xce_hook_init(&g_wjfl_c.hooksys, XCE_CONTROLLER_HOOK_MAX));
g_wjfl_c.jobmgr = makeInstance(&xkvmgr_type, &int_params);
TEST_ASSERT_NOT_NULL(g_wjfl_c.jobmgr);
memset(&g_wjfl_p, 0, sizeof(g_wjfl_p));
g_wjfl_p.cluster = &g_wjfl_c;
g_wjfl_fail_hook_fires = 0;
g_wjfl_captured_code = 0;
}
static void wjfl_teardown(void)
{
xce_hook_fini(&g_wjfl_c.hooksys);
if (g_wjfl_c.jobmgr)
dereference(g_wjfl_c.jobmgr);
}
static job_submit_data_t g_wjfl_seed_submit;
static job_t* wjfl_seed_job(uint64_t jobid, uint32_t state)
{
xce_job_t jdef;
memset(&g_wjfl_seed_submit, 0, sizeof(g_wjfl_seed_submit));
g_wjfl_seed_submit.queue = (char*)"q";
g_wjfl_seed_submit.user = (char*)"tester";
memset(&jdef, 0, sizeof(jdef));
jdef.jobid = jobid;
jdef.state = state;
jdef.start_time = 1699999000;
jdef.submit = &g_wjfl_seed_submit;
return makeJob(g_wjfl_c.jobmgr, &jdef);
}
/* wal_jobstate_t{jobid, 0, STAGE_FLAG|stage} + int32_t code --
* persist_submit_job_fail_fact()'s payload shape. */
typedef struct {
wal_jobstate_t js;
int32_t code;
} __attribute__((packed)) wjfl_fail_payload_t;
static wjfl_fail_payload_t wjfl_fail_payload(uint64_t jobid, uint32_t stage,
int32_t code)
{
wjfl_fail_payload_t pl;
memset(&pl, 0, sizeof(pl));
pl.js.jobid = jobid;
pl.js.state = 0;
pl.js._pad = WAL_JOB_FAIL_STAGE_FLAG | stage;
pl.code = code;
return pl;
}
static void wjfl_assert_exit_code(job_exitinfo_t* got, int expected)
{
TEST_ASSERT_NOT_NULL_MESSAGE(got,
"a MAIN-stage WAL_JOB_FAIL carrying a code must populate "
"job.exit_info (bug-1754), not leave it NULL");
TEST_ASSERT_EQUAL_INT(expected, got->exitcode);
}
/* ── apply_committed_entry ────────────────────────────────────────────── */
static void test_wjfl_apply_committed_entry_sets_exit_info(void)
{
wjfl_setup();
job_t* j = wjfl_seed_job(74001, JOB_STAT_RUN);
TEST_ASSERT_NOT_NULL(j);
xce_add_hook(&g_wjfl_c.hooksys, XCE_CONTROLLER_HOOK_JOB,
wjfl_fail_spy_hook, NULL);
wjfl_fail_payload_t pl = wjfl_fail_payload(74001, JOB_EXEC_STAGE_MAIN, 42);
int rc = apply_committed_entry(&g_wjfl_p, WAL_JOB_FAIL, 400, &pl,
sizeof(pl));
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_UINT32(JOB_STAT_EXIT, j->job.state);
wjfl_assert_exit_code(j->job.exit_info, 42);
TEST_ASSERT_EQUAL_INT(1, g_wjfl_fail_hook_fires);
TEST_ASSERT_EQUAL_INT_MESSAGE(42, g_wjfl_captured_code,
"HOOK_DETAIL_JOB_FAIL subscribers must see the wire-carried code, "
"not a fabricated 0");
wjfl_teardown();
}
static void test_wjfl_apply_committed_entry_post_exec_keeps_job_exit_info(void)
{
wjfl_setup();
job_t* j = wjfl_seed_job(74002, JOB_STAT_DONE);
TEST_ASSERT_NOT_NULL(j);
j->job.exit_info = calloc(1, sizeof(job_exitinfo_t));
TEST_ASSERT_NOT_NULL(j->job.exit_info);
j->job.exit_info->exitcode = 0;
wjfl_fail_payload_t pl
= wjfl_fail_payload(74002, JOB_EXEC_STAGE_POST_EXEC, 3);
int rc = apply_committed_entry(&g_wjfl_p, WAL_JOB_FAIL, 401, &pl,
sizeof(pl));
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_UINT32(JOB_STAT_DONE | JOB_STAT_PERR, j->job.state);
TEST_ASSERT_NOT_NULL(j->job.exit_info);
TEST_ASSERT_EQUAL_INT_MESSAGE(0, j->job.exit_info->exitcode,
"a POST_EXEC failure's code is the post-exec script's, not the "
"job's -- it must not overwrite the job's own exit code");
wjfl_teardown();
}
static void test_wjfl_apply_committed_entry_backward_compat_old_payload(void)
{
wjfl_setup();
job_t* j = wjfl_seed_job(74003, JOB_STAT_RUN);
TEST_ASSERT_NOT_NULL(j);
wal_jobstate_t js = { .jobid = 74003, .state = 0,
._pad = WAL_JOB_FAIL_STAGE_FLAG
| JOB_EXEC_STAGE_MAIN };
int rc = apply_committed_entry(&g_wjfl_p, WAL_JOB_FAIL, 402, &js,
sizeof(js));
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_UINT32(JOB_STAT_EXIT, j->job.state);
TEST_ASSERT_NULL_MESSAGE(j->job.exit_info,
"an old-format WAL_JOB_FAIL (no code trailer) must leave exit_info "
"NULL rather than fabricate an exit code");
wjfl_teardown();
}
/* ── apply_wal_entry ──────────────────────────────────────────────────── */
static void test_wjfl_apply_wal_entry_sets_state_and_exit_info(void)
{
wjfl_setup();
job_t* j = wjfl_seed_job(74004, JOB_STAT_RUN);
TEST_ASSERT_NOT_NULL(j);
wjfl_fail_payload_t pl = wjfl_fail_payload(74004, JOB_EXEC_STAGE_MAIN, 9);
int rc = apply_wal_entry(&g_wjfl_p, WAL_JOB_FAIL, 403, &pl, sizeof(pl));
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_UINT32_MESSAGE(JOB_STAT_EXIT, j->job.state,
"a deferred (STAGE_FLAG) WAL_JOB_FAIL carries state 0 on the wire; "
"the standby seam must recompute EXIT, not assign that 0");
wjfl_assert_exit_code(j->job.exit_info, 9);
wjfl_teardown();
}
/* ── Restart replay through the real on_job_fail() receipt path ───────── */
static int wjfl_encode_job(const xce_job_t* j, uint8_t** out, size_t* out_len)
{
struct capn c;
struct capn_segment* cs;
struct Job cj;
Job_ptr jp;
capn_init_malloc(&c);
cs = capn_root(&c).seg;
encode_Job(cs, &cj, (xce_job_t*)j);
jp = new_Job(cs);
write_Job(&cj, jp);
capn_setp(capn_root(&c), 0, jp.p);
*out_len = capn_size(&c);
*out = malloc(*out_len);
if (!*out) { capn_free(&c); return -ENOMEM; }
capn_write_mem(&c, *out, *out_len, 0);
capn_free(&c);
return 0;
}
static void test_wjfl_real_on_job_fail_survives_restart(void)
{
char tmp[256];
strcpy(tmp, "/tmp/xce_wjfl_onfail_XXXXXX");
TEST_ASSERT_NOT_NULL(mkdtemp(tmp));
const uint64_t jobid = 74005;
const int real_exit_code = 2;
/* Phase 1: durable WAL_JOB_SUBMIT + WAL_JOB_START seeds, a live RUN job,
* then the real worker-FAIL receipt handler appends WAL_JOB_FAIL. */
{
persist_config_t cfg;
cluster_t c;
kvmgr_params_t int_params = { .key_type = KVMGR_KEY_INT,
.nslots = 32, .delta = 16 };
if (makeString((char*)"wjfl-onfail-probe") == NULL)
xstring_table_init(64);
if (!g_wjfl_jobobj_ready) {
TEST_ASSERT_EQUAL_INT(0, job_obj_init(32, 32));
g_wjfl_jobobj_ready = 1;
}
memset(&c, 0, sizeof(c));
TEST_ASSERT_EQUAL_INT(0,
xce_hook_init(&c.hooksys, XCE_CONTROLLER_HOOK_MAX));
c.jobmgr = makeInstance(&xkvmgr_type, &int_params);
TEST_ASSERT_NOT_NULL(c.jobmgr);
cfg = persist_config_defaults();
cfg.wal_sync_ms = 5000;
persist_set_workdir(&c.persist, tmp, "ctrl-test");
TEST_ASSERT_EQUAL_INT(0, persist_init(&c.persist, &cfg, NULL));
TEST_ASSERT_EQUAL_INT(0, persist_start(&c.persist));
job_submit_data_t submit;
xce_job_t jdef;
uint8_t* buf = NULL;
size_t sz = 0;
memset(&submit, 0, sizeof(submit));
submit.queue = (char*)"oq";
submit.user = (char*)"tester";
memset(&jdef, 0, sizeof(jdef));
jdef.jobid = jobid;
jdef.state = JOB_STAT_PEND;
jdef.submit = &submit;
TEST_ASSERT_EQUAL_INT(0, wjfl_encode_job(&jdef, &buf, &sz));
TEST_ASSERT_EQUAL_INT(0,
persist_wal_async(&c.persist, WAL_JOB_SUBMIT, buf, (uint32_t)sz));
free(buf);
wal_jobstate_t start_js = { .jobid = jobid, .state = JOB_STAT_RUN,
._pad = 0 };
TEST_ASSERT_EQUAL_INT(0,
persist_wal_async(&c.persist, WAL_JOB_START, &start_js,
sizeof(start_js)));
jdef.state = JOB_STAT_RUN;
jdef.start_time = 1699999500;
TEST_ASSERT_NOT_NULL(makeJob(c.jobmgr, &jdef));
job_failinfo_t fail = { .jobid = jobid,
.stage = JOB_EXEC_STAGE_MAIN,
.code = real_exit_code,
.detail = (char*)"exit" };
TEST_ASSERT_EQUAL_INT(0, on_job_fail(&c, &fail));
TEST_ASSERT_EQUAL_INT(0, persist_flush(&c.persist));
persist_stop(&c.persist);
persist_fini(&c.persist);
dereference(c.jobmgr);
xce_hook_fini(&c.hooksys);
}
/* Phase 2: fresh persist_t + cluster_t, recover from disk. */
{
persist_t* p2 = calloc(1, sizeof(persist_t));
cluster_t c2;
job_t* j2 = NULL;
kvmgr_params_t int_params = { .key_type = KVMGR_KEY_INT,
.nslots = 32, .delta = 16 };
persist_config_t cfg = persist_config_defaults();
TEST_ASSERT_NOT_NULL(p2);
cfg.wal_sync_ms = 5000;
persist_set_workdir(p2, tmp, "ctrl-test");
TEST_ASSERT_EQUAL_INT(0, persist_init(p2, &cfg, NULL));
TEST_ASSERT_EQUAL_INT(0, persist_start(p2));
memset(&c2, 0, sizeof(c2));
TEST_ASSERT_EQUAL_INT(0,
xce_hook_init(&c2.hooksys, XCE_CONTROLLER_HOOK_MAX));
c2.jobmgr = makeInstance(&xkvmgr_type, &int_params);
TEST_ASSERT_NOT_NULL(c2.jobmgr);
p2->cluster = &c2;
TEST_ASSERT_EQUAL_INT(0, persist_recover(p2));
lock(c2.jobmgr, RTTI_LOCK_READ);
key_find(c2.jobmgr, (void*)jobid, (void**)&j2);
unlock(c2.jobmgr);
TEST_ASSERT_NOT_NULL(j2);
TEST_ASSERT_EQUAL_UINT32(JOB_STAT_EXIT, j2->job.state);
wjfl_assert_exit_code(j2->job.exit_info, real_exit_code);
xce_hook_fini(&c2.hooksys);
dereference(c2.jobmgr);
persist_stop(p2);
persist_fini(p2);
free(p2);
}
char cmd[512];
snprintf(cmd, sizeof(cmd), "rm -rf %s", tmp);
if (system(cmd) != 0)
fprintf(stderr, "cleanup warning: rm -rf %s failed\n", tmp);
}
/* ── Test group entry point ───────────────────────────────────────────── */
void test_wal_job_fail_fact(void)
{
UnitySetTestFile(__FILE__);
RUN_TEST(test_wjfl_apply_committed_entry_sets_exit_info);
RUN_TEST(test_wjfl_apply_committed_entry_post_exec_keeps_job_exit_info);
RUN_TEST(test_wjfl_apply_committed_entry_backward_compat_old_payload);
RUN_TEST(test_wjfl_apply_wal_entry_sets_state_and_exit_info);
RUN_TEST(test_wjfl_real_on_job_fail_survives_restart);
}

45
src/controller/testsuite/test_worker_resync.c
View file

@ -3013,31 +3013,15 @@ static void test_full_rejoin_reconciliation_round_trip(void)
TEST_ASSERT_EQUAL_UINT32_MESSAGE(JOB_STAT_EXIT, j2_finfail->job.state,
"finished-failure: the DEFERRED on_job_fail/record_job_fail_"
"fact fact must apply to EXIT on replay (Req 4.1)");
/* Req 4.2 is DOCUMENTED-unsatisfiable for the finished-FAILURE
* finish_time specifically (bug-1747, pre-existing, systemic,
* explicitly deferred, unrelated to this task's own boundary) --
* NOT re-asserted here as if it were correct. This round-trip test
* additionally confirms, and CONCERNS reports, a related, more
* complete gap bug-1747's own text does not mention: the worker-
* reported EXIT CODE (entry->exit_code, plumbed all the way into
* job_failinfo_t.code by resync_apply_finished_entry, resync.c) is
* not merely mistimed but is NEVER durably recorded or applied to
* job.job.exit_info AT ALL for a FAIL outcome, by any existing path
* in this codebase -- record_job_fail_fact's own WAL_JOB_FAIL
* payload (persist_submit_job_fact_ex(..., WAL_JOB_FAIL, jobid, 0,
* WAL_JOB_FAIL_STAGE_FLAG|stage), job.c) carries no code slot at
* all, and apply_job_fail_transition() (job.c) never touches job.
* job.exit_info. This is asserted honestly below as NULL -- the
* TRUE current behavior -- rather than asserting a "correct" value
* this codebase cannot actually produce (this file's own task 3.2
* Implementation Notes: "a green suite is not proof of correctness
* when the test itself encodes the wrong expectation"). */
TEST_ASSERT_NULL_MESSAGE(j2_finfail->job.exit_info,
"pre-existing, systemic gap (see comment above, reported in "
"this task's own CONCERNS, not fixed here): a worker-reported "
"exit code for a FAIL outcome is never durably recorded by any "
"existing path in this codebase -- exit_info stays NULL, not "
"the worker-reported code");
/* Req 4.2 remains DOCUMENTED-unsatisfiable for the finished-
* FAILURE finish_time specifically (bug-1747) -- not asserted here.
* The worker-reported EXIT CODE, however, is durably recorded via
* WAL_JOB_FAIL's code trailer since bug-1754. */
TEST_ASSERT_NOT_NULL_MESSAGE(j2_finfail->job.exit_info,
"bug-1754: a finished-failure's worker-reported exit code must "
"survive restart replay in job.exit_info");
TEST_ASSERT_EQUAL_INT(worker_reported_exit_code,
j2_finfail->job.exit_info->exitcode);
TEST_ASSERT_EQUAL_UINT32_MESSAGE(JOB_STAT_PEND, j2_unrec->job.state,
"unrecognized: the EAGER record_job_reconcile_pending_fact's "
@ -4080,12 +4064,11 @@ static void test_burst_recovery_unbounded_by_job_count(void)
"EXIT, not DONE, distinctly from its 5 finished-success "
"siblings in the SAME round");
/* Req 4.2 remains documented-unsatisfiable for FAIL's own
* finish_time specifically (bug-1747, pre-existing, systemic, out
* of this task's boundary -- see tasks 7.1/7.2's own identical
* finding); exit_info stays NULL for the same pre-existing reason,
* asserted honestly rather than as a wrong "correct" value this
* codebase cannot actually produce. */
TEST_ASSERT_NULL(j2_lost[TWR_BURST_N_LOST - 1]->job.exit_info);
* finish_time specifically (bug-1747). Its exit code is recorded
* since bug-1754. */
TEST_ASSERT_NOT_NULL(j2_lost[TWR_BURST_N_LOST - 1]->job.exit_info);
TEST_ASSERT_EQUAL_INT(lost_fail_exit_code,
j2_lost[TWR_BURST_N_LOST - 1]->job.exit_info->exitcode);
/* The 'none duplicated' completion criterion, confirmed to
* survive a restart replay too: the already-DONE job's ORIGINAL

12
src/include/persist.h
View file

@ -1523,6 +1523,18 @@ int persist_submit_job_start_fact(persist_t* p, uint64_t jobid, int state,
int persist_submit_job_finish_fact(persist_t* p, uint64_t jobid, int state,
uint64_t finish_time, int exit_code);
/* bug-1754: WAL_JOB_FAIL's deferred-apply sibling of
* persist_submit_job_fact_ex() -- header {jobid, 0, WAL_JOB_FAIL_STAGE_FLAG
* | stage} followed by a fixed int32_t code (job_failinfo_t.code). A worker
* reports every nonzero job exit as a FAIL, so for a MAIN-stage failure
* `code` is the job's real exit code; apply_job_fail_exit_code() (job.c)
* adopts it into job.exit_info at both apply seams. A payload of exactly
* sizeof(wal_jobstate_t) (any WAL_JOB_FAIL entry written before this fix)
* carries no code and leaves exit_info untouched. Returns the local-append
* rc, same contract as persist_submit_job_fact(). */
int persist_submit_job_fail_fact(persist_t* p, uint64_t jobid, uint32_t stage,
int code);
/* bug-1679: fire XCE_CONTROLLER_HOOK_JOB's HOOK_DETAIL_JOB_UPDATE AFTER_EVENT
* for a job whose durable WAL fact the caller has ALREADY appended itself
* (e.g. dispatch_a_job_()'s own persist_submit_job_dispatch_fact() call,

8
src/include/wal_types.h
View file

@ -44,9 +44,11 @@ typedef struct {
* as-is". A nonzero _pad's exact interpretation is per-type — see the two
* markers below. */
#define WAL_JOB_FAIL_STAGE_FLAG 0x80000000u
/* WAL_JOB_FAIL: _pad = WAL_JOB_FAIL_STAGE_FLAG | job_failinfo_t.stage.
* Apply time reconstructs a minimal job_failinfo_t from {jobid, stage}
* and calls the real apply_job_fail_transition() against current state. */
/* WAL_JOB_FAIL: _pad = WAL_JOB_FAIL_STAGE_FLAG | job_failinfo_t.stage,
* optionally followed by an int32_t job_failinfo_t.code (bug-1754).
* Apply time reconstructs a minimal job_failinfo_t from {jobid, stage,
* code} and calls the real apply_job_fail_transition() against current
* state, then apply_job_fail_exit_code() when the code is present. */
#define WAL_JOB_UPDATE_POSTDONE_MARK 1u
/* WAL_JOB_UPDATE: _pad = WAL_JOB_UPDATE_POSTDONE_MARK marks a postdone
* merge-fact (`state` is unused/0) — apply time calls