lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat(raft-commit-safety): raft_advance_commit Figure-8 current-term rule (tasks 3.1-3.3, 6.1)

Browse source 
AdvanceCommitIndex (raft.tla:259/270, closes G4): leader-only (-EPERM); Max(agreeIndexes)
= highest N with a quorum ((n_peers/2)+1) of match_index>=N, self counted as log_len;
scan log_len downward. CURRENT-TERM RULE: advance commit_index to N only when
on_get_entry_term(N)==s->term, else HOLD (Figure-8 safety — never commit a previous-term
entry on replication count alone; a later current-term commit indirectly commits it).
Monotonic; fires on_commit_advanced once per strict advance; commit_index<=log_len.

3.3: raft_on_append_entries_reply left untouched — verified it never advances commit
(separation, Req 2.6). 6.1: sim commit-driver helper raft_sim_advance_commit added.

Req 6.3 two-phase indirect-commit transition covered end-to-end by task 7.1 (sim).
ASAN green (230 tests).
This commit is contained in:
Rongsong Shen 2026-06-21 23:27:12 +00:00
parent 3ba6f03a9e
commit 195f660f9a
5 changed files with 350 additions and 4 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -2670,6 +2670,267 @@ static void test_ae_reply_stale_term_ignored(void)
TEST_ASSERT_EQUAL_UINT64(match_before, s.match_index[1]);
}
/* ---------- raft_advance_commit tests (task 3.1/3.2/3.3, raft.tla
* AdvanceCommitIndex 259/270 — current-term Figure-8 rule) ---------- */
/* Spy on_commit_advanced: records the last reported commit_index and a fire
* counter so tests can assert the strict-advance / fire-once semantics (the
* library fires this ONLY on a strict commit_index advance). */
typedef struct {
int call_count;
uint64_t last_commit_index;
} commit_advanced_spy_t;
static commit_advanced_spy_t g_ca_spy;
static void reset_ca_spy(void)
{
memset(&g_ca_spy, 0, sizeof(g_ca_spy));
}
static void spy_commit_advanced(void *ctx, uint64_t commit_index)
{
(void)ctx;
g_ca_spy.call_count++;
g_ca_spy.last_commit_index = commit_index;
}
/*
* test_advance_commit_advances_current_term (Req 1.2-1.7, 2.2, 2.3)
*
* Leader, term=2, log_len=3. A quorum (self + peer 1 of a 3-node cluster) agrees
* through index 3, and the entry at index 3 is from the CURRENT term (2). The
* commit index must advance to 3 and on_commit_advanced(3) must fire exactly once.
*/
static void test_advance_commit_advances_current_term(void)
{
raft_state_t s;
raft_callbacks_t cbs = make_mock_cbs();
cbs.on_get_entry_term = spy_get_entry_term;
cbs.on_commit_advanced = spy_commit_advanced;
reset_mock();
reset_gt_spy();
reset_ca_spy();
TEST_ASSERT_EQUAL_INT(0, init_node_log_len(&s, &cbs, 3, 0, 0));
/* Hand-set leader state directly (do not exercise the election path). */
s.role = RAFT_LEADER;
s.term = 2;
s.log_len = 3;
/* every entry 1..3 is from the current term 2 */
g_gt_spy.term_at[1] = 2;
g_gt_spy.term_at[2] = 2;
g_gt_spy.term_at[3] = 2;
/* peer 1 has replicated through index 3 (self counts as log_len=3) → quorum (2 of 3) */
s.match_index[1] = 3;
s.match_index[2] = 0;
int rc = raft_advance_commit(&s);
TEST_ASSERT_EQUAL_INT(RAFT_OK, rc);
TEST_ASSERT_EQUAL_UINT64(3, s.commit_index);
TEST_ASSERT_EQUAL_INT(1, g_ca_spy.call_count);
TEST_ASSERT_EQUAL_UINT64(3, g_ca_spy.last_commit_index);
}
/*
* test_advance_commit_figure8_hold_previous_term (Req 1.2-1.7, 2.2, 2.3)
*
* Figure-8 safety: a quorum agrees through index 2, but the entry at index 2 is
* from a PREVIOUS term (1) while the leader's current term is 3, and no
* current-term entry sits above it that also has quorum. The commit index must
* NOT advance and on_commit_advanced must NOT fire.
*/
static void test_advance_commit_figure8_hold_previous_term(void)
{
raft_state_t s;
raft_callbacks_t cbs = make_mock_cbs();
cbs.on_get_entry_term = spy_get_entry_term;
cbs.on_commit_advanced = spy_commit_advanced;
reset_mock();
reset_gt_spy();
reset_ca_spy();
TEST_ASSERT_EQUAL_INT(0, init_node_log_len(&s, &cbs, 3, 0, 0));
s.role = RAFT_LEADER;
s.term = 3;
s.log_len = 3;
/* entries 1,2 are from a previous term (1); entry 3 (current term) is NOT
* yet replicated to a quorum. */
g_gt_spy.term_at[1] = 1;
g_gt_spy.term_at[2] = 1;
g_gt_spy.term_at[3] = 3;
/* peer 1 agrees only through index 2 (a previous-term entry); self has 3. */
s.match_index[1] = 2;
s.match_index[2] = 0;
int rc = raft_advance_commit(&s);
TEST_ASSERT_EQUAL_INT(RAFT_OK, rc);
TEST_ASSERT_EQUAL_UINT64(0, s.commit_index); /* held — not advanced over prev-term */
TEST_ASSERT_EQUAL_INT(0, g_ca_spy.call_count);
}
/*
* test_advance_commit_no_quorum (Req 1.2-1.7, 2.2, 2.3)
*
* No index reaches quorum (a 3-node cluster where no peer has replicated any
* entry; only self agrees). agreeIndexes is empty → commit_index unchanged, no
* callback.
*/
static void test_advance_commit_no_quorum(void)
{
raft_state_t s;
raft_callbacks_t cbs = make_mock_cbs();
cbs.on_get_entry_term = spy_get_entry_term;
cbs.on_commit_advanced = spy_commit_advanced;
reset_mock();
reset_gt_spy();
reset_ca_spy();
TEST_ASSERT_EQUAL_INT(0, init_node_log_len(&s, &cbs, 3, 0, 0));
s.role = RAFT_LEADER;
s.term = 2;
s.log_len = 3;
g_gt_spy.term_at[1] = 2;
g_gt_spy.term_at[2] = 2;
g_gt_spy.term_at[3] = 2;
/* no peer has replicated anything → only self agrees (1 of 3 — not quorum) */
s.match_index[1] = 0;
s.match_index[2] = 0;
int rc = raft_advance_commit(&s);
TEST_ASSERT_EQUAL_INT(RAFT_OK, rc);
TEST_ASSERT_EQUAL_UINT64(0, s.commit_index);
TEST_ASSERT_EQUAL_INT(0, g_ca_spy.call_count);
}
/*
* test_advance_commit_monotonic (Req 1.2-1.7, 2.2, 2.3)
*
* A later call that computes a SMALLER Max(agreeIndexes) must never lower
* commit_index. First advance to 3, then drop a peer's match_index so the newly
* computed Max is 1 (< 3): commit_index stays 3 and no second callback fires.
*/
static void test_advance_commit_monotonic(void)
{
raft_state_t s;
raft_callbacks_t cbs = make_mock_cbs();
cbs.on_get_entry_term = spy_get_entry_term;
cbs.on_commit_advanced = spy_commit_advanced;
reset_mock();
reset_gt_spy();
reset_ca_spy();
TEST_ASSERT_EQUAL_INT(0, init_node_log_len(&s, &cbs, 3, 0, 0));
s.role = RAFT_LEADER;
s.term = 2;
s.log_len = 3;
g_gt_spy.term_at[1] = 2;
g_gt_spy.term_at[2] = 2;
g_gt_spy.term_at[3] = 2;
s.match_index[1] = 3;
s.match_index[2] = 0;
/* first advance → commit_index = 3 */
TEST_ASSERT_EQUAL_INT(RAFT_OK, raft_advance_commit(&s));
TEST_ASSERT_EQUAL_UINT64(3, s.commit_index);
TEST_ASSERT_EQUAL_INT(1, g_ca_spy.call_count);
/* now the peer regresses to index 1; new Max(agreeIndexes) = 1 < 3 */
s.match_index[1] = 1;
TEST_ASSERT_EQUAL_INT(RAFT_OK, raft_advance_commit(&s));
TEST_ASSERT_EQUAL_UINT64(3, s.commit_index); /* never lowered */
TEST_ASSERT_EQUAL_INT(1, g_ca_spy.call_count); /* no second fire */
}
/*
* test_advance_commit_non_leader_eperm (Req 1.2-1.7, 2.2, 2.3)
*
* Off the leader, raft_advance_commit returns -EPERM, makes no change to
* commit_index, and fires no callback.
*/
static void test_advance_commit_non_leader_eperm(void)
{
raft_state_t s;
raft_callbacks_t cbs = make_mock_cbs();
cbs.on_get_entry_term = spy_get_entry_term;
cbs.on_commit_advanced = spy_commit_advanced;
reset_mock();
reset_gt_spy();
reset_ca_spy();
TEST_ASSERT_EQUAL_INT(0, init_node_log_len(&s, &cbs, 3, 0, 0));
s.role = RAFT_FOLLOWER;
s.term = 2;
s.log_len = 3;
g_gt_spy.term_at[1] = 2;
g_gt_spy.term_at[2] = 2;
g_gt_spy.term_at[3] = 2;
s.match_index[1] = 3;
s.match_index[2] = 3;
int rc = raft_advance_commit(&s);
TEST_ASSERT_EQUAL_INT(-EPERM, rc);
TEST_ASSERT_EQUAL_UINT64(0, s.commit_index);
TEST_ASSERT_EQUAL_INT(0, g_ca_spy.call_count);
}
/*
* test_ae_reply_does_not_advance_commit (task 3.3, Req 2.6)
*
* Commit-advance is SEPARATE from the AppendEntries-reply handler. A
* raft_on_append_entries_reply that updates match_index[] to a committable quorum
* must NOT, by itself, change commit_index or fire on_commit_advanced. Only a
* subsequent raft_advance_commit advances the commit index.
*
* NOTE: raft_on_append_entries_reply is owned upstream (raft-log-replication) and
* is NOT modified by this spec; this test only asserts that contract holds.
*/
static void test_ae_reply_does_not_advance_commit(void)
{
raft_state_t s;
raft_callbacks_t cbs = make_mock_cbs();
cbs.on_get_entry_term = spy_get_entry_term;
cbs.on_commit_advanced = spy_commit_advanced;
reset_mock();
reset_gt_spy();
reset_ca_spy();
/* restored log_len = 3 → after drive_to_leader next_index[peer]=4 */
TEST_ASSERT_EQUAL_INT(0, init_node_log_len(&s, &cbs, 3, 0, 3));
drive_to_leader(&s);
/* all 3 entries are from the leader's current term → would be committable */
g_gt_spy.term_at[1] = s.term;
g_gt_spy.term_at[2] = s.term;
g_gt_spy.term_at[3] = s.term;
uint64_t commit_before = s.commit_index;
/* a success reply moving peer 1 to a committable quorum index */
int rc = raft_on_append_entries_reply(&s, /*peer_idx=*/1, /*success=*/1,
/*match_index=*/3, /*reply_term=*/s.term);
TEST_ASSERT_EQUAL_INT(RAFT_OK, rc);
TEST_ASSERT_EQUAL_UINT64(3, s.match_index[1]); /* match recorded */
/* the reply ALONE must not move commit or fire the callback */
TEST_ASSERT_EQUAL_UINT64(commit_before, s.commit_index);
TEST_ASSERT_EQUAL_INT(0, g_ca_spy.call_count);
/* only the explicit commit-advance step advances commit_index */
TEST_ASSERT_EQUAL_INT(RAFT_OK, raft_advance_commit(&s));
TEST_ASSERT_EQUAL_UINT64(3, s.commit_index);
TEST_ASSERT_EQUAL_INT(1, g_ca_spy.call_count);
}
/* ---------- raft_on_election_timeout tests (task 3.5) ---------- */
/*
@ -5948,6 +6209,14 @@ void test_raft(void)
RUN_TEST(test_ae_reply_failure_decrements_next_index_floor);
RUN_TEST(test_ae_reply_stale_term_ignored);
/* raft_advance_commit (task 3.1/3.2/3.3, AdvanceCommitIndex + Figure-8 rule) */
RUN_TEST(test_advance_commit_advances_current_term);
RUN_TEST(test_advance_commit_figure8_hold_previous_term);
RUN_TEST(test_advance_commit_no_quorum);
RUN_TEST(test_advance_commit_monotonic);
RUN_TEST(test_advance_commit_non_leader_eperm);
RUN_TEST(test_ae_reply_does_not_advance_commit);
/* raft_on_vote_request (task 3.3) */
RUN_TEST(test_vote_req_stale_term);
RUN_TEST(test_vote_req_higher_term_updates_state);