fix: bug-1445 -- first-boot promotion check races its own first replay

raft_cb_become_leader()'s eligibility gate read
persist_apply_resource_fenced() before this process's own first
cluster_init_phase2()/persist_recover()/replay_wal() call ever ran --
so a node whose first-ever replay hit a resource-exhaustion apply
failure could still become CTRL_PRIMARY, because the fence that
replay would engage didn't exist yet when the gate checked it. Every
LATER promotion evaluation was already correctly protected; this was
a narrow gap specific to a process's single very-first election.

Fix: re-check persist_apply_resource_fenced() immediately after
cluster_init_phase2() completes on first boot, refusing promotion
(same WARN + ha_arm_promotion_fence_timer() + return pattern as the
existing refusal branch) if that first replay just engaged the fence.

Confirmed ha_elect_evaluate_and_signal (the other election path) is
dead in production for the promotion outcome -- every live call site
of handle_request_vote_reply passes vote_granted=0 since bug-828
redirected real grant recording to the record-only, non-signaling
ha_record_vote_reply -- so raft_cb_become_leader is the sole live
promotion path and this fix's scope is complete.

TDD: new test test_first_boot_promotion_evaluates_own_first_replay_fence
wires ha_phase2_hook to actually call persist_recover(), reproducing
production ordering exactly. RED-confirmed against the unfixed code
at the precise predicted assertion; GREEN-confirmed after the fix.
Full controller_unit_test regression (bundles every HA-adjacent
testsuite/test_*.c) green, 0 new failures.

Originally documented (not fixed) in job-array-single-wal-record
task 7.2's Implementation Notes; logged as bug-1445 and fixed here,
outside that spec's own task list per its persist.c-only boundary.
This commit is contained in:
Rongsong Shen 2026-07-17 10:42:13 +00:00
parent 5ddc20f359
commit ff01db3665
4 changed files with 227 additions and 6 deletions

View file

