test(controller): cover WAL_JOB_DISPATCH allocation persistence (bug-1678)
test_dispatch_commit.c's test_dc_success_appends_wal_job_dispatch_durably (already drove the real dispatch_a_job_() production path through a real WAL-write + persist_recover() restart cycle, previously only checking recovered state) now also asserts the recovered job.allocs matches what was dispatched -- host, resource name, value. This is the primary regression test for the user's exact acceptance criterion: "after recover the job should get the same as it's state before shutdown." Independently reproduced RED->GREEN (not just narrated): reverted the production fix with the test extension kept, rebuilt, confirmed FAIL at exactly this assertion; restored, rebuilt, confirmed PASS. test_wal_job_dispatch_fact.c: added 3 new tests exercising the richer payload directly on both apply_committed_entry and apply_wal_entry, plus a full restart-replay variant. All 3 pre-existing tests (bare wal_jobstate_t payloads, no trailing bytes) pass completely unchanged, proving backward compatibility with any WAL_JOB_DISPATCH entry written before this fix. Full controller_unit_test suite independently re-run: 1136/1136 pass, zero regressions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
This commit is contained in:
parent
db142d415f
commit
f8851353cd
2 changed files with 250 additions and 0 deletions
|
|
@ -619,6 +619,31 @@ static void test_dc_success_appends_wal_job_dispatch_durably(void)
|
|||
"WAL_JOB_DISPATCH fact appended by dispatch_a_job_()'s real "
|
||||
"success path -- no WAL_JOB_START anywhere in this log");
|
||||
|
||||
/* bug-1678: restart replay must recover not just STATE but the
|
||||
* resource allocation itself, or EXEC_HOST is permanently blank for
|
||||
* a job that was genuinely RUNNING before shutdown -- the user's
|
||||
* exact acceptance criterion ("after recover the job should get the
|
||||
* same as it's state before shutdown"). Mirrors Phase 1's real
|
||||
* dispatch: host "wkr-replay", one resource "slots" value 1.0. */
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(j2->job.allocs,
|
||||
"restart replay must recover the dispatch-time resource "
|
||||
"allocation (bug-1678), not just the RUN state");
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(1, j2->job.allocs->nhosts,
|
||||
"recovered allocation must have exactly 1 host, matching what "
|
||||
"was dispatched in Phase 1");
|
||||
TEST_ASSERT_NOT_NULL(j2->job.allocs->allocs);
|
||||
TEST_ASSERT_NOT_NULL(j2->job.allocs->allocs[0]);
|
||||
TEST_ASSERT_EQUAL_STRING_MESSAGE("wkr-replay",
|
||||
j2->job.allocs->allocs[0]->host,
|
||||
"recovered allocation's host must match the dispatched worker");
|
||||
TEST_ASSERT_EQUAL_INT(1, j2->job.allocs->allocs[0]->nallocs);
|
||||
TEST_ASSERT_NOT_NULL(j2->job.allocs->allocs[0]->allocs);
|
||||
TEST_ASSERT_NOT_NULL(j2->job.allocs->allocs[0]->allocs[0]);
|
||||
TEST_ASSERT_EQUAL_STRING("slots",
|
||||
j2->job.allocs->allocs[0]->allocs[0]->resname);
|
||||
TEST_ASSERT_EQUAL_FLOAT(1.0f,
|
||||
j2->job.allocs->allocs[0]->allocs[0]->value);
|
||||
|
||||
xce_hook_fini(&c2.hooksys);
|
||||
if (c2.jobmgr) dereference(c2.jobmgr);
|
||||
dc_teardown_persist(p2);
|
||||
|
|
|
|||
|
|
@ -46,6 +46,7 @@
|
|||
#include "wal_types.h"
|
||||
#include "xce/cluster_def.h"
|
||||
#include "xce/types.h"
|
||||
#include "xcealloc.h"
|
||||
#include "xstring.h"
|
||||
|
||||
/* jobj.c's jobsub table and the process-global xstring table are shared
|
||||
|
|
@ -161,6 +162,92 @@ static void test_wjd_apply_committed_entry_transitions_pend_to_run(void)
|
|||
printf("test_wjd_apply_committed_entry_transitions_pend_to_run: PASS\n");
|
||||
}
|
||||
|
||||
/* bug-1678: build a wal_jobstate_t header + trailing Cap'n Proto JobAlloc
|
||||
* bytes -- persist_submit_job_dispatch_fact()'s payload shape -- mirroring
|
||||
* capnp_encode_joballoc (persist.c, file-static) since job.c/persist.c's own
|
||||
* static helpers aren't linkable from a test TU. Caller owns *out. */
|
||||
static int wjd_build_dispatch_payload(uint64_t jobid, uint32_t state,
|
||||
xce_job_alloc_t* alloc,
|
||||
uint8_t** out, size_t* out_len)
|
||||
{
|
||||
struct capn c;
|
||||
struct capn_segment* cs;
|
||||
struct JobAlloc cja;
|
||||
JobAlloc_ptr jap;
|
||||
uint8_t* allocbuf;
|
||||
size_t alloclen;
|
||||
wal_jobstate_t js = { .jobid = jobid, .state = state, ._pad = 0 };
|
||||
|
||||
capn_init_malloc(&c);
|
||||
cs = capn_root(&c).seg;
|
||||
encode_JobAlloc(cs, &cja, alloc);
|
||||
jap = new_JobAlloc(cs);
|
||||
write_JobAlloc(&cja, jap);
|
||||
capn_setp(capn_root(&c), 0, jap.p);
|
||||
|
||||
alloclen = capn_size(&c);
|
||||
allocbuf = malloc(alloclen);
|
||||
if (!allocbuf) { capn_free(&c); return -ENOMEM; }
|
||||
capn_write_mem(&c, allocbuf, alloclen, 0);
|
||||
capn_free(&c);
|
||||
|
||||
*out_len = sizeof(js) + alloclen;
|
||||
*out = malloc(*out_len);
|
||||
if (!*out) { free(allocbuf); return -ENOMEM; }
|
||||
memcpy(*out, &js, sizeof(js));
|
||||
memcpy(*out + sizeof(js), allocbuf, alloclen);
|
||||
free(allocbuf);
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* One host ("wjd-worker"), one resource ("slots", value 1.0) -- the same
|
||||
* minimal shape test_dispatch_commit.c's own dispatch tests use. Caller owns
|
||||
* the result (release via free_JobAlloc_ptr). */
|
||||
static xce_job_alloc_t* wjd_make_test_alloc(uint64_t jobid)
|
||||
{
|
||||
return make_job_alloc(jobid, 1,
|
||||
make_host_allocs(1,
|
||||
make_host_alloc((char*)"wjd-worker", 1,
|
||||
make_allocs(1,
|
||||
make_alloc((char*)"slots", NULL, 0, 1.0f, 0)))));
|
||||
}
|
||||
|
||||
static void test_wjd_apply_committed_entry_transitions_and_sets_allocs(void)
|
||||
{
|
||||
wjd_setup();
|
||||
|
||||
job_t* j = wjd_seed_job(71004, JOB_STAT_PEND);
|
||||
TEST_ASSERT_NOT_NULL(j);
|
||||
|
||||
xce_job_alloc_t* alloc = wjd_make_test_alloc(71004);
|
||||
TEST_ASSERT_NOT_NULL(alloc);
|
||||
|
||||
uint8_t* payload = NULL;
|
||||
size_t plen = 0;
|
||||
TEST_ASSERT_EQUAL_INT(0,
|
||||
wjd_build_dispatch_payload(71004, JOB_STAT_RUN, alloc, &payload,
|
||||
&plen));
|
||||
free_JobAlloc_ptr(&alloc);
|
||||
|
||||
int rc = apply_committed_entry(&g_wjd_p, WAL_JOB_DISPATCH, 102, payload,
|
||||
(uint32_t)plen);
|
||||
free(payload);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
TEST_ASSERT_EQUAL_UINT32(JOB_STAT_RUN, j->job.state);
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(j->job.allocs,
|
||||
"a WAL_JOB_DISPATCH payload with trailing JobAlloc bytes must set "
|
||||
"job.allocs (bug-1678), not just state");
|
||||
TEST_ASSERT_EQUAL_INT(1, j->job.allocs->nhosts);
|
||||
TEST_ASSERT_EQUAL_STRING("wjd-worker", j->job.allocs->allocs[0]->host);
|
||||
TEST_ASSERT_EQUAL_STRING("slots",
|
||||
j->job.allocs->allocs[0]->allocs[0]->resname);
|
||||
|
||||
wjd_teardown();
|
||||
printf("test_wjd_apply_committed_entry_transitions_and_sets_allocs: "
|
||||
"PASS\n");
|
||||
}
|
||||
|
||||
/* ── apply_wal_entry(): standby catch-up / replicated-apply dispatch ────── */
|
||||
|
||||
static void
|
||||
|
|
@ -192,6 +279,41 @@ test_wjd_apply_wal_entry_transitions_pend_to_run_no_hook(void)
|
|||
"PASS\n");
|
||||
}
|
||||
|
||||
static void test_wjd_apply_wal_entry_transitions_and_sets_allocs(void)
|
||||
{
|
||||
wjd_setup();
|
||||
|
||||
job_t* j = wjd_seed_job(71005, JOB_STAT_PEND);
|
||||
TEST_ASSERT_NOT_NULL(j);
|
||||
|
||||
xce_job_alloc_t* alloc = wjd_make_test_alloc(71005);
|
||||
TEST_ASSERT_NOT_NULL(alloc);
|
||||
|
||||
uint8_t* payload = NULL;
|
||||
size_t plen = 0;
|
||||
TEST_ASSERT_EQUAL_INT(0,
|
||||
wjd_build_dispatch_payload(71005, JOB_STAT_RUN, alloc, &payload,
|
||||
&plen));
|
||||
free_JobAlloc_ptr(&alloc);
|
||||
|
||||
int rc = apply_wal_entry(&g_wjd_p, WAL_JOB_DISPATCH, 103, payload,
|
||||
(uint32_t)plen);
|
||||
free(payload);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
TEST_ASSERT_EQUAL_UINT32(JOB_STAT_RUN, j->job.state);
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(j->job.allocs,
|
||||
"apply_wal_entry's catch-up dispatch must also recover job.allocs "
|
||||
"from a WAL_JOB_DISPATCH payload's trailing JobAlloc bytes "
|
||||
"(bug-1678) -- required for a promoted standby, not just the "
|
||||
"primary's own boot replay");
|
||||
TEST_ASSERT_EQUAL_INT(1, j->job.allocs->nhosts);
|
||||
TEST_ASSERT_EQUAL_STRING("wjd-worker", j->job.allocs->allocs[0]->host);
|
||||
|
||||
wjd_teardown();
|
||||
printf("test_wjd_apply_wal_entry_transitions_and_sets_allocs: PASS\n");
|
||||
}
|
||||
|
||||
/* ── Restart-replay Observable (this task's completion criterion) ───────── */
|
||||
|
||||
static persist_t* wjd_make_fresh_persist(const char* tmpdir)
|
||||
|
|
@ -330,12 +452,115 @@ test_wjd_restart_replay_recovers_running_without_start_fact(void)
|
|||
"PASS\n");
|
||||
}
|
||||
|
||||
/* bug-1678: sibling of the test above, proving the user's exact acceptance
|
||||
* criterion ("after recover the job should get the same as it's state
|
||||
* before shutdown") at this file's own direct-payload granularity: a
|
||||
* WAL_JOB_DISPATCH fact carrying trailing JobAlloc bytes survives a real
|
||||
* restart (persist_recover over real on-disk WAL) with job.allocs intact,
|
||||
* not just job.state. test_dispatch_commit.c's
|
||||
* test_dc_success_appends_wal_job_dispatch_durably covers the same
|
||||
* observable through the REAL dispatch_a_job_() production call site; this
|
||||
* test isolates it at the WAL fact level, mirroring how this file's other
|
||||
* tests already isolate apply_committed_entry/apply_wal_entry from the live
|
||||
* caller. */
|
||||
static void
|
||||
test_wjd_restart_replay_recovers_running_and_allocs_with_dispatch_fact(void)
|
||||
{
|
||||
char tmp[256];
|
||||
strcpy(tmp, "/tmp/xce_wjd_replay2_XXXXXX");
|
||||
TEST_ASSERT_NOT_NULL(mkdtemp(tmp));
|
||||
|
||||
/* Phase 1: write the log to real disk. */
|
||||
{
|
||||
persist_t* p = wjd_make_fresh_persist(tmp);
|
||||
TEST_ASSERT_NOT_NULL(p);
|
||||
|
||||
job_submit_data_t submit;
|
||||
memset(&submit, 0, sizeof(submit));
|
||||
submit.queue = (char*)"q";
|
||||
submit.user = (char*)"tester";
|
||||
|
||||
xce_job_t jdef;
|
||||
memset(&jdef, 0, sizeof(jdef));
|
||||
jdef.jobid = 71006;
|
||||
jdef.state = JOB_STAT_PEND;
|
||||
jdef.submit = &submit;
|
||||
|
||||
uint8_t* job_payload = NULL;
|
||||
size_t job_plen = 0;
|
||||
TEST_ASSERT_EQUAL_INT(0,
|
||||
wjd_encode_job(&jdef, &job_payload, &job_plen));
|
||||
TEST_ASSERT_EQUAL_INT(0,
|
||||
persist_wal_async(p, WAL_JOB_SUBMIT, job_payload,
|
||||
(uint32_t)job_plen));
|
||||
free(job_payload);
|
||||
|
||||
xce_job_alloc_t* alloc = wjd_make_test_alloc(71006);
|
||||
TEST_ASSERT_NOT_NULL(alloc);
|
||||
|
||||
uint8_t* dispatch_payload = NULL;
|
||||
size_t dispatch_plen = 0;
|
||||
TEST_ASSERT_EQUAL_INT(0,
|
||||
wjd_build_dispatch_payload(71006, JOB_STAT_RUN, alloc,
|
||||
&dispatch_payload, &dispatch_plen));
|
||||
free_JobAlloc_ptr(&alloc);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(0,
|
||||
persist_wal_async(p, WAL_JOB_DISPATCH, dispatch_payload,
|
||||
(uint32_t)dispatch_plen));
|
||||
free(dispatch_payload);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(0, persist_flush(p));
|
||||
wjd_teardown_persist(p);
|
||||
}
|
||||
|
||||
/* Phase 2: fresh persist_t + fresh cluster_t, recover from disk —
|
||||
* simulates a controller restart. */
|
||||
{
|
||||
persist_t* p2 = wjd_make_fresh_persist(tmp);
|
||||
TEST_ASSERT_NOT_NULL(p2);
|
||||
|
||||
wjd_setup();
|
||||
p2->cluster = &g_wjd_c;
|
||||
|
||||
int rc = persist_recover(p2);
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
|
||||
job_t* j = wjd_find_job(71006);
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(j,
|
||||
"restart replay must construct the job via WAL_JOB_SUBMIT");
|
||||
TEST_ASSERT_EQUAL_UINT32(JOB_STAT_RUN, j->job.state);
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(j->job.allocs,
|
||||
"restart replay must recover job.allocs from the "
|
||||
"WAL_JOB_DISPATCH fact's trailing JobAlloc bytes (bug-1678), "
|
||||
"not just job.state");
|
||||
TEST_ASSERT_EQUAL_INT(1, j->job.allocs->nhosts);
|
||||
TEST_ASSERT_EQUAL_STRING("wjd-worker",
|
||||
j->job.allocs->allocs[0]->host);
|
||||
|
||||
wjd_teardown();
|
||||
wjd_teardown_persist(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);
|
||||
|
||||
printf("test_wjd_restart_replay_recovers_running_and_allocs_with_"
|
||||
"dispatch_fact: PASS\n");
|
||||
}
|
||||
|
||||
/* ── Test group entry point ───────────────────────────────────────────── */
|
||||
|
||||
void test_wal_job_dispatch_fact(void)
|
||||
{
|
||||
UnitySetTestFile(__FILE__);
|
||||
RUN_TEST(test_wjd_apply_committed_entry_transitions_pend_to_run);
|
||||
RUN_TEST(test_wjd_apply_committed_entry_transitions_and_sets_allocs);
|
||||
RUN_TEST(test_wjd_apply_wal_entry_transitions_pend_to_run_no_hook);
|
||||
RUN_TEST(test_wjd_apply_wal_entry_transitions_and_sets_allocs);
|
||||
RUN_TEST(test_wjd_restart_replay_recovers_running_without_start_fact);
|
||||
RUN_TEST(
|
||||
test_wjd_restart_replay_recovers_running_and_allocs_with_dispatch_fact);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue