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

fix(raft): vote-reply per-peer dedup + stale-term drop (bug-1095/1096)

Browse source 
raft_on_vote_reply double-counted duplicated vote-responses (counter ignored
from_idx) and failed to drop stale-term replies, allowing two leaders at the
same term (bug-851 split-brain family). Mirror the existing pre-vote per-peer
dedup pattern (new vote_replies[] set) and add DropStaleResponse (term <
currentTerm). Surfaced by the raft-sim-conformance oracle (task 4.2).

47 raft / 264 overall lib tests pass.
This commit is contained in:
Rongsong Shen 2026-06-24 13:56:24 +00:00
parent e15bbf8190
commit b5b7e01b92
3 changed files with 135 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

40
src/lib/raft/raft.c
View file

@ -70,6 +70,10 @@ int raft_init(raft_state_t *s, const raft_config_t *cfg)
s->self_idx = cfg->self_idx;
s->votes_granted = 0;
s->replies_received = 0;
/* Defensive: a node must never carry stale per-peer vote flags into its
* first election (mirrors pre_vote_replies). raft_start_election re-clears
* them, but zero here so a never-elected node has a clean array. */
memset(s->vote_replies, 0, sizeof(s->vote_replies));
s->election_decided = 0;
s->election_timeout_ms = cfg->election_timeout_ms ? cfg->election_timeout_ms : 2000;
s->my_priority = cfg->my_priority;
@ -110,6 +114,13 @@ int raft_start_election(raft_state_t *s)
s->voted_for_term = s->term;
s->votes_granted = 1; /* self-vote */
s->replies_received = 0;
/* Fresh per-peer vote-reply tally for this election (mirrors
* pre_vote_replies reset). Mark self as already-counted so a stray
* self-addressed reply cannot double-count the self-vote that
* votes_granted already includes. */
memset(s->vote_replies, 0, sizeof(s->vote_replies));
if (s->self_idx >= 0 && s->self_idx < RAFT_MAX_PEERS)
s->vote_replies[s->self_idx] = 1;
s->election_decided = 0;
if (s->cbs->on_trace)
@ -421,8 +432,6 @@ int raft_on_pre_vote_reply(raft_state_t *s, int peer_idx, int granted)
int raft_on_vote_reply(raft_state_t *s, int from_idx, int granted, uint64_t term)
{
(void)from_idx;
if (s->cbs->on_trace)
s->cbs->on_trace(s->ctx, "vote_reply_in", NULL);
@ -438,6 +447,15 @@ int raft_on_vote_reply(raft_state_t *s, int from_idx, int granted, uint64_t term
return RAFT_BECAME_FOLLOWER;
}
/* DropStaleResponse (raft.tla DropStaleResponse, mterm < currentTerm):
* a reply from an older term is discarded, NOT counted toward this
* election — and it is NOT a higher term, so it must not step us down. */
if (term < s->term) {
if (s->cbs->on_trace)
s->cbs->on_trace(s->ctx, "vote_reply_stale_drop", NULL);
return RAFT_PENDING;
}
/* Step 2: no-op guard — election already decided */
if (s->election_decided) {
if (s->cbs->on_trace)
@ -445,6 +463,24 @@ int raft_on_vote_reply(raft_state_t *s, int from_idx, int granted, uint64_t term
return RAFT_PENDING;
}
/* Bounds guard: ignore a reply from an out-of-range peer (mirrors the
* pre-vote per-peer dedup bound). */
if (from_idx < 0 || from_idx >= s->n_peers || from_idx >= RAFT_MAX_PEERS) {
if (s->cbs->on_trace)
s->cbs->on_trace(s->ctx, "vote_reply_out_of_range", NULL);
return RAFT_PENDING;
}
/* Per-peer idempotency (raft.tla votesGranted is a SET): a duplicate reply
* from a peer we already counted is ignored — a legal DuplicateMessage must
* not double-count toward quorum (Election Safety). */
if (s->vote_replies[from_idx]) {
if (s->cbs->on_trace)
s->cbs->on_trace(s->ctx, "vote_reply_dup", NULL);
return RAFT_PENDING;
}
s->vote_replies[from_idx] = 1;
/* Step 3: count this reply */
s->replies_received++;

1
src/lib/raft/raft.h
View file

@ -34,6 +34,7 @@ typedef struct {
int self_idx; /* this node's index in [0, n_peers) */
int votes_granted; /* votes collected in current election */
int replies_received; /* total peer replies received (for all-replied-no-quorum, Req 4.4) */
int vote_replies[RAFT_MAX_PEERS]; /* per-peer replied flag; avoids double-count (mirrors pre_vote_replies) */
int election_decided; /* 1 once on_become_leader/follower fired */
uint32_t election_timeout_ms;
int my_priority; /* priority index for jitter (0 = highest) */

96
src/lib/testsuite/test_raft.c
View file

@ -1545,6 +1545,100 @@ static void test_vote_reply_pending_waiting(void)
TEST_ASSERT_EQUAL_INT(0, g_mock.cancel_timeout_count);
}
/*
* test_vote_reply_duplicate_grant_not_double_counted (bug-1095, raft.tla
* votesGranted-as-SET, idempotent under DuplicateMessage)
*
* 5-node cluster (quorum=3). Node starts a real election via raft_start_election
* (self-vote=1, vote_replies[self]=1). A GRANTED reply from peer 1 → votes=2,
* PENDING. The SAME peer-1 grant delivered AGAIN (a legal DuplicateMessage) must
* be IGNORED: RAFT_PENDING, votes still 2, NOT leader. Only a grant from a
* DISTINCT peer 2 reaches quorum (votes=3 → RAFT_BECAME_LEADER).
*
* Before the fix the duplicate would push votes to 3 and elect prematurely
* (Election Safety violation — a node could win on a single peer's vote).
*/
static void test_vote_reply_duplicate_grant_not_double_counted(void)
{
raft_state_t s;
raft_callbacks_t cbs = make_mock_cbs();
cbs.on_trace = mock_trace;
reset_mock();
TEST_ASSERT_EQUAL_INT(0, init_node(&s, &cbs, 5, 0));
/* Real election start: CANDIDATE, self-vote=1, vote_replies[self]=1. */
TEST_ASSERT_EQUAL_INT(RAFT_OK, raft_start_election(&s));
TEST_ASSERT_EQUAL_INT(RAFT_CANDIDATE, s.role);
TEST_ASSERT_EQUAL_INT(1, s.votes_granted);
/* First grant from peer 1 → votes=2, still below quorum=3. */
int rc1 = raft_on_vote_reply(&s, 1, 1, s.term);
TEST_ASSERT_EQUAL_INT(RAFT_PENDING, rc1);
TEST_ASSERT_EQUAL_INT(2, s.votes_granted);
TEST_ASSERT_EQUAL_INT(RAFT_CANDIDATE, s.role);
/* DUPLICATE grant from peer 1 → must be ignored (no double-count). */
int rcdup = raft_on_vote_reply(&s, 1, 1, s.term);
TEST_ASSERT_EQUAL_INT(RAFT_PENDING, rcdup);
TEST_ASSERT_EQUAL_INT(2, s.votes_granted); /* still 2, NOT 3 */
TEST_ASSERT_EQUAL_INT(RAFT_CANDIDATE, s.role);
TEST_ASSERT_EQUAL_INT(0, g_mock.become_leader_count);
/* DISTINCT peer 2 grant → votes=3 = quorum → leader. */
int rc2 = raft_on_vote_reply(&s, 2, 1, s.term);
TEST_ASSERT_EQUAL_INT(RAFT_BECAME_LEADER, rc2);
TEST_ASSERT_EQUAL_INT(RAFT_LEADER, s.role);
TEST_ASSERT_EQUAL_INT(3, s.votes_granted);
TEST_ASSERT_EQUAL_INT(1, g_mock.become_leader_count);
}
/*
* test_vote_reply_stale_term_dropped (bug-1096, raft.tla DropStaleResponse —
* discard a response whose mterm < currentTerm)
*
* Candidate at term=2. A vote reply carrying term=1 (< current) must be DROPPED:
* RAFT_PENDING, NOT counted toward votes, and NO step-down (the stale reply is
* not a higher term, so it must not flip the node to FOLLOWER). The node stays a
* CANDIDATE with its vote tally unchanged.
*
* Before the fix the stale reply was counted (replies_received++ and, if granted,
* votes_granted++) — corrupting the current election's quorum accounting.
*/
static void test_vote_reply_stale_term_dropped(void)
{
raft_state_t s;
raft_callbacks_t cbs = make_mock_cbs();
cbs.on_trace = mock_trace;
reset_mock();
TEST_ASSERT_EQUAL_INT(0, init_node(&s, &cbs, 5, 0));
/* Drive to a real election, then bump to term 2 (second election) so a
* reply at term 1 is strictly stale. */
TEST_ASSERT_EQUAL_INT(RAFT_OK, raft_start_election(&s)); /* term 1 */
s.election_decided = 0;
s.role = RAFT_FOLLOWER; /* allow a second start_election */
TEST_ASSERT_EQUAL_INT(RAFT_OK, raft_start_election(&s)); /* term 2 */
TEST_ASSERT_EQUAL_UINT64(2, s.term);
TEST_ASSERT_EQUAL_INT(1, s.votes_granted);
TEST_ASSERT_EQUAL_INT(0, s.replies_received);
/* Stale GRANTED reply at term 1 (< current term 2) → dropped. */
int rc = raft_on_vote_reply(&s, 1, 1, 1);
TEST_ASSERT_EQUAL_INT(RAFT_PENDING, rc);
/* Not counted, no step-down, still CANDIDATE at term 2. */
TEST_ASSERT_EQUAL_INT(1, s.votes_granted); /* unchanged */
TEST_ASSERT_EQUAL_INT(0, s.replies_received); /* unchanged */
TEST_ASSERT_EQUAL_INT(RAFT_CANDIDATE, s.role);
TEST_ASSERT_EQUAL_UINT64(2, s.term);
TEST_ASSERT_EQUAL_INT(0, g_mock.become_follower_count);
TEST_ASSERT_EQUAL_INT(0, g_mock.become_leader_count);
}
/* ---------- leader progress init on becoming leader (task 2.2) ---------- */
/* Init a node with a non-zero restored log length (initial_log_len). */
@ -7805,6 +7899,8 @@ void test_raft(void)
RUN_TEST(test_vote_reply_no_quorum_all_replied_3node);
RUN_TEST(test_vote_reply_quorum_5node);
RUN_TEST(test_vote_reply_pending_waiting);
RUN_TEST(test_vote_reply_duplicate_grant_not_double_counted);
RUN_TEST(test_vote_reply_stale_term_dropped);
RUN_TEST(test_leader_progress_init_single_node);
RUN_TEST(test_leader_progress_init_multi_node);