diff --git a/.kiro/specs/raft-log-replication/tasks.md b/.kiro/specs/raft-log-replication/tasks.md index 3849b06..28f461d 100644 --- a/.kiro/specs/raft-log-replication/tasks.md +++ b/.kiro/specs/raft-log-replication/tasks.md @@ -120,14 +120,14 @@ ## 7. HandleAppendEntriesResponse — raft_on_append_entries_reply (raft.tla:393) -- [ ] 7.1 RED: reply advance/decrement/stale-drop tests +- [x] 7.1 RED: reply advance/decrement/stale-drop tests - Test: success sets `next_index[peer]=matchIndex+1`, `match_index[peer]=reply.matchIndex`; failure sets `next_index[peer]=Max(next_index[peer]-1,1)` (floor at 1), `match_index` unchanged; a reply with `reply_term != currentTerm` is ignored - Observable completion: tests compile and FAIL - _Requirements: 5.1, 5.2, 5.3_ - _Boundary: test_raft.c_ - _Depends: 2.2_ -- [ ] 7.2 GREEN: implement `raft_on_append_entries_reply` +- [x] 7.2 GREEN: implement `raft_on_append_entries_reply` - Stale-term drop; success/failure branches; `next_index` floored at 1; do NOT recompute or advance any commit index from `match_index` - Observable completion: 7.1 tests pass - _Requirements: 5.1, 5.2, 5.3, 5.4_ diff --git a/src/lib/raft/raft.c b/src/lib/raft/raft.c index 0b666f6..c45ed4c 100644 --- a/src/lib/raft/raft.c +++ b/src/lib/raft/raft.c @@ -677,3 +677,35 @@ int raft_on_append_entries_request(raft_state_t *s, int leader_idx, return RAFT_OK; } } + +int raft_on_append_entries_reply(raft_state_t *s, int peer_idx, int success, + uint64_t match_index, uint64_t reply_term) +{ + /* ---- Stale-term drop FIRST (raft.tla HandleAppendEntriesResponse 415, Req 5.4) ---- + * The response is meaningful only if it pertains to the leader's current + * term (m.mterm == currentTerm[i]). A reply carrying any other term — from + * an old in-flight RPC or a future leader — is discarded with no state + * change. No commit index is ever advanced here (Req 5.4; owned by + * raft-commit-safety). */ + if (reply_term != s->term) + return RAFT_OK; + + if (success) { + /* success (raft.tla 397, Req 5.2): nextIndex'[peer] = m.mmatchIndex + 1, + * matchIndex'[peer] = m.mmatchIndex. */ + s->match_index[peer_idx] = match_index; + s->next_index[peer_idx] = match_index + 1; + } else { + /* failure (raft.tla 401, Req 5.3): nextIndex'[peer] = + * Max(nextIndex[peer] - 1, 1) — back off one index, floored at 1. + * matchIndex is left unchanged. */ + uint64_t next = s->next_index[peer_idx]; + s->next_index[peer_idx] = (next > 1) ? (next - 1) : 1; + } + + if (s->cbs->on_trace) + s->cbs->on_trace(s->ctx, success ? "ae_reply_success" : "ae_reply_fail", + NULL); + + return RAFT_OK; +} diff --git a/src/lib/testsuite/test_raft.c b/src/lib/testsuite/test_raft.c index 4ebd6a0..e30b37e 100644 --- a/src/lib/testsuite/test_raft.c +++ b/src/lib/testsuite/test_raft.c @@ -2488,6 +2488,111 @@ static void test_ae_req_leader_commit_not_acted(void) TEST_ASSERT_EQUAL_UINT64(1, s.log_len); } +/* ---------- raft_on_append_entries_reply tests (task 7.1/7.2, + * raft.tla HandleAppendEntriesResponse 393) ---------- */ + +/* + * test_ae_reply_success_advances_indices (Req 5.1, 5.2) + * + * A success reply at the leader's current term must: + * - set next_index[peer] = match_index + 1 + * - set match_index[peer] = match_index + * - return RAFT_OK + * It must NOT advance any commit index (no commit field exists; not asserted here). + */ +static void test_ae_reply_success_advances_indices(void) +{ + raft_state_t s; + raft_callbacks_t cbs = make_mock_cbs(); + + reset_mock(); + /* restored log_len = 5; after drive_to_leader next_index[peer]=6, match_index[peer]=0 */ + TEST_ASSERT_EQUAL_INT(0, init_node_log_len(&s, &cbs, 3, 0, 5)); + drive_to_leader(&s); + + /* peer 1 acknowledged up through index 4 */ + int rc = raft_on_append_entries_reply(&s, /*peer_idx=*/1, /*success=*/1, + /*match_index=*/4, /*reply_term=*/s.term); + + TEST_ASSERT_EQUAL_INT(RAFT_OK, rc); + TEST_ASSERT_EQUAL_UINT64(5, s.next_index[1]); /* match_index + 1 */ + TEST_ASSERT_EQUAL_UINT64(4, s.match_index[1]); /* == match_index */ +} + +/* + * test_ae_reply_failure_decrements_next_index_floor (Req 5.1, 5.3) + * + * A failure reply at the leader's current term must: + * - set next_index[peer] = Max(next_index[peer] - 1, 1) (decrement, floored at 1) + * - leave match_index[peer] unchanged + * - stay at 1 when next_index[peer] is already 1 (verify the floor) + * - return RAFT_OK + */ +static void test_ae_reply_failure_decrements_next_index_floor(void) +{ + raft_state_t s; + raft_callbacks_t cbs = make_mock_cbs(); + + reset_mock(); + /* restored log_len = 5 → next_index[peer] = 6 after drive_to_leader */ + TEST_ASSERT_EQUAL_INT(0, init_node_log_len(&s, &cbs, 3, 0, 5)); + drive_to_leader(&s); + + /* seed a non-zero match_index to prove failure leaves it untouched */ + s.match_index[1] = 3; + + /* first failure: 6 → 5 */ + int rc = raft_on_append_entries_reply(&s, /*peer_idx=*/1, /*success=*/0, + /*match_index=*/0, /*reply_term=*/s.term); + TEST_ASSERT_EQUAL_INT(RAFT_OK, rc); + TEST_ASSERT_EQUAL_UINT64(5, s.next_index[1]); + TEST_ASSERT_EQUAL_UINT64(3, s.match_index[1]); /* unchanged */ + + /* drive next_index down to the floor */ + s.next_index[1] = 1; + rc = raft_on_append_entries_reply(&s, /*peer_idx=*/1, /*success=*/0, + /*match_index=*/0, /*reply_term=*/s.term); + TEST_ASSERT_EQUAL_INT(RAFT_OK, rc); + TEST_ASSERT_EQUAL_UINT64(1, s.next_index[1]); /* floored at 1, not 0 */ + TEST_ASSERT_EQUAL_UINT64(3, s.match_index[1]); /* still unchanged */ +} + +/* + * test_ae_reply_stale_term_ignored (Req 5.4, raft.tla 415) + * + * A reply whose reply_term != s->term is a stale-term drop: neither next_index + * nor match_index for the peer may change. Returns RAFT_OK. + */ +static void test_ae_reply_stale_term_ignored(void) +{ + raft_state_t s; + raft_callbacks_t cbs = make_mock_cbs(); + + reset_mock(); + TEST_ASSERT_EQUAL_INT(0, init_node_log_len(&s, &cbs, 3, 0, 5)); + drive_to_leader(&s); + + /* snapshot current per-peer indices (next_index[1]=6, match_index[1]=0) */ + uint64_t next_before = s.next_index[1]; + uint64_t match_before = s.match_index[1]; + + /* stale success reply (older term) must be ignored */ + int rc = raft_on_append_entries_reply(&s, /*peer_idx=*/1, /*success=*/1, + /*match_index=*/9, + /*reply_term=*/s.term - 1); + TEST_ASSERT_EQUAL_INT(RAFT_OK, rc); + TEST_ASSERT_EQUAL_UINT64(next_before, s.next_index[1]); + TEST_ASSERT_EQUAL_UINT64(match_before, s.match_index[1]); + + /* stale failure reply (newer term, e.g. from a future leader) must also be ignored */ + rc = raft_on_append_entries_reply(&s, /*peer_idx=*/1, /*success=*/0, + /*match_index=*/0, + /*reply_term=*/s.term + 1); + TEST_ASSERT_EQUAL_INT(RAFT_OK, rc); + TEST_ASSERT_EQUAL_UINT64(next_before, s.next_index[1]); + TEST_ASSERT_EQUAL_UINT64(match_before, s.match_index[1]); +} + /* ---------- raft_on_election_timeout tests (task 3.5) ---------- */ /* @@ -5116,6 +5221,11 @@ void test_raft(void) RUN_TEST(test_ae_req_append_extends_log); RUN_TEST(test_ae_req_leader_commit_not_acted); + /* raft_on_append_entries_reply (task 7.1/7.2) */ + RUN_TEST(test_ae_reply_success_advances_indices); + RUN_TEST(test_ae_reply_failure_decrements_next_index_floor); + RUN_TEST(test_ae_reply_stale_term_ignored); + /* raft_on_vote_request (task 3.3) */ RUN_TEST(test_vote_req_stale_term); RUN_TEST(test_vote_req_higher_term_updates_state);