From e5b9b83955a56d9ab7165404ee75c30d57ef122d Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sun, 21 Jun 2026 15:50:33 +0000 Subject: [PATCH] feat(raft-log-replication): raft_log_append leader client-entry append (tasks 3.1, 3.2) Leader-only (-EPERM for follower/candidate, no mutation/callback); index= log_len+1, term=currentTerm (raft.tla ClientRequest:246, Req 2.1-2.4); persist-before-act: on_persist_entry called once BEFORE log_len advances, non-zero return propagated with no phantom entry (Req 2.5); no commit-index advance, no transport I/O. Spy-based tests for all four branches. ASAN green. --- .kiro/specs/raft-log-replication/tasks.md | 4 +- src/lib/raft/raft.c | 33 ++++ src/lib/testsuite/test_raft.c | 177 ++++++++++++++++++++++ 3 files changed, 212 insertions(+), 2 deletions(-) diff --git a/.kiro/specs/raft-log-replication/tasks.md b/.kiro/specs/raft-log-replication/tasks.md index e1fe7a44..e2510910 100644 --- a/.kiro/specs/raft-log-replication/tasks.md +++ b/.kiro/specs/raft-log-replication/tasks.md @@ -49,14 +49,14 @@ ## 3. ClientRequest — raft_log_append (raft.tla:246) -- [ ] 3.1 RED: append tests (leader stamps term, follower rejected) +- [x] 3.1 RED: append tests (leader stamps term, follower rejected) - Test: leader `raft_log_append` assigns index `log_len+1`, stamps `currentTerm`, calls `on_persist_entry` exactly once with `(index, term, entry)`, returns the index; follower call returns `-EPERM` with no log change and no callback - Observable completion: tests compile and FAIL (no implementation yet) - _Requirements: 2.1, 2.2, 2.3, 2.4_ - _Boundary: test_raft.c_ - _Depends: 2.2_ -- [ ] 3.2 GREEN: implement `raft_log_append` +- [x] 3.2 GREEN: implement `raft_log_append` - Role guard (`-EPERM` if not leader); persist-before-act (`on_persist_entry` before incrementing `log_len`); set `*out_index`; do NOT advance any commit index - Observable completion: 3.1 tests pass; `log_len` only advances on a successful persist (no phantom entry on persist failure) - _Requirements: 2.1, 2.2, 2.3, 2.4, 2.5_ diff --git a/src/lib/raft/raft.c b/src/lib/raft/raft.c index 3e254504..336e953e 100644 --- a/src/lib/raft/raft.c +++ b/src/lib/raft/raft.c @@ -470,3 +470,36 @@ int raft_check_and_start_election(raft_state_t *s) (void)rc; return RAFT_ELECTION_STARTED; } + +int raft_log_append(raft_state_t *s, const void *entry, size_t len, + uint64_t *out_index) +{ + /* Role gate (Req 2.2): only a leader may grow the log. Reject without any + * log mutation or callback (raft.tla ClientRequest is enabled only at the + * leader). */ + if (s->role != RAFT_LEADER) + return -EPERM; + + /* Assign the next 1-based index (== WAL LSN) and stamp the leader's current + * term, mirroring raft.tla ClientRequest's [term ↦ currentTerm, value ↦ v] + * (Req 2.1, 2.3, 2.4). */ + uint64_t index = s->log_len + 1; + uint64_t term = s->term; + + /* Persist-before-act (Req 2.3, 2.5): durably write the entry BEFORE + * advancing log_len. On failure, propagate the error verbatim and leave the + * log untouched (no phantom entry). */ + int rc = s->cbs->on_persist_entry(s->ctx, index, term, entry, len); + if (rc != 0) + return rc; + + /* Commit the append only after a successful persist. */ + s->log_len = index; + if (out_index) + *out_index = index; + + if (s->cbs->on_trace) + s->cbs->on_trace(s->ctx, "log_append", NULL); + + return RAFT_OK; +} diff --git a/src/lib/testsuite/test_raft.c b/src/lib/testsuite/test_raft.c index 2cda479a..38e6cb61 100644 --- a/src/lib/testsuite/test_raft.c +++ b/src/lib/testsuite/test_raft.c @@ -1519,6 +1519,177 @@ static void test_leader_progress_init_multi_node(void) } } +/* ---------- raft_log_append tests (task 3.1/3.2, raft.tla ClientRequest 246) ---------- */ + +/* Spy on_persist_entry: records every call's (index, term, bytes, len) and a + * call count. A separate fail-mode variant returns a configurable non-zero rc + * to exercise the persist-failure path (no log mutation, error propagated). */ +typedef struct { + int call_count; + uint64_t last_index; + uint64_t last_term; + size_t last_len; + unsigned char last_bytes[RAFT_SIM_ENTRY_LEN]; + int return_value; /* 0 = success; non-zero = persist failure */ +} persist_entry_spy_t; + +static persist_entry_spy_t g_pe_spy; + +static void reset_pe_spy(void) +{ + memset(&g_pe_spy, 0, sizeof(g_pe_spy)); + g_pe_spy.return_value = 0; +} + +static int spy_persist_entry(void *ctx, uint64_t index, uint64_t term, + const void *entry, size_t len) +{ + (void)ctx; + g_pe_spy.call_count++; + g_pe_spy.last_index = index; + g_pe_spy.last_term = term; + g_pe_spy.last_len = len; + size_t n = len < sizeof(g_pe_spy.last_bytes) ? len : sizeof(g_pe_spy.last_bytes); + if (entry && n) + memcpy(g_pe_spy.last_bytes, entry, n); + return g_pe_spy.return_value; +} + +/* Drive a node to LEADER via the multi-node quorum path (start_election → + * one peer grant), leaving log_len at the restored value. */ +static void drive_to_leader(raft_state_t *s) +{ + TEST_ASSERT_EQUAL_INT(RAFT_OK, raft_start_election(s)); + TEST_ASSERT_EQUAL_INT(RAFT_CANDIDATE, s->role); + TEST_ASSERT_EQUAL_INT(RAFT_BECAME_LEADER, raft_on_vote_reply(s, 1, 1, s->term)); + TEST_ASSERT_EQUAL_INT(RAFT_LEADER, s->role); +} + +/* + * test_log_append_leader_assigns_index_stamps_term (Req 2.1, 2.3, 2.4, 2.5) + * + * LEADER raft_log_append must: + * - assign index log_len+1 (1-based, == WAL LSN) + * - stamp the entry with s->term (currentTerm), not a zero placeholder + * - call on_persist_entry EXACTLY ONCE with (index, term, entry, len) + * - return that index via out_index + * - return RAFT_OK and advance log_len by exactly one + */ +static void test_log_append_leader_assigns_index_stamps_term(void) +{ + raft_state_t s; + raft_callbacks_t cbs = make_mock_cbs(); + cbs.on_persist_entry = spy_persist_entry; + + reset_mock(); + reset_pe_spy(); + /* restored log_len = 4 so the first append lands at index 5 */ + TEST_ASSERT_EQUAL_INT(0, init_node_log_len(&s, &cbs, 3, 0, 4)); + drive_to_leader(&s); + uint64_t term_at_append = s.term; + + const unsigned char payload[] = { 0xDE, 0xAD, 0xBE, 0xEF }; + uint64_t out_index = 0; + int rc = raft_log_append(&s, payload, sizeof(payload), &out_index); + + TEST_ASSERT_EQUAL_INT(RAFT_OK, rc); + /* assigned index = log_len(before)+1 = 5, returned via out_index */ + TEST_ASSERT_EQUAL_UINT64(5, out_index); + /* log_len advanced by exactly one */ + TEST_ASSERT_EQUAL_UINT64(5, s.log_len); + + /* on_persist_entry called exactly once with (index=5, term=currentTerm, bytes, len) */ + TEST_ASSERT_EQUAL_INT(1, g_pe_spy.call_count); + TEST_ASSERT_EQUAL_UINT64(5, g_pe_spy.last_index); + TEST_ASSERT_EQUAL_UINT64(term_at_append, g_pe_spy.last_term); + TEST_ASSERT_TRUE(term_at_append != 0); /* not a zero placeholder (Req 2.4) */ + TEST_ASSERT_EQUAL_UINT64(sizeof(payload), g_pe_spy.last_len); + TEST_ASSERT_EQUAL_UINT8_ARRAY(payload, g_pe_spy.last_bytes, sizeof(payload)); +} + +/* + * test_log_append_follower_rejected (Req 2.2) + * + * FOLLOWER raft_log_append must return -EPERM with NO log change and NO callback. + */ +static void test_log_append_follower_rejected(void) +{ + raft_state_t s; + raft_callbacks_t cbs = make_mock_cbs(); + cbs.on_persist_entry = spy_persist_entry; + + reset_mock(); + reset_pe_spy(); + TEST_ASSERT_EQUAL_INT(0, init_node_log_len(&s, &cbs, 3, 0, 4)); + /* raft_init leaves the node RAFT_FOLLOWER */ + TEST_ASSERT_EQUAL_INT(RAFT_FOLLOWER, s.role); + + uint64_t out_index = 12345; /* sentinel: must remain untouched */ + const unsigned char payload[] = { 0x01 }; + int rc = raft_log_append(&s, payload, sizeof(payload), &out_index); + + TEST_ASSERT_EQUAL_INT(-EPERM, rc); + TEST_ASSERT_EQUAL_UINT64(4, s.log_len); /* no log change */ + TEST_ASSERT_EQUAL_INT(0, g_pe_spy.call_count); /* no callback */ +} + +/* + * test_log_append_candidate_rejected (Req 2.2) + * + * CANDIDATE raft_log_append must also return -EPERM (only RAFT_LEADER may append), + * with NO log change and NO callback. + */ +static void test_log_append_candidate_rejected(void) +{ + raft_state_t s; + raft_callbacks_t cbs = make_mock_cbs(); + cbs.on_persist_entry = spy_persist_entry; + + reset_mock(); + reset_pe_spy(); + TEST_ASSERT_EQUAL_INT(0, init_node_log_len(&s, &cbs, 3, 0, 4)); + TEST_ASSERT_EQUAL_INT(RAFT_OK, raft_start_election(&s)); + TEST_ASSERT_EQUAL_INT(RAFT_CANDIDATE, s.role); + + uint64_t out_index = 0; + const unsigned char payload[] = { 0x02 }; + int rc = raft_log_append(&s, payload, sizeof(payload), &out_index); + + TEST_ASSERT_EQUAL_INT(-EPERM, rc); + TEST_ASSERT_EQUAL_UINT64(4, s.log_len); + TEST_ASSERT_EQUAL_INT(0, g_pe_spy.call_count); +} + +/* + * test_log_append_persist_failure_propagates (Req 2.3, 2.5) + * + * When on_persist_entry returns non-zero, raft_log_append must: + * - propagate that exact non-zero return value + * - NOT advance log_len (no phantom entry — persist-before-act) + */ +static void test_log_append_persist_failure_propagates(void) +{ + raft_state_t s; + raft_callbacks_t cbs = make_mock_cbs(); + cbs.on_persist_entry = spy_persist_entry; + + reset_mock(); + reset_pe_spy(); + g_pe_spy.return_value = -EIO; /* persist fails */ + TEST_ASSERT_EQUAL_INT(0, init_node_log_len(&s, &cbs, 3, 0, 4)); + drive_to_leader(&s); + + uint64_t out_index = 0; + const unsigned char payload[] = { 0x03, 0x04 }; + int rc = raft_log_append(&s, payload, sizeof(payload), &out_index); + + /* error propagated verbatim */ + TEST_ASSERT_EQUAL_INT(-EIO, rc); + /* persist was attempted (persist-before-act) but log_len did NOT advance */ + TEST_ASSERT_EQUAL_INT(1, g_pe_spy.call_count); + TEST_ASSERT_EQUAL_UINT64(4, s.log_len); +} + /* ---------- raft_on_election_timeout tests (task 3.5) ---------- */ /* @@ -4117,6 +4288,12 @@ void test_raft(void) RUN_TEST(test_leader_progress_init_single_node); RUN_TEST(test_leader_progress_init_multi_node); + /* raft_log_append (task 3.1/3.2) */ + RUN_TEST(test_log_append_leader_assigns_index_stamps_term); + RUN_TEST(test_log_append_follower_rejected); + RUN_TEST(test_log_append_candidate_rejected); + RUN_TEST(test_log_append_persist_failure_propagates); + /* raft_on_vote_request (task 3.3) */ RUN_TEST(test_vote_req_stale_term); RUN_TEST(test_vote_req_higher_term_updates_state);