test(raft-ha-core-safety): fix 2 stale tests surfaced by first full-suite ASAN sweep (task 7.1)
A full 69-group sweep of controller_unit_test (this spec's harness had never been run end-to-end before) found 2 deterministic failures, both test-only staleness against already-shipped, already-approved production behavior from earlier in this same spec: - test_ha_vote_gate: test_pre_vote_catch_up_denies asserted the pre-task-5.3 unconditional in_catch_up PreVote deny. Task 5.3 (bf88681) deliberately gated that deny on primary_ctrl_fd>=0 && ha_heartbeat_is_fresh to fix a real round-4 election-stall — this test's exact scenario (stale heartbeat, primary_ctrl_fd=-1) is the case 5.3 intentionally flipped to grant. Split into test_pre_vote_catch_up_grants_when_primary_unreachable (matches the current contract) and test_pre_vote_catch_up_denies_while_primary_live (preserves the original protective intent, correctly scoped). - test_egress_filter: test_live_submit_tags_submit_lsn called the JOB_NEW persist_job_hook_fun AFTER_EVENT hook directly, but task 5.9 (15be5d9, bug-1306 fix) made that hook's WAL_MODE_WAIT branch notify-only, moving the real WAL append + commit barrier into persist_submit_wal_and_commit(). 5.9 relocated 4 equivalent tests in test_persist_hooks.c but missed this one. Now drives persist_submit_wal_and_commit() directly, mirroring that pattern. No production code changed. Full 69-group suite: 0 Failures, 0 ASAN reports (independently reproduced 3x: initial sweep, implementer, reviewer).
This commit is contained in:
parent
9344b8ed60
commit
7eefe769a3
3 changed files with 91 additions and 38 deletions
|
|
@ -195,7 +195,7 @@
|
|||
- _Depends: 2.4, 3.2, 4.2_
|
||||
|
||||
- [ ] 7. Validation: regression and acceptance soak gate
|
||||
- [ ] 7.1 Full controller suite under ASAN with the new harness green
|
||||
- [x] 7.1 Full controller suite under ASAN with the new harness green
|
||||
- Build and run the full controller unit suite under ASAN (suite-by-suite if the aggregate hits the known 300s cap); fix any regression from the ingest/persist/election changes
|
||||
- Observable completion: the full suite is green under ASAN and every `test_ha_core_safety` case passes with no ASAN errors
|
||||
- _Requirements: 7.4_
|
||||
|
|
|
|||
|
|
@ -295,29 +295,21 @@ static int ef_encode_job(const xce_job_t *j, uint8_t **out, size_t *out_len)
|
|||
return 0;
|
||||
}
|
||||
|
||||
/* Fire a job persistence hook through a real va_list (the hook reads its args
|
||||
* via va_arg, mirroring test_persist_hooks.c's call_job_hook). */
|
||||
static int call_job_hook(int detail, persist_t *p, ...)
|
||||
{
|
||||
va_list ap;
|
||||
va_start(ap, p);
|
||||
int rc = persist_job_hook_fun(HOOK_AFTER_EVENT, HOOK_AFTER_EVENT, detail,
|
||||
p, ap);
|
||||
va_end(ap);
|
||||
return rc;
|
||||
}
|
||||
|
||||
/* JOB_NEW AFTER_EVENT args: (cluster_t*, xlist_t* jlist). */
|
||||
static int call_job_hook_new(persist_t *p, cluster_t *c, xlist_t *jlist)
|
||||
{
|
||||
return call_job_hook(HOOK_DETAIL_JOB_NEW, p, c, jlist);
|
||||
}
|
||||
|
||||
/* ── Tests ──────────────────────────────────────────────────────────────── */
|
||||
|
||||
/* 3.3 (live tag): firing the JOB_NEW persistence hook for a submitted job
|
||||
* must tag the job_t with the LSN of the WAL_JOB_SUBMIT entry it wrote.
|
||||
* RED: persist_job_hook_fun does not set submit_lsn yet. */
|
||||
/* 3.3 (live tag): submitting a job must tag the job_t with the LSN of the
|
||||
* WAL_JOB_SUBMIT entry it wrote.
|
||||
*
|
||||
* Durability for WAL_JOB_SUBMIT was moved OUT of the JOB_NEW persistence hook
|
||||
* into the explicit raft step persist_submit_wal_and_commit() by
|
||||
* raft-ha-core-safety task 5.9 (bug-1306: xce_run_hooks returns only the
|
||||
* LAST-registered hook's rc, so a later JOB AFTER_EVENT subscriber's 0 could
|
||||
* clobber this hook's commit -ETIMEDOUT and ack an uncommitted entry). The
|
||||
* JOB_NEW hook is now notification-only (pinned by
|
||||
* test_persist_hooks.c:test_job_hook_new_is_notify_only) and performs no WAL
|
||||
* append, so this test now drives the explicit step directly — mirroring
|
||||
* test_persist_hooks.c:test_job_hook_new_writes_wal_entry, which was
|
||||
* relocated the same way when 15be5d9 landed. */
|
||||
static void test_live_submit_tags_submit_lsn(void)
|
||||
{
|
||||
ef_setup();
|
||||
|
|
@ -326,18 +318,19 @@ static void test_live_submit_tags_submit_lsn(void)
|
|||
|
||||
uint64_t lsn_before = g_c.persist.current_lsn;
|
||||
|
||||
/* JOB_NEW AFTER_EVENT args: (cluster_t*, xlist_t* jlist) */
|
||||
void *jptr = j;
|
||||
xlist_t jlist = { .count = 1, .caps = 1, .delta = 1, .data = &jptr };
|
||||
|
||||
/* The JOB_NEW hook does a SYNC append; with no real standby ACKs in this
|
||||
* harness it returns -ETIMEDOUT, but the LOCAL WAL write still succeeds and
|
||||
* the LSN advances (production keeps the entry on replication-degraded
|
||||
* timeout). The tagging contract is independent of replication status, so
|
||||
* we require only that the local append happened. */
|
||||
int rc = call_job_hook_new(&g_c.persist, &g_c, &jlist);
|
||||
TEST_ASSERT_TRUE_MESSAGE(rc == 0 || rc == -ETIMEDOUT || rc == -ENODEV,
|
||||
"JOB_NEW local append should have succeeded");
|
||||
/* persist_submit_wal_and_commit does a SYNC append; with no real standby
|
||||
* ACKs in this harness it returns -ETIMEDOUT, but the LOCAL WAL write
|
||||
* still succeeds and the LSN advances (production keeps the entry on
|
||||
* replication-degraded timeout). The tagging contract is independent of
|
||||
* replication status, so we require only that the local append happened. */
|
||||
int rc = persist_submit_wal_and_commit(&g_c.persist, &jlist);
|
||||
TEST_ASSERT_TRUE_MESSAGE(
|
||||
rc == 0 || rc == -ETIMEDOUT || rc == -XCE_ERR_NOT_READY ||
|
||||
rc == -ENODEV,
|
||||
"JOB_NEW local append should have succeeded");
|
||||
|
||||
uint64_t appended_lsn = g_c.persist.current_lsn - 1;
|
||||
TEST_ASSERT_GREATER_THAN_UINT64(lsn_before, g_c.persist.current_lsn);
|
||||
|
|
|
|||
|
|
@ -601,9 +601,26 @@ static void test_pre_vote_standby_fresh_heartbeat_denies(void)
|
|||
printf("test_pre_vote_standby_fresh_heartbeat_denies: PASS\n");
|
||||
}
|
||||
|
||||
/* ── Req 3.1/3.2: catch-up standby denies a PreVote ────────────────────────── */
|
||||
/* ── Req 3.1/3.2 (post raft-ha-core-safety task 5.3): catch-up standby vote
|
||||
* gate is scoped to a LIVE primary, not unconditional ──────────────────────
|
||||
*
|
||||
* commit bf88681 (task 5.3, bug-1272) deliberately relaxed the in_catch_up
|
||||
* deny in raft_cb_vote_eligible (ha.c): it now denies ONLY while a primary is
|
||||
* actually LIVE (primary_ctrl_fd>=0 AND ha_heartbeat_is_fresh) feeding this
|
||||
* node. Once the primary is unreachable, a catching-up survivor that meets
|
||||
* the freshness floor MUST be grantable — otherwise two lagging survivors
|
||||
* whose primary just died mutually deny every PreVote forever (the round-4
|
||||
* election-stall this task fixed; see test_ha_core_safety.c's
|
||||
* test_5_3_catchup_survivor_grants_fresh_candidate_when_primary_gone /
|
||||
* test_5_3_catchup_denies_while_live_primary_feeds, which pin the same
|
||||
* contract against the real raft_on_pre_vote_request path). The two cases
|
||||
* below split what used to be a single unconditional-deny test. */
|
||||
|
||||
static void test_pre_vote_catch_up_denies(void)
|
||||
/* Was test_pre_vote_catch_up_denies — that name asserted the PRE-5.3
|
||||
* unconditional deny, which 5.3 deliberately relaxed for exactly this
|
||||
* scenario (primary unreachable + candidate meets the floor). Renamed to
|
||||
* assert the current, intended contract instead of reverting the fix. */
|
||||
static void test_pre_vote_catch_up_grants_when_primary_unreachable(void)
|
||||
{
|
||||
ha_state_t ha;
|
||||
ha_init(&ha, &g_gate_cfg, 0);
|
||||
|
|
@ -611,8 +628,47 @@ static void test_pre_vote_catch_up_denies(void)
|
|||
atomic_store_explicit(&ha.term, 4ULL, memory_order_release);
|
||||
ha.raft.term = 4ULL;
|
||||
ha.voted_for_term = 0;
|
||||
ha.primary_ctrl_fd = -1;
|
||||
set_last_activity_age(&ha, 60000L); /* stale heartbeat → not sticky */
|
||||
ha.primary_ctrl_fd = -1; /* primary is gone — nothing to catch up FROM */
|
||||
set_last_activity_age(&ha, 60000L); /* stale heartbeat → not sticky either */
|
||||
atomic_store_explicit(&ha.in_catch_up, 1, memory_order_release);
|
||||
|
||||
prevote_state_snapshot_t before;
|
||||
snapshot_prevote_state(&ha, &before);
|
||||
|
||||
uint64_t reply_term = 0;
|
||||
int granted = pre_vote_dispatch_decide(&ha,
|
||||
5ULL,
|
||||
100ULL, 50ULL, /* candidate meets the floor */
|
||||
100ULL, 50ULL,
|
||||
0, &reply_term, NULL, NULL);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(1, granted,
|
||||
"a catching-up standby with NO live primary must GRANT a PreVote from a "
|
||||
"candidate that meets the freshness floor (task 5.3 / bug-1272) — the "
|
||||
"in_catch_up deny is scoped to a LIVE primary, not unconditional");
|
||||
TEST_ASSERT_EQUAL_UINT64_MESSAGE(4ULL, reply_term,
|
||||
"granted PreVote reply carries the voter's UNCHANGED term (no adoption)");
|
||||
assert_prevote_state_unchanged(&ha, &before);
|
||||
|
||||
ha_fini(&ha);
|
||||
printf("test_pre_vote_catch_up_grants_when_primary_unreachable: PASS\n");
|
||||
}
|
||||
|
||||
/* Companion case preserving the ORIGINAL Req 3.x protective intent, correctly
|
||||
* scoped: in_catch_up while a primary is ACTIVELY feeding this node (live fd +
|
||||
* fresh heartbeat) must still deny — task 5.3 only lifts the deny once the
|
||||
* primary is gone; it must never disrupt a primary that is alive and
|
||||
* replicating. */
|
||||
static void test_pre_vote_catch_up_denies_while_primary_live(void)
|
||||
{
|
||||
ha_state_t ha;
|
||||
ha_init(&ha, &g_gate_cfg, 0);
|
||||
atomic_store_explicit(&ha.role, (int32_t)CTRL_STANDBY, memory_order_release);
|
||||
atomic_store_explicit(&ha.term, 4ULL, memory_order_release);
|
||||
ha.raft.term = 4ULL;
|
||||
ha.voted_for_term = 0;
|
||||
ha.primary_ctrl_fd = 999; /* a live primary connection */
|
||||
set_last_activity_age(&ha, 500L); /* fresh heartbeat (< 10 s) */
|
||||
atomic_store_explicit(&ha.in_catch_up, 1, memory_order_release);
|
||||
|
||||
prevote_state_snapshot_t before;
|
||||
|
|
@ -626,13 +682,16 @@ static void test_pre_vote_catch_up_denies(void)
|
|||
0, &reply_term, NULL, NULL);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(0, granted,
|
||||
"a catching-up standby must deny PreVote (Req 3.x)");
|
||||
"a catching-up standby with a LIVE primary still feeding it must DENY "
|
||||
"PreVote (Req 3.x) — the 5.3 relaxation applies ONLY once the primary "
|
||||
"is unreachable");
|
||||
TEST_ASSERT_EQUAL_UINT64_MESSAGE(4ULL, reply_term,
|
||||
"denied PreVote reply carries the voter's UNCHANGED term (no adoption)");
|
||||
assert_prevote_state_unchanged(&ha, &before);
|
||||
|
||||
ha.primary_ctrl_fd = -1; /* defensive: never let ha_fini touch fd 999 */
|
||||
ha_fini(&ha);
|
||||
printf("test_pre_vote_catch_up_denies: PASS\n");
|
||||
printf("test_pre_vote_catch_up_denies_while_primary_live: PASS\n");
|
||||
}
|
||||
|
||||
/* ── Req 2.2: stale-freshness candidate denied even when stickiness lapsed ──── */
|
||||
|
|
@ -1093,7 +1152,8 @@ void test_ha_vote_gate(void)
|
|||
* no mutation, reply term unchanged) */
|
||||
RUN_TEST(test_pre_vote_primary_role_denies);
|
||||
RUN_TEST(test_pre_vote_standby_fresh_heartbeat_denies);
|
||||
RUN_TEST(test_pre_vote_catch_up_denies);
|
||||
RUN_TEST(test_pre_vote_catch_up_grants_when_primary_unreachable);
|
||||
RUN_TEST(test_pre_vote_catch_up_denies_while_primary_live);
|
||||
RUN_TEST(test_pre_vote_stale_freshness_denies);
|
||||
RUN_TEST(test_pre_vote_stale_term_denies_no_adoption);
|
||||
RUN_TEST(test_pre_vote_granted_when_stickiness_lapsed_and_fresh);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue