From 722fee5323662784eccfa51bf3f2c0b6c347375b Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Tue, 16 Jun 2026 04:24:23 +0000 Subject: [PATCH] feat(ha-prevote): implement pure voter-side PreVote decision (task 2.1) raft_on_pre_vote_request: deny on stale term, else reuse on_vote_eligible (primary/heartbeat-stickiness/catch-up gates). Never mutates term/voted_for/ role and never persists or adopts a higher term. 6 RED-first raft unit tests assert the purity invariant; libxce suite green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .kiro/specs/ha-prevote/tasks.md | 2 +- src/lib/raft/raft.c | 39 ++++++ src/lib/testsuite/test_raft.c | 209 ++++++++++++++++++++++++++++++++ 3 files changed, 249 insertions(+), 1 deletion(-) diff --git a/.kiro/specs/ha-prevote/tasks.md b/.kiro/specs/ha-prevote/tasks.md index f72f0d9c..307b8c84 100644 --- a/.kiro/specs/ha-prevote/tasks.md +++ b/.kiro/specs/ha-prevote/tasks.md @@ -16,7 +16,7 @@ - _Boundary: raft pre-vote functions_ - [ ] 2. Core: raft-library pure PreVote decision and tally (test-first, no side effects) -- [ ] 2.1 Implement the voter-side PreVote decision +- [x] 2.1 Implement the voter-side PreVote decision - RED first: add raft unit tests asserting grant for a fresh, higher-or-equal-term candidate when the eligibility callback allows it, and deny on stale term or when the eligibility callback rejects. - Reuse the existing vote-eligibility callback (primary-deny, fresh-heartbeat stickiness, catch-up deny) and let the dispatcher apply freshness beforehand, exactly as the real-vote path does. - Observable: after a granted PreVote the test confirms term, voted-for, and role are all unchanged and no persist callback fired. diff --git a/src/lib/raft/raft.c b/src/lib/raft/raft.c index d909a9b5..fbef39db 100644 --- a/src/lib/raft/raft.c +++ b/src/lib/raft/raft.c @@ -190,6 +190,45 @@ int raft_on_vote_request(raft_state_t *s, int from_idx, uint64_t term, return RAFT_GRANT; } +int raft_on_pre_vote_request(raft_state_t *s, int from_idx, uint64_t cand_term) +{ + /* PreVote is a side-effect-free straw poll: it answers "would you vote for + * me in cand_term?" WITHOUT changing any persistent state. Unlike + * raft_on_vote_request, this function never adopts cand_term (even when + * higher — that adoption is the very thing PreVote exists to prevent), never + * touches term / voted_for / voted_for_term / role, and never calls + * on_persist. The dispatcher applies the freshness floor before this call, + * exactly as on the real-vote path. */ + + /* Step 1: stale term — deny (mirrors raft_on_vote_request Step 1). A + * candidate proposing a term we already passed could never win a real + * election, so it cannot win a PreVote either. No term adoption: even when + * cand_term > s->term we only evaluate, never adopt. */ + if (cand_term < s->term) { + if (s->cbs->on_trace) + s->cbs->on_trace(s->ctx, "pre_vote_denied_stale_term", NULL); + return RAFT_DENY; + } + + /* Step 2: application-level eligibility gate — reuse the EXISTING + * on_vote_eligible callback (primary-deny / fresh-heartbeat stickiness / + * catch-up deny), identical to raft_on_vote_request Step 5. If NULL, every + * Raft-eligible candidate is granted. */ + if (s->cbs->on_vote_eligible != NULL) { + if (s->cbs->on_vote_eligible(s->ctx, from_idx, cand_term) == 0) { + if (s->cbs->on_trace) + s->cbs->on_trace(s->ctx, "pre_vote_denied_eligibility", NULL); + return RAFT_DENY; + } + } + + /* Grant — record NOTHING. No voted_for write (so a later real vote in this + * term is still grantable), no persist, no term/role change. */ + if (s->cbs->on_trace) + s->cbs->on_trace(s->ctx, "pre_vote_granted", NULL); + return RAFT_GRANT; +} + int raft_on_vote_reply(raft_state_t *s, int from_idx, int granted, uint64_t term) { (void)from_idx; diff --git a/src/lib/testsuite/test_raft.c b/src/lib/testsuite/test_raft.c index 12faf0fe..082b9275 100644 --- a/src/lib/testsuite/test_raft.c +++ b/src/lib/testsuite/test_raft.c @@ -2920,6 +2920,207 @@ static void test_proto_G2_partition_minority_no_quorum(void) #endif /* XCE_TESTING */ +/* ---------- raft_on_pre_vote_request tests (task 2.1) ---------- */ + +/* + * raft_on_pre_vote_request is the voter-side PreVote decision. It mirrors + * raft_on_vote_request's Step 1 (stale-term deny) and Step 5 (on_vote_eligible + * gate) but strips ALL persistent mutation: it must never touch term, + * voted_for, voted_for_term, role, and must never call on_persist. Freshness is + * applied by the dispatcher BEFORE this call (same contract as the real vote). + */ + +/* + * test_pre_vote_req_grant_higher_term_eligible + * + * Node at term=5 (FOLLOWER, no vote), receives PreVote for cand_term=6 with the + * eligibility callback granting (return 1). Expected: grant (1), and term / + * voted_for / voted_for_term / role all unchanged, on_persist NOT called. + */ +static void test_pre_vote_req_grant_higher_term_eligible(void) +{ + raft_state_t s; + raft_callbacks_t cbs = make_mock_cbs(); + + cbs.on_trace = mock_trace; + cbs.on_vote_eligible = mock_vote_eligible; + reset_mock(); + g_mock.vote_eligible_return_value = 1; /* eligibility grants */ + + TEST_ASSERT_EQUAL_INT(0, init_vote_node(&s, &cbs, 3, 0, 5)); + + int rc = raft_on_pre_vote_request(&s, 1, 6); + + TEST_ASSERT_EQUAL_INT(RAFT_GRANT, rc); + + /* PURITY: nothing persistent may change on a granted PreVote */ + TEST_ASSERT_EQUAL_UINT64(5, s.term); + TEST_ASSERT_EQUAL_INT(-1, s.voted_for); + TEST_ASSERT_EQUAL_UINT64(0, s.voted_for_term); + TEST_ASSERT_EQUAL_INT(RAFT_FOLLOWER, s.role); + TEST_ASSERT_EQUAL_INT(0, g_mock.persist_count); + + /* eligibility gate was consulted */ + TEST_ASSERT_EQUAL_INT(1, g_mock.vote_eligible_call_count); +} + +/* + * test_pre_vote_req_grant_equal_term_eligible + * + * Equal-term candidate (cand_term == s->term) must NOT be denied by the + * stale-term gate (deny is only for cand_term < term), and is granted when + * eligibility allows. Purity invariant still holds. + */ +static void test_pre_vote_req_grant_equal_term_eligible(void) +{ + raft_state_t s; + raft_callbacks_t cbs = make_mock_cbs(); + + cbs.on_trace = mock_trace; + cbs.on_vote_eligible = mock_vote_eligible; + reset_mock(); + g_mock.vote_eligible_return_value = 1; + + TEST_ASSERT_EQUAL_INT(0, init_vote_node(&s, &cbs, 3, 0, 5)); + + int rc = raft_on_pre_vote_request(&s, 1, 5); /* equal term */ + + TEST_ASSERT_EQUAL_INT(RAFT_GRANT, rc); + + TEST_ASSERT_EQUAL_UINT64(5, s.term); + TEST_ASSERT_EQUAL_INT(-1, s.voted_for); + TEST_ASSERT_EQUAL_UINT64(0, s.voted_for_term); + TEST_ASSERT_EQUAL_INT(RAFT_FOLLOWER, s.role); + TEST_ASSERT_EQUAL_INT(0, g_mock.persist_count); +} + +/* + * test_pre_vote_req_deny_stale_term + * + * Node at term=5, receives PreVote for cand_term=4 (< term). Expected: deny (0) + * WITHOUT consulting on_vote_eligible, no term-adoption, no persist. + */ +static void test_pre_vote_req_deny_stale_term(void) +{ + raft_state_t s; + raft_callbacks_t cbs = make_mock_cbs(); + + cbs.on_trace = mock_trace; + cbs.on_vote_eligible = mock_vote_eligible; + reset_mock(); + g_mock.vote_eligible_return_value = 1; /* would grant if consulted */ + + TEST_ASSERT_EQUAL_INT(0, init_vote_node(&s, &cbs, 3, 0, 5)); + + int rc = raft_on_pre_vote_request(&s, 1, 4); /* stale */ + + TEST_ASSERT_EQUAL_INT(RAFT_DENY, rc); + + /* Stale-term denial short-circuits BEFORE the eligibility gate. */ + TEST_ASSERT_EQUAL_INT(0, g_mock.vote_eligible_call_count); + + /* PURITY: term must NOT be adopted, no persist. */ + TEST_ASSERT_EQUAL_UINT64(5, s.term); + TEST_ASSERT_EQUAL_INT(-1, s.voted_for); + TEST_ASSERT_EQUAL_UINT64(0, s.voted_for_term); + TEST_ASSERT_EQUAL_INT(RAFT_FOLLOWER, s.role); + TEST_ASSERT_EQUAL_INT(0, g_mock.persist_count); +} + +/* + * test_pre_vote_req_deny_eligibility_rejects + * + * Fresh, higher-term candidate but the eligibility callback rejects (returns 0) + * — e.g. primary-deny / fresh-heartbeat stickiness / catch-up. Expected: deny, + * with the eligibility gate consulted, and the purity invariant preserved. + */ +static void test_pre_vote_req_deny_eligibility_rejects(void) +{ + raft_state_t s; + raft_callbacks_t cbs = make_mock_cbs(); + + cbs.on_trace = mock_trace; + cbs.on_vote_eligible = mock_vote_eligible; + reset_mock(); + g_mock.vote_eligible_return_value = 0; /* eligibility denies */ + + TEST_ASSERT_EQUAL_INT(0, init_vote_node(&s, &cbs, 3, 0, 5)); + + int rc = raft_on_pre_vote_request(&s, 1, 6); + + TEST_ASSERT_EQUAL_INT(RAFT_DENY, rc); + TEST_ASSERT_EQUAL_INT(1, g_mock.vote_eligible_call_count); + + TEST_ASSERT_EQUAL_UINT64(5, s.term); + TEST_ASSERT_EQUAL_INT(-1, s.voted_for); + TEST_ASSERT_EQUAL_UINT64(0, s.voted_for_term); + TEST_ASSERT_EQUAL_INT(RAFT_FOLLOWER, s.role); + TEST_ASSERT_EQUAL_INT(0, g_mock.persist_count); +} + +/* + * test_pre_vote_req_null_eligible_grants + * + * When on_vote_eligible is NULL, a fresh higher-or-equal-term candidate is + * granted (all Raft-eligible candidates granted), with no mutation/persist. + */ +static void test_pre_vote_req_null_eligible_grants(void) +{ + raft_state_t s; + raft_callbacks_t cbs = make_mock_cbs(); + + cbs.on_trace = mock_trace; + /* on_vote_eligible deliberately left NULL */ + reset_mock(); + + TEST_ASSERT_EQUAL_INT(0, init_vote_node(&s, &cbs, 3, 0, 5)); + + int rc = raft_on_pre_vote_request(&s, 1, 6); + + TEST_ASSERT_EQUAL_INT(RAFT_GRANT, rc); + + TEST_ASSERT_EQUAL_UINT64(5, s.term); + TEST_ASSERT_EQUAL_INT(-1, s.voted_for); + TEST_ASSERT_EQUAL_UINT64(0, s.voted_for_term); + TEST_ASSERT_EQUAL_INT(RAFT_FOLLOWER, s.role); + TEST_ASSERT_EQUAL_INT(0, g_mock.persist_count); +} + +/* + * test_pre_vote_req_grant_preserves_existing_vote (Req 2.4) + * + * A node that has already voted for someone in the current term may still GRANT + * a PreVote (PreVote does not consume / change the recorded vote). Confirms the + * existing voted_for / voted_for_term are untouched by a granted PreVote. + */ +static void test_pre_vote_req_grant_preserves_existing_vote(void) +{ + raft_state_t s; + raft_callbacks_t cbs = make_mock_cbs(); + + cbs.on_trace = mock_trace; + cbs.on_vote_eligible = mock_vote_eligible; + reset_mock(); + g_mock.vote_eligible_return_value = 1; + + TEST_ASSERT_EQUAL_INT(0, init_vote_node(&s, &cbs, 3, 0, 5)); + /* Node already cast its real vote for peer 1 in term 5. */ + s.voted_for = 1; + s.voted_for_term = 5; + + /* A different candidate (peer 2) probes a PreVote for term 6. */ + int rc = raft_on_pre_vote_request(&s, 2, 6); + + TEST_ASSERT_EQUAL_INT(RAFT_GRANT, rc); + + /* The recorded real vote must be untouched (no consume, no overwrite). */ + TEST_ASSERT_EQUAL_INT(1, s.voted_for); + TEST_ASSERT_EQUAL_UINT64(5, s.voted_for_term); + TEST_ASSERT_EQUAL_UINT64(5, s.term); + TEST_ASSERT_EQUAL_INT(RAFT_FOLLOWER, s.role); + TEST_ASSERT_EQUAL_INT(0, g_mock.persist_count); +} + /* ---------- test_raft entry point ---------- */ void test_raft(void) @@ -2974,6 +3175,14 @@ void test_raft(void) RUN_TEST(test_check_and_start_not_voted); RUN_TEST(test_check_and_start_voted_for_self); + /* raft_on_pre_vote_request (task 2.1) */ + RUN_TEST(test_pre_vote_req_grant_higher_term_eligible); + RUN_TEST(test_pre_vote_req_grant_equal_term_eligible); + RUN_TEST(test_pre_vote_req_deny_stale_term); + RUN_TEST(test_pre_vote_req_deny_eligibility_rejects); + RUN_TEST(test_pre_vote_req_null_eligible_grants); + RUN_TEST(test_pre_vote_req_grant_preserves_existing_vote); + /* simulator init (task 4.1) */ #ifdef XCE_TESTING RUN_TEST(test_sim_init_3node);