@ -223,6 +223,22 @@ static int trf_encode_job_submit(uint64_t base_jobid, uint64_t array_size,
return 0;
}
/* bug-1445 test-only stub: reproduces the REAL production call chain
* (cluster_init_phase2 -> persist_recover -> replay_wal) at the REAL call
* site inside raft_cb_become_leader's is_first_boot block, wired via
* ha_phase2_hook (ha.h:1619) since XCE_TESTING builds skip the real
* cluster_init_phase2() otherwise (ha.c:1431-1433). Deliberately calls ONLY
* persist_recover -- the narrow slice bug-1445 is about -- not the rest of
* cluster_init_phase2's other work (config load etc.), which this test does
* not need. */
#ifdef XCE_TESTING
static int trf_phase2_hook_calls_persist_recover(void *c)
{
cluster_t *cc = (cluster_t *)c;
return persist_recover(&cc->persist);
}
#endif
/* ── The test: crash-while-would-be-fenced, restart into a boot replay that
* hits the SAME resource-exhaustion condition ────────────────────────────── */
@ -473,12 +489,181 @@ test_replay_wal_engages_fence_on_resource_exhaustion_failure(void)
"PASS\n");
}
/* ── bug-1445: a node's very FIRST-EVER promotion evaluation races its OWN
* first replay ─────────────────────────────────────────────────────────────
* Unlike the sibling test above (which calls persist_recover() BEFORE the
* election path, proving a LATER evaluation is protected), this test leaves
* the persist_t completely un-recovered when raft_cb_become_leader_with_
* cluster_for_test() is invoked -- ha->phase2_done is fresh (0) and
* ha_phase2_hook is wired to actually call persist_recover(), so
* cluster_init_phase2()'s real call chain runs from INSIDE this SAME
* election evaluation, exactly as it does in production
* (raft_cb_become_leader, ha.c ~1427-1439). This is the exact scenario
* bug-1445 names and the sibling test's own SCOPE NOTE explicitly says it
* does not cover. */
static void
test_first_boot_promotion_evaluates_own_first_replay_fence(void)
{
char tmp[256];
strcpy(tmp, "/tmp/xce_trf_firstboot_XXXXXX");
TEST_ASSERT_NOT_NULL(mkdtemp(tmp));
const uint64_t base_jobid = 981000;
const uint64_t array_size = 5;
const int fail_index = 2;
job_submit_data_t spec;
memset(&spec, 0, sizeof(spec));
spec.queue = (char *)"normal";
spec.user = (char *)"tester";
spec.mintasks = 1;
spec.maxtasks = 1;
spec.job_name = (char *)"first-boot-fence";
uint8_t *payload = NULL;
size_t len = 0;
int enc_rc = trf_encode_job_submit(base_jobid, array_size, &spec,
&payload, &len);
/* Physically append ONE committed-shape array entry, then tear down
* without ever applying it -- mirrors the sibling test's phase 1,
* simulating a genuinely fresh, never-recovered on-disk WAL. */
{
persist_config_t cfg1 = persist_config_defaults();
cfg1.wal_sync_ms = 5000;
persist_t *p1 = calloc(1, sizeof(persist_t));
TEST_ASSERT_NOT_NULL(p1);
persist_set_workdir(p1, tmp, "ctrl-test");
TEST_ASSERT_EQUAL_INT(0, persist_init(p1, &cfg1, NULL));
TEST_ASSERT_EQUAL_INT(0, persist_start(p1));
TEST_ASSERT_EQUAL_INT(0,
persist_wal_async(p1, WAL_JOB_ARRAY_SUBMIT, payload,
(uint32_t)len));
TEST_ASSERT_EQUAL_INT(0, persist_flush(p1));
persist_stop(p1);
persist_fini(p1);
free(p1);
}
trf_build_cluster();
persist_config_t cfg2 = persist_config_defaults();
cfg2.wal_sync_ms = 5000;
persist_set_workdir(&g_trf_c.persist, tmp, "ctrl-test");
TEST_ASSERT_EQUAL_INT(0,
persist_init(&g_trf_c.persist, &cfg2, &g_trf_c));
TEST_ASSERT_EQUAL_INT(0, persist_start(&g_trf_c.persist));
/* persist_init already defaults recovery_verdict=RECOVER_CLEAN and
* apply_verdict=APPLY_CLEAN (persist.c:613,623) -- set explicitly anyway
* to mirror the sibling test's defensive style, not because a fresh
* persist_t needs it. */
g_trf_c.persist.recovery_verdict = RECOVER_CLEAN;
job_array_apply_test_clear_force_fail();
job_array_apply_test_set_force_fail_at(fail_index);
/* Precondition: this persist_t has NEVER been recovered -- the fence
* must read clear, exactly the state the real eligibility gate
* (ha.c:1337) sees at a process's very first election. This is the
* blind spot bug-1445 exploits. */
bool fenced_precondition = persist_apply_resource_fenced(&g_trf_c.persist);
ha_config_t hcfg = {
.heartbeat_interval_ms = 3000,
.heartbeat_timeout_ms = 10000,
.election_timeout_ms = 2000,
.probe_connect_timeout_ms = 500,
.max_promotion_retries = 5,
};
TEST_ASSERT_EQUAL_INT(0, ha_init(&g_trf_c.ha, &hcfg, 0));
g_trf_c.ha.raft_c = &g_trf_c;
g_trf_c.ha.repl = NULL; /* no healthy peer reachable at this level */
TEST_ASSERT_EQUAL_INT(0,
delaymgr_init(&g_trf_c.delaymgr, 5, trf_run_delay_task));
TEST_ASSERT_EQUAL_INT(0, delaymgr_start(&g_trf_c.delaymgr));
ha_set_promotion_fence_window_ms(60);
/* The crux of the reproduction: ha->phase2_done is 0 (fresh ha_init
* above), so raft_cb_become_leader's is_first_boot block fires and
* calls this hook -- which calls persist_recover() for real, from
* INSIDE this same election evaluation, racing the gate exactly like
* production. */
ha_phase2_hook = trf_phase2_hook_calls_persist_recover;
raft_cb_become_leader_with_cluster_for_test(&g_trf_c, /*term=*/1);
ha_phase2_hook = NULL; /* restore -- shared process-global test seam */
int32_t role_after_election =
atomic_load_explicit(&g_trf_c.ha.role, memory_order_acquire);
int in_catch_up_after_election =
atomic_load_explicit(&g_trf_c.ha.in_catch_up, memory_order_acquire);
bool fenced_after_election =
persist_apply_resource_fenced(&g_trf_c.persist);
uint64_t applied_after_election =
persist_last_applied_lsn(&g_trf_c.persist);
bool any_element_visible = false;
for (uint64_t i = 0; i < array_size; i++) {
if (trf_find_job(base_jobid + i) != NULL)
any_element_visible = true;
}
ha_set_promotion_fence_window_ms(0); /* restore default */
ha_cancel_promotion_fence_timer(&g_trf_c.ha);
delaymgr_stop(&g_trf_c.delaymgr);
delaymgr_fini(&g_trf_c.delaymgr);
ha_fini(&g_trf_c.ha);
job_array_apply_test_clear_force_fail();
free(payload);
persist_stop(&g_trf_c.persist);
persist_fini(&g_trf_c.persist);
trf_teardown_cluster();
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);
/* ── Assertions ───────────────────────────────────────────────────── */
TEST_ASSERT_EQUAL_INT(0, enc_rc);
TEST_ASSERT_FALSE_MESSAGE(fenced_precondition,
"a never-recovered persist_t must start with the fence CLEAR -- "
"this is the exact blind spot bug-1445 exploits");
TEST_ASSERT_EQUAL_UINT64_MESSAGE(0, applied_after_election,
"the first-ever replay (triggered from inside the election path) "
"must halt at the failed LSN, same as any other halt-and-fence");
TEST_ASSERT_TRUE_MESSAGE(fenced_after_election,
"the node's OWN first-ever replay, triggered from inside this SAME "
"election evaluation, must have engaged the Live Apply Resource "
"Fence by the time this call returns");
TEST_ASSERT_FALSE_MESSAGE(any_element_visible,
"no partial/divergent set of array elements may be constructed "
"(Req 9.6)");
TEST_ASSERT_NOT_EQUAL_MESSAGE((int32_t)CTRL_PRIMARY, role_after_election,
"bug-1445: a node whose OWN first-ever replay (racing this SAME "
"election evaluation) engages the Live Apply Resource Fence must "
"be refused promotion -- not silently become CTRL_PRIMARY on a "
"gate decision that was made before that first replay ever ran");
TEST_ASSERT_EQUAL_INT_MESSAGE(1, in_catch_up_after_election,
"a node refused for this reason must be fenced via the SAME "
"existing in_catch_up mechanism every other refusal reason uses "
"-- no separate mechanism, no drift");
printf("test_first_boot_promotion_evaluates_own_first_replay_fence: "
"PASS\n");
}
#endif /* XCE_TESTING */
void test_job_array_replay_fence(void)
{
#ifdef XCE_TESTING
RUN_TEST(test_replay_wal_engages_fence_on_resource_exhaustion_failure);
RUN_TEST(test_first_boot_promotion_evaluates_own_first_replay_fence);
#else
printf("test_job_array_replay_fence: skipped (no XCE_TESTING)\n");
#endif