wdgzyjx/myxce
1
0
Fork 0
forked from lei.chen/myxce
Code Pull requests Activity

test(raft-adapter-conformance): PreVote post-connect failure routing + fast-quorum coverage

Browse source 
Adds pre_vote_ep_err_handler_for_test (XCE_TESTING-guarded) and two
tests: PreVote post-connect failure reaches pre_vote_reply_dispatch's
tally (Req 4.3), and a round mixing that failure mode with a delivered
reply reaches its quorum decision via the fast all-replied path
without the full round timeout (Req 4.5). Real-vote coverage (Req 4.2)
was already proven by task 3.1's own test, cited not duplicated.

pre_vote_reply_dispatch has no mock hook seam, so both tests assert
directly on raft_state_t's tally fields, mirroring test_ha_prevote.c's
existing verification technique.

1061 -> 1063 tests, 0 failures.
This commit is contained in:
Rongsong Shen 2026-07-29 14:38:36 +00:00
parent b315ffe466
commit 62db70d16d
3 changed files with 187 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

4
.kiro/specs/raft-adapter-conformance/tasks.md
View file

@ -40,7 +40,7 @@
- _Requirements: 4.2, 4.3, 4.4_
- _Boundary: ha.c vote endpoint handlers_
- [ ] 3.2 Test post-connect vote and pre-vote failure routing
- [x] 3.2 Test post-connect vote and pre-vote failure routing
- Extend the existing test-construction helper (`vote_ep_ctx_build_for_test`) to synthesize a post-connect failure on a real-vote endpoint and assert it reaches `vote_reply_dispatch`'s tally
- Do the same for a PreVote endpoint against `pre_vote_reply_dispatch`
- Confirm a round where every peer's outcome is now counted (including this failure mode) reaches a quorum decision without waiting for the full round timeout
@ -119,3 +119,5 @@
- Task 2.2 (bug-1482): `myreq_handler`'s first unconditional step is `xce_decode(pkt, &request)`, which itself unconditionally `free_packet(pkt)` + frees the `io_packet_t` struct before returning on every path — `pkt` ownership transfers into `xce_decode`, the caller must never touch/free it afterward. Any new `_for_test` wrapper around `myreq_handler` (or `xce_decode` callers generally) must document this, not the reverse ("caller owns/frees pkt") — that wrong assumption is what caused the original test-only wrapper's doc comment to mislead a reviewer into requesting a double-free.
- Task 2.2: driving a test through `cluster.c`'s real `myreq_handler` (rather than testing extracted logic in isolation) requires adding `cluster.c` to `controller_unit_test_src` in `meson.build` plus link-satisfying fakes for symbols its OTHER unrelated switch-cases reference (task.c/opget.c/sched.c/apiserver.c/myconf.c) — this is pre-existing, established convention in this codebase (`test_worker_join.c`, `test_commit_barrier_reply.c` already do it for the same reason), not scope creep. Any new test driving `myreq_handler` end-to-end should expect this same footprint.
- New test-only wrappers exposing a `static` production dispatch function (the `_for_test` pattern, e.g. `peer_ctrl_msg_handler_for_test`, `myreq_handler_for_test`) MUST be wrapped in `#ifdef XCE_TESTING`/`#endif` — the source file compiles into the production binary too, and an unguarded wrapper ships as a new linked symbol in production with zero production purpose.
- Task 3.2 (bug-1483, not fixed): driving a PreVote round to `RAFT_PRE_VOTE_LOST` in a unit test (`c==NULL`) without first stubbing `ha_prevote_rearm_liveness_hook` falls through to the real `ha_arm_heartbeat_timeout_vtask`, which callocs into the static `s_liveness_ctx` and never frees it when `c==NULL` short-circuits the scheduling guard. `test_ha_prevote.c`'s existing LOST-path tests already stub-and-reset this hook — any new PreVote-round test that reaches LOST must do the same.
- `pre_vote_reply_dispatch` has no `XCE_TESTING` mock-hook seam (unlike `vote_reply_dispatch`'s `xce_vote_reply_hook`) — it calls `raft_on_pre_vote_reply` directly. Tests verifying PreVote routing must assert on `raft_state_t`'s own tally fields (`pre_vote_replies[]`, `pre_votes_granted`, `pre_vote_in_progress`) instead, per `test_ha_prevote.c`'s established technique.

13
src/controller/ha.c
View file

@ -7670,6 +7670,19 @@ static int pre_vote_ep_err_handler(void *ep_raw)
return 0;
}
#ifdef XCE_TESTING
/*
* pre_vote_ep_err_handler_for_test: call the static pre_vote_ep_err_handler on
* ep. Allows tests to invoke the PreVote err_handler without going through
* iomgr/endpoint machinery — the PreVote twin of vote_ep_err_handler_for_test
* (raft-adapter-conformance task 3.2).
*/
int pre_vote_ep_err_handler_for_test(endpoint_t *ep)
{
return pre_vote_ep_err_handler((void *)ep);
}
#endif /* XCE_TESTING */
/* ── ha_init_raft: one-time raft state machine initialization ────────────────
*
* Called once at startup after ha_restore_term() to wire the raft state

171
src/controller/testsuite/test_ha_vote_send.c
View file

@ -716,6 +716,174 @@ static void test_post_connect_vote_failure_routes_through_production_tally(void)
printf("test_post_connect_vote_failure_routes_through_production_tally: PASS\n");
}
/* ── raft-adapter-conformance task 3.2: post-connect PreVote failure routing ──
*
* pre_vote_ep_err_handler's election-phase branch (task 3.1) must count a
* post-connect PreVote endpoint failure through pre_vote_reply_dispatch --
* the SAME production tally path a connect-time failure or a delivered
* PreVote reply uses -- not a legacy no-op path. Requirement 4.3 (mirror of
* 4.2 for the PreVote endpoint kind).
*
* Test seam: pre_vote_reply_dispatch has no mock hook analogous to
* xce_vote_reply_hook (it calls raft_on_pre_vote_reply directly). Instead we
* assert directly on raft_state_t's own PreVote tally fields
* (pre_vote_replies[], pre_votes_granted, pre_vote_in_progress) -- the same
* fields test_ha_prevote.c already asserts on for the delivered-reply path
* (pre_vote_reply_dispatch_for_test). Pre-fix, pre_vote_ep_err_handler routed
* this failure through the legacy handle_request_vote_reply path, which never
* touches raft_on_pre_vote_reply or these fields -- so pre_vote_replies[1]
* would stay 0 and this test would fail against pre-fix code.
*/
extern int pre_vote_ep_err_handler_for_test(endpoint_t *ep);
extern void pre_vote_reply_dispatch_for_test(ha_state_t *ha, cluster_t *c,
int peer_idx, int granted);
/* A LOST round re-arms the liveness watchdog (ha_prevote_rearm_liveness).
* With the hook unset that falls through to the real
* ha_arm_heartbeat_timeout_vtask, which -- with c==NULL in unit-test mode --
* calloc's a ctx into the static s_liveness_ctx and never arms a delaymgr
* task to free it (delay_task_ms is gated on `c && c->delaymgr.running`),
* leaking it and leaving a stale pointer into this test's stack-local `ha`
* once the test returns. test_ha_prevote.c avoids this identical hazard on
* its own LOST-path tests via the same hook override -- mirrored here. */
extern int (*ha_prevote_rearm_liveness_hook)(ha_state_t *ha, cluster_t *c);
static int stub_prevote_rearm_liveness(ha_state_t *ha, cluster_t *c)
{
(void)ha; (void)c;
return 0;
}
/* Build a 3-node (self + 2 peers) PreVote round in progress, mirroring
* test_ha_prevote.c's make_three_node + ha_start_prevote_async(&ha, NULL)
* fixture: role stays CTRL_STANDBY (the real production role throughout a
* PreVote round -- a node only becomes CTRL_ELECTING after WINNING the
* round), self-grant is recorded by the real raft_begin_pre_vote, and both
* non-self peers (absolute index 1 and 2) start unreplied. */
static void start_three_node_prevote_round(ha_state_t *ha)
{
ha_do_demote(ha);
ha->raft.n_peers = 3;
ha->raft.self_idx = ha->my_priority; /* my_priority == 0 for this fixture */
ha->raft.role = RAFT_FOLLOWER;
ha->n_peers = 3;
ha_start_prevote_async(ha, NULL);
}
static void test_post_connect_prevote_failure_routes_through_production_tally(void)
{
ha_state_t ha;
TEST_ASSERT_EQUAL_INT(0, ha_init(&ha, &g_vote_cfg, 0));
atomic_store_explicit(&ha.term, 4ULL, memory_order_release);
start_three_node_prevote_round(&ha);
TEST_ASSERT_EQUAL_INT_MESSAGE(1, ha.raft.pre_vote_in_progress,
"round must be in progress before the failure arrives");
TEST_ASSERT_EQUAL_INT_MESSAGE(1, ha.raft.pre_votes_granted,
"self-grant must already be recorded by raft_begin_pre_vote");
ha_elect_ctx_t *ctx = alloc_ctx_for_vote(2);
pthread_mutex_init(&ctx->lock, NULL);
endpoint_t ep;
make_endpoint(&ep);
/* Build a PreVote endpoint context as if raft_cb_send_pre_vote_request had
* already connected it (on_vote_connected fired, request sent) -- then
* the connection fails before any reply arrives: iomgr invokes the
* registered err_handler (pre_vote_ep_err_handler) on this same ep. */
vote_ep_ctx_build_for_test(&ep, &ha, ctx, /*peer_idx=*/1,
4ULL, 100ULL, 50ULL, "ctrl-a");
int rc = pre_vote_ep_err_handler_for_test(&ep);
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_INT_MESSAGE(1, ha.raft.pre_vote_replies[1],
"post-connect PreVote failure must reach pre_vote_reply_dispatch's "
"tally (raft_on_pre_vote_reply), not a legacy no-op path");
TEST_ASSERT_EQUAL_INT_MESSAGE(1, ha.raft.pre_votes_granted,
"a post-connect PreVote failure must be counted as NOT granted "
"(pre_votes_granted stays at the self-grant only)");
TEST_ASSERT_EQUAL_INT_MESSAGE(1, ha.raft.pre_vote_in_progress,
"round stays pending -- only 1 of 2 non-self peers has been counted, "
"no quorum(2) or all-replied(2) condition met yet");
free(ep.data);
free_ctx_for_vote(ctx);
ha_fini(&ha);
printf("test_post_connect_prevote_failure_routes_through_production_tally: PASS\n");
}
/* ── raft-adapter-conformance task 3.2: fast quorum decision (Req 4.5) ────────
*
* Once every peer in a PreVote round has had its outcome counted -- whether
* via a delivered reply or via this post-connect failure mode -- the round
* must reach its quorum decision immediately (synchronously inside the
* counting call), without waiting for the round's full timeout
* (ha_prevote_round_timeout_fn is never invoked in this test). Mixes both
* counting paths in the same round to prove the failure path is a full
* first-class participant in the fast "all replied" decision, not merely
* counted without unblocking the round.
*/
static void test_prevote_round_reaches_quorum_decision_via_fast_tally(void)
{
ha_state_t ha;
TEST_ASSERT_EQUAL_INT(0, ha_init(&ha, &g_vote_cfg, 0));
atomic_store_explicit(&ha.term, 4ULL, memory_order_release);
start_three_node_prevote_round(&ha);
/* This test's mixed round decides LOST (below), which re-arms the
* liveness watchdog -- stub it out so the real (c==NULL) production path
* is never exercised here (see extern decl comment above). */
ha_prevote_rearm_liveness_hook = stub_prevote_rearm_liveness;
ha_elect_ctx_t *ctx = alloc_ctx_for_vote(2);
pthread_mutex_init(&ctx->lock, NULL);
/* Peer 1: post-connect endpoint failure -- counted via
* pre_vote_ep_err_handler (the fix under test). */
endpoint_t ep1;
make_endpoint(&ep1);
vote_ep_ctx_build_for_test(&ep1, &ha, ctx, /*peer_idx=*/1,
4ULL, 100ULL, 50ULL, "ctrl-a");
TEST_ASSERT_EQUAL_INT(0, pre_vote_ep_err_handler_for_test(&ep1));
TEST_ASSERT_EQUAL_INT_MESSAGE(1, ha.raft.pre_vote_in_progress,
"round must still be pending -- peer 2's outcome is not counted yet");
/* Peer 2: a normal delivered denial through the production reply-dispatch
* path -- the same tally the post-connect failure above also reaches. */
pre_vote_reply_dispatch_for_test(&ha, NULL, /*peer_idx=*/2, /*granted=*/0);
/* Every non-self peer's outcome is now counted (one via the post-connect
* failure path, one via a delivered reply) -- the round must reach its
* LOST decision immediately, without any call into
* ha_prevote_round_timeout_fn (Requirement 4.5's fast "all replied"
* path). */
TEST_ASSERT_EQUAL_INT_MESSAGE(0, ha.raft.pre_vote_in_progress,
"Req 4.5: once every peer outcome is counted -- including a "
"post-connect failure -- the round must reach its quorum decision "
"without waiting for the round's full timeout");
TEST_ASSERT_EQUAL_INT_MESSAGE(1, ha.raft.pre_votes_granted,
"neither non-self peer granted -- self-grant only, quorum(2) never "
"reached");
TEST_ASSERT_EQUAL_INT_MESSAGE(CTRL_STANDBY,
(int)atomic_load_explicit(&ha.role, memory_order_acquire),
"a LOST PreVote round leaves the node CTRL_STANDBY (Req 1.4, "
"unchanged by this fix)");
ha_prevote_rearm_liveness_hook = NULL; /* reset -- do not leak into other tests */
free(ep1.data);
free_ctx_for_vote(ctx);
ha_fini(&ha);
printf("test_prevote_round_reaches_quorum_decision_via_fast_tally: PASS\n");
}
/* ── Suite entry point ───────────────────────────────────────────────────── */
void test_ha_vote_send(void)
@ -734,4 +902,7 @@ void test_ha_vote_send(void)
RUN_TEST(test_bug1417_middle_index_real_vote_from_peer2_not_dropped);
/* raft-adapter-conformance task 3.1 */
RUN_TEST(test_post_connect_vote_failure_routes_through_production_tally);
/* raft-adapter-conformance task 3.2 */
RUN_TEST(test_post_connect_prevote_failure_routes_through_production_tally);
RUN_TEST(test_prevote_round_reaches_quorum_decision_via_fast_tally);
}