diff --git a/src/controller/include/cluster.h b/src/controller/include/cluster.h index f5ff7c31..3911ec29 100644 --- a/src/controller/include/cluster.h +++ b/src/controller/include/cluster.h @@ -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 diff --git a/src/controller/job.c b/src/controller/job.c index 839c9d89..11f1a047 100644 --- a/src/controller/job.c +++ b/src/controller/job.c @@ -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; } diff --git a/src/controller/meson.build b/src/controller/meson.build index c711c895..445ca06d 100644 --- a/src/controller/meson.build +++ b/src/controller/meson.build @@ -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', diff --git a/src/controller/persist.c b/src/controller/persist.c index 8cc2d27b..f65b632f 100644 --- a/src/controller/persist.c +++ b/src/controller/persist.c @@ -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 diff --git a/src/controller/resync.c b/src/controller/resync.c index ba946bd0..814d49ca 100644 --- a/src/controller/resync.c +++ b/src/controller/resync.c @@ -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 diff --git a/src/controller/testsuite/controller_test.c b/src/controller/testsuite/controller_test.c index 19f6d2c0..d8ca7dea 100644 --- a/src/controller/testsuite/controller_test.c +++ b/src/controller/testsuite/controller_test.c @@ -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); diff --git a/src/controller/testsuite/test_wal_job_fail_fact.c b/src/controller/testsuite/test_wal_job_fail_fact.c new file mode 100644 index 00000000..92353ae2 --- /dev/null +++ b/src/controller/testsuite/test_wal_job_fail_fact.c @@ -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 + +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#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); +} diff --git a/src/controller/testsuite/test_worker_resync.c b/src/controller/testsuite/test_worker_resync.c index b143b78c..4e7ce366 100644 --- a/src/controller/testsuite/test_worker_resync.c +++ b/src/controller/testsuite/test_worker_resync.c @@ -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 diff --git a/src/include/persist.h b/src/include/persist.h index 98a3220f..d1c81f0d 100644 --- a/src/include/persist.h +++ b/src/include/persist.h @@ -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, diff --git a/src/include/wal_types.h b/src/include/wal_types.h index 1096ae6c..c00f17df 100644 --- a/src/include/wal_types.h +++ b/src/include/wal_types.h @@ -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