diff --git a/.kiro/specs/raft-adapter-conformance/tasks.md b/.kiro/specs/raft-adapter-conformance/tasks.md index eb28760..0f08b80 100644 --- a/.kiro/specs/raft-adapter-conformance/tasks.md +++ b/.kiro/specs/raft-adapter-conformance/tasks.md @@ -49,7 +49,7 @@ - _Depends: 3.1_ - [ ] 4. Core: document the dormant AppendEntries sender and guard it against silent regression -- [ ] 4.1 Record the production-authoritative send mechanism and add a non-regression guard +- [x] 4.1 Record the production-authoritative send mechanism and add a non-regression guard - Extend the `on_send_append_entries` doc-comment (`ha.c`, the `ha_cb_send_append_entries_stub` callback) to state plainly that this callback is reachable only via `raft_send_append_entries`, which has no production caller, and that `repl_broadcast`/`ha_stream_wal_to_standby` are the actual production `AppendEntries`-equivalent send/retry/backoff mechanism - Add an `XCE_TESTING`-only call counter to `ha_cb_send_append_entries_stub` - Observable: the counter exists and is queryable from a test; the doc-comment names both the dormant path and the live substitute explicitly diff --git a/src/controller/ha.c b/src/controller/ha.c index 9611a31..3771d9c 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -2533,12 +2533,68 @@ static int ha_read_wal_entry_at(const char *wal_path, uint64_t lsn, * Non-blocking (ha-async-io): delegates to repl_send_append_entries which * enqueues via the async repl transport — never blocks in the callback. * peer_idx is the library's raft peer index, mapped 1:1 to the repl standby - * registry slot (the same index match_index/next_index use). */ + * registry slot (the same index match_index/next_index use). + * + * DORMANT IN PRODUCTION (raft-adapter-conformance task 4.1, Requirement 3.3): + * this callback is wired as on_send_append_entries (see s_real_cbs below) and + * is fully protocol-conformant, but it is reachable ONLY via the raft + * library's raft_send_append_entries (src/lib/raft/raft.c:595), which has + * ZERO production call sites — every caller is either + * src/lib/raft/raft_sim.c (the pure library simulation harness) or the + * XCE_TESTING-only test helper ha_cb_send_append_entries_for_test (below). + * Nothing in the running controller ever DRIVES raft_send_append_entries, so + * this callback is registered but never invoked in production. + * + * The mechanism that ACTUALLY drives AppendEntries-equivalent + * send/retry/backoff in production today is a different, coarser + * substitute: repl_broadcast (src/controller/repl.c) fans out each + * freshly-appended WAL entry to every standby as it is written (retrying via + * ha_raft_on_repl_nack on a NACK), and ha_stream_wal_to_standby + * (src/controller/config_repl.c) streams a standby's catch-up range once it + * falls behind. Together these two are the sole production + * AppendEntries-equivalent send/retry/backoff mechanism for Requirement 3.3 + * — NOT this per-peer, nextIndex-driven sender. + * + * This is a deliberate documentation-only resolution, not an oversight: see + * research.md's "Documentation-only resolution for the dead nextIndex-driven + * AppendEntries sender (Requirement 3)" decision and + * src/controller/CLAUDE.md's TLA+ Action-to-Code Mapping for the recorded + * disposition. The dead path is kept as an intentionally-dormant, + * protocol-exact reference implementation (retiring or wiring it in were + * both rejected — see research.md) rather than silently left undocumented. + * Its dormancy is guarded against silent regression by the XCE_TESTING-only + * ha_send_append_entries_stub_call_count counter incremented immediately + * below: a future change that accidentally gives raft_send_append_entries a + * production caller moves this counter off zero, which task 4.2's + * non-regression test (test_ha_core_safety.c) asserts against while + * confirming repl_broadcast/ha_stream_wal_to_standby fire for the same + * scenario. + * + * See also the independently-derived reachability trace at + * test_ha_core_safety.c:2436-2464 and the ha_apply_single_entry comment + * above (raft-ha-core-safety task 6.2), which confirm the same + * zero-production-caller finding for this function's only caller + * (repl_send_append_entries). */ +#ifdef XCE_TESTING +/* ha_send_append_entries_stub_call_count (raft-adapter-conformance task 4.1, + * Requirement 3.3): non-regression guard counting invocations of + * ha_cb_send_append_entries_stub (the on_send_append_entries callback). + * Expected to stay at 0 across any test that drives only the production + * replication path (repl_broadcast / ha_stream_wal_to_standby) — see the + * DORMANT IN PRODUCTION comment above. External linkage under XCE_TESTING + * only, so a test can read/reset it directly (declared extern in ha.h, + * mirroring the ha_wal_ack_send_hook seam). NULL/0 semantics: always 0 in a + * production build (the whole symbol does not exist outside XCE_TESTING). */ +uint64_t ha_send_append_entries_stub_call_count = 0; +#endif static int ha_cb_send_append_entries_stub(void *ctx, int peer_idx, uint64_t prev_log_index, uint64_t prev_log_term, uint64_t leader_commit, int n_entries) { +#ifdef XCE_TESTING + ha_send_append_entries_stub_call_count++; +#endif ha_state_t *ha = (ha_state_t *)ctx; if (!ha || !ha->raft_c) return -EINVAL; /* test mode without a cluster: cannot send */ diff --git a/src/controller/ha.h b/src/controller/ha.h index 92b83c0..c26b5c7 100644 --- a/src/controller/ha.h +++ b/src/controller/ha.h @@ -1422,6 +1422,22 @@ void ha_cb_commit_advanced_for_test(ha_state_t *ha, uint64_t commit_index); * term via the library. TEST-ONLY. */ void ha_arm_own_term_gate_for_test(cluster_t *c); + +/* + * ha_send_append_entries_stub_call_count (raft-adapter-conformance task 4.1, + * Requirement 3.3): test-only seam counting invocations of + * ha_cb_send_append_entries_stub, the on_send_append_entries callback. That + * callback is reachable only via the raft library's raft_send_append_entries + * (zero production callers) or ha_cb_send_append_entries_for_test above — + * see the DORMANT IN PRODUCTION comment on ha_cb_send_append_entries_stub in + * ha.c for the full finding. A test that drives only the production + * replication path (repl_broadcast / ha_stream_wal_to_standby) must observe + * this counter stay at 0; a future change that accidentally gives + * raft_send_append_entries a production caller moves it off zero. NULL in + * production (the whole symbol does not exist outside XCE_TESTING). + * TEST-ONLY. + */ +extern uint64_t ha_send_append_entries_stub_call_count; #endif /* diff --git a/src/controller/testsuite/test_ha_core_safety.c b/src/controller/testsuite/test_ha_core_safety.c index e8514a5..cb42523 100644 --- a/src/controller/testsuite/test_ha_core_safety.c +++ b/src/controller/testsuite/test_ha_core_safety.c @@ -2645,6 +2645,38 @@ static void test_6_2_real_senders_never_produce_termblind_shape(void) printf("test_6_2_real_senders_never_produce_termblind_shape: done\n"); } +/* Task 4.1 (raft-adapter-conformance, Requirement 3.3) sanity check: this is + * NOT task 4.2's "drives the production heartbeat/replication path + * end-to-end and asserts the counter stays at zero" non-regression test — + * that test belongs to task 4.2 and lives separately. This is only a + * trivial proof that the counter itself works: it is directly + * readable/resettable from a test (XCE_TESTING extern linkage, ha.h) and + * increments by exactly one per invocation of ha_cb_send_append_entries_stub + * (driven here via the existing ha_cb_send_append_entries_for_test seam). + * ha==NULL still reaches the increment — it is the very first statement in + * the callback body, before the ha/raft_c NULL guard — so this needs no + * cluster/persist fixture at all. */ +static void test_4_1_send_append_entries_stub_call_count_increments(void) +{ + ha_send_append_entries_stub_call_count = 0; + + int rc = ha_cb_send_append_entries_for_test(NULL, /*peer*/0, + /*prev_idx*/0, /*prev_term*/0, + /*leader_commit*/0, /*n*/0); + TEST_ASSERT_EQUAL_INT(-EINVAL, rc); + TEST_ASSERT_EQUAL_UINT64_MESSAGE(1, ha_send_append_entries_stub_call_count, + "ha_send_append_entries_stub_call_count must increment once per " + "ha_cb_send_append_entries_stub invocation"); + + rc = ha_cb_send_append_entries_for_test(NULL, /*peer*/0, 0, 0, 0, 0); + TEST_ASSERT_EQUAL_INT(-EINVAL, rc); + TEST_ASSERT_EQUAL_UINT64_MESSAGE(2, ha_send_append_entries_stub_call_count, + "the counter must accumulate across multiple invocations, not just " + "latch at 1"); + + printf("test_4_1_send_append_entries_stub_call_count_increments: PASS\n"); +} + /* ── Suite entry point ─────────────────────────────────────────────────────*/ void test_ha_core_safety(void) @@ -2740,4 +2772,9 @@ void test_ha_core_safety(void) * above and in ha.c. */ RUN_TEST(test_6_2_restart_preserves_term_identity); RUN_TEST(test_6_2_real_senders_never_produce_termblind_shape); + /* Task 4.1 (raft-adapter-conformance) sanity check: the new + * ha_send_append_entries_stub_call_count non-regression-guard counter + * itself increments correctly. Task 4.2 owns the fuller "stays at zero + * across a real production replication drive" assertion. */ + RUN_TEST(test_4_1_send_append_entries_stub_call_count_increments); }