diff --git a/.kiro/specs/ha-repl-hardening/tasks.md b/.kiro/specs/ha-repl-hardening/tasks.md index a68a981..990fa4a 100644 --- a/.kiro/specs/ha-repl-hardening/tasks.md +++ b/.kiro/specs/ha-repl-hardening/tasks.md @@ -219,14 +219,14 @@ - All tests pass under ASAN - _Requirements: 3.1, 3.3, 3.4, 3.5, 5.3, 5.5_ -- [ ] 6.3 Unit tests for safe commit point from vote replies +- [x] 6.3 Unit tests for safe commit point from vote replies - Test: voters at LSNs `[99, 100]`, self at `100` → `safe_committed_lsn=100` - Test: voters at LSNs `[50, 51]`, self at `50` → `safe_committed_lsn=51` (majority has 51) - Test: single node → `safe_committed_lsn = self.applied_lsn` - All tests pass under ASAN - _Requirements: 2.3_ -- [ ] 6.4 Unit tests for snapshot chunk transfer +- [x] 6.4 Unit tests for snapshot chunk transfer - Test: 128KB snapshot in 2 chunks → standby receives, verifies total size, atomically renames, loads; `applied_lsn = checkpoint_lsn` - Test: mid-transfer reconnect → standby discards `snap.part`, primary restarts from chunk 0 on reconnect - Test: snapshot transfer only triggered when `standby_lsn < oldest_wal_lsn` diff --git a/src/controller/ha.c b/src/controller/ha.c index 13acdef..0b133f6 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -348,14 +348,15 @@ static void raft_cb_start_election_after_delay(void *ctx, uint32_t delay_ms) * * Algorithm (design §Flow 0: Post-Election — Determine Safe Commit Point): * 1. Collect LSNs from all granted voters: - * - Self always granted: self_lsn = votes[0].my_lsn (when n_peers > 0) + * - Self always granted: self_lsn = ctx->self_lsn (stored unconditionally) * - Each peer with vote_granted=1: lsn = votes[i].peer_lsn * 2. Sort granted LSNs descending. * 3. safe_committed_lsn = sorted[floor(n_granted / 2)] * (the majority-threshold element — the highest LSN held by a majority) - * 4. Same logic for config tags (votes[i].my_tag / votes[i].peer_tag). + * 4. Same logic for config tags (ctx->self_tag / votes[i].peer_tag). * - * Single-node (n_peers == 0): votes[] is empty, outputs 0 for both fields. + * Single-node (n_peers == 0): votes[] is empty; safe_committed_lsn = ctx->self_lsn. + * This prevents WAL truncation to 0 on re-election in a single-node cluster. * * Requirements: 2.3 (ha-repl-hardening spec) * Boundary: Controller FSM (ha.c) @@ -375,10 +376,11 @@ void ha_compute_safe_committed_lsn(ha_elect_ctx_t *ctx, { if (!ctx || !safe_lsn_out || !safe_tag_out) return; - /* Single-node: no votes[] entries — self is the only majority. */ + /* Single-node: no votes[] entries — self is the only majority. + * Return self_lsn so WAL is not truncated to 0 on re-election. */ if (ctx->n_peers == 0) { - *safe_lsn_out = 0; - *safe_tag_out = 0; + *safe_lsn_out = ctx->self_lsn; + *safe_tag_out = ctx->self_tag; return; } @@ -397,9 +399,9 @@ void ha_compute_safe_committed_lsn(ha_elect_ctx_t *ctx, int n_granted = 0; - /* Self always granted: self's lsn/tag lives in votes[0].my_lsn / my_tag */ - lsns[n_granted] = ctx->votes[0].my_lsn; - tags[n_granted] = ctx->votes[0].my_tag; + /* Self always granted: self's lsn/tag from the dedicated self_lsn/self_tag fields */ + lsns[n_granted] = ctx->self_lsn; + tags[n_granted] = ctx->self_tag; n_granted++; /* Each peer with vote_granted=1 */ @@ -3073,7 +3075,13 @@ void ha_start_election_async(ha_state_t *ha, cluster_t *c, pthread_mutex_init(&ctx->lock, NULL); sem_init(&ctx->done, 0, 0); - /* Store candidate's own tag/lsn; votes[0] is read by raft_cb_send_vote_request */ + /* Store candidate's own tag/lsn unconditionally in self_lsn/self_tag (task 6.3): + * ha_compute_safe_committed_lsn uses these for the majority computation, + * including the single-node case where votes[] is empty. */ + ctx->self_lsn = my_lsn; + ctx->self_tag = my_config_tag; + + /* Also store in votes[0] for raft_cb_send_vote_request (multi-node only) */ if (n_other > 0) { ctx->votes[0].my_tag = my_config_tag; ctx->votes[0].my_lsn = my_lsn; diff --git a/src/controller/ha.h b/src/controller/ha.h index 4e7dbce..19a7f1f 100644 --- a/src/controller/ha.h +++ b/src/controller/ha.h @@ -108,6 +108,11 @@ typedef struct { * cluster_init_phase2(c) and arm the committing timeout watchdog. * NULL in unit tests (c=NULL bypasses phase2 and watchdog arming). */ cluster_t *c; + /* Task 6.3: candidate's own LSN/tag, stored unconditionally so that + * ha_compute_safe_committed_lsn can use them even for single-node elections + * where votes[] is empty (n_peers=0). */ + uint64_t self_lsn; /* Candidate's own WAL LSN at election start */ + uint64_t self_tag; /* Candidate's own config_tag at election start */ ha_vote_entry_t votes[]; /* C99 flexible array: n_peers-1 entries */ } ha_elect_ctx_t; @@ -297,9 +302,11 @@ void ha_nop_reply_state_report(ha_state_t *ha, myconf_t *myconf, * Collects LSNs from all granted voters (self + peers with vote_granted=1), * sorts descending, and returns the majority-threshold element. * - * single-node (n_peers=0): outputs 0 for both fields (no vote data available). + * single-node (n_peers=0): returns ctx->self_lsn and ctx->self_tag directly — + * self is the only majority; returning 0 would truncate all WAL on re-election. * * Requirements: 2.3 (ha-repl-hardening spec) + * Task 6.3: single-node fix (self_lsn/self_tag fields added to ha_elect_ctx_t) */ void ha_compute_safe_committed_lsn(ha_elect_ctx_t *ctx, uint64_t *safe_lsn_out, diff --git a/src/controller/testsuite/test_config_repl.c b/src/controller/testsuite/test_config_repl.c index aa410e0..452dbc7 100644 --- a/src/controller/testsuite/test_config_repl.c +++ b/src/controller/testsuite/test_config_repl.c @@ -1932,6 +1932,113 @@ static void test_snapshot_recv_null_args_fail(void) printf("test_snapshot_recv_null_args_fail: PASS\n"); } +/* Test 6.4-A: 128KB snapshot in 2 equal chunks (65536 bytes each) — standby + * receives both chunks, verifies total_size matches, snap.part is atomically + * renamed to snap-{lsn}.bin, persist_load_snapshot is called successfully, + * and applied_lsn equals the checkpoint_lsn embedded in the snapshot. + * + * This is the primary task-6.4 coverage test for Req 4.2, 4.3. + * Uses the same 65536-byte chunk size as the sender test + * (test_snapshot_transfer_two_chunks) to exercise the full receive path. */ +static void test_snapshot_recv_128kb_two_chunks(void) +{ + char tmpdir[] = "/tmp/xcectest_snap128kb_XXXXXX"; + TEST_ASSERT_NOT_NULL(mkdtemp(tmpdir)); + + char snap_dir[512]; + snprintf(snap_dir, sizeof(snap_dir), "%s/snap", tmpdir); + TEST_ASSERT_EQUAL_INT(0, mkdir(snap_dir, 0700)); + + persist_t *p = calloc(1, sizeof(persist_t)); + TEST_ASSERT_NOT_NULL(p); + persist_config_t pcfg = persist_config_defaults(); + persist_set_workdir(p, tmpdir, "ctrl-snap128kb"); + persist_init(p, &pcfg, NULL); + persist_start(p); + + ha_state_t ha; + snap_recv_ha_init(&ha); + + /* 128KB snapshot split into two 65536-byte chunks */ + const uint64_t snap_lsn = 0x64; /* LSN 100 — matches sender test */ + const size_t chunk_size = 65536; + const size_t total_size = 131072; /* 128 * 1024 */ + + /* Allocate and zero-fill chunks */ + uint8_t *chunk0 = calloc(1, chunk_size); + uint8_t *chunk1 = calloc(1, chunk_size); + TEST_ASSERT_NOT_NULL(chunk0); + TEST_ASSERT_NOT_NULL(chunk1); + + /* Embed valid XCESNAP header in chunk0 so persist_load_snapshot succeeds */ + memcpy(chunk0, "XCESNAP\0", 8); + memcpy(chunk0 + 8, &snap_lsn, 8); + + /* ── Chunk 0: not last ── */ + snapshot_transfer_t c0; + memset(&c0, 0, sizeof(c0)); + c0.snapshot_lsn = snap_lsn; + c0.chunk_index = 0; + c0.chunk_offset = 0; + c0.total_size = (uint64_t)total_size; + c0.is_last = 0; + + int rc = ha_handle_snapshot_chunk(&ha, p, snap_dir, &c0, chunk0, chunk_size); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, rc, "chunk 0 must succeed"); + + /* snap.part must exist and be exactly chunk_size bytes */ + char part_path[600]; + snprintf(part_path, sizeof(part_path), "%s/snap.part", snap_dir); + struct stat st; + TEST_ASSERT_EQUAL_INT_MESSAGE(0, stat(part_path, &st), + "snap.part must exist after chunk 0"); + TEST_ASSERT_EQUAL_UINT64_MESSAGE((uint64_t)chunk_size, (uint64_t)st.st_size, + "snap.part must be chunk_size bytes after chunk 0"); + + /* ── Chunk 1: isLast=true ── */ + snapshot_transfer_t c1; + memset(&c1, 0, sizeof(c1)); + c1.snapshot_lsn = snap_lsn; + c1.chunk_index = 1; + c1.chunk_offset = (uint64_t)chunk_size; + c1.total_size = (uint64_t)total_size; + c1.is_last = 1; + + rc = ha_handle_snapshot_chunk(&ha, p, snap_dir, &c1, chunk1, chunk_size); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, rc, "chunk 1 (isLast) must succeed"); + + /* snap.part must be gone (atomically renamed) */ + TEST_ASSERT_NOT_EQUAL_MESSAGE(0, stat(part_path, &st), + "snap.part must be gone after isLast rename"); + + /* Final file snap-{lsn:016x}.bin must exist with total_size bytes */ + char final_path[600]; + snprintf(final_path, sizeof(final_path), + "%s/snap-%016" PRIx64 ".bin", snap_dir, snap_lsn); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, stat(final_path, &st), + "snap-{lsn}.bin must exist after isLast"); + TEST_ASSERT_EQUAL_UINT64_MESSAGE((uint64_t)total_size, (uint64_t)st.st_size, + "snap-{lsn}.bin must be exactly total_size bytes"); + + /* applied_lsn must equal checkpoint_lsn (snap_lsn) after successful load */ + uint64_t got_lsn = (uint64_t)atomic_load_explicit(&ha.applied_lsn, + memory_order_acquire); + TEST_ASSERT_EQUAL_UINT64_MESSAGE(snap_lsn, got_lsn, + "applied_lsn must equal checkpoint_lsn after load"); + + free(chunk0); + free(chunk1); + ha_fini(&ha); + persist_stop(p); + persist_fini(p); + free(p); + + char cmd[512]; + snprintf(cmd, sizeof(cmd), "rm -rf %s", tmpdir); + system(cmd); + printf("test_snapshot_recv_128kb_two_chunks: PASS\n"); +} + /* Test 3.5d: reconnected standby discards snap.part and restarts from chunk 0. * * Design §Snapshot Transfer: "Mid-transfer reconnect always restarts snapshot @@ -2196,6 +2303,8 @@ void test_config_repl(void) RUN_TEST(test_snapshot_recv_two_chunks_assembles_and_renames); RUN_TEST(test_snapshot_recv_size_mismatch_fails); RUN_TEST(test_snapshot_recv_null_args_fail); + /* Task 6.4-A: 128KB snapshot in 2 chunks — full-size receive path */ + RUN_TEST(test_snapshot_recv_128kb_two_chunks); /* Task 3.5d: reconnect discards snap.part */ RUN_TEST(test_snapshot_recv_reconnect_discards_snappart); /* Task 5.1: handle_ctrlhello connection setup only — no repl messages */ diff --git a/src/controller/testsuite/test_ha_evaluate_committing.c b/src/controller/testsuite/test_ha_evaluate_committing.c index b6cab06..0394cd6 100644 --- a/src/controller/testsuite/test_ha_evaluate_committing.c +++ b/src/controller/testsuite/test_ha_evaluate_committing.c @@ -466,7 +466,9 @@ static void test_safe_committed_lsn_two_peers_at_99_and_100(void) pthread_mutex_init(&ctx->lock, NULL); sem_init(&ctx->done, 0, 0); - /* Self data stored in votes[0].my_lsn / my_tag */ + /* Self data: self_lsn/self_tag (task 6.3) and votes[0].my_lsn/my_tag */ + ctx->self_lsn = 100; + ctx->self_tag = 5; ctx->votes[0].my_lsn = 100; ctx->votes[0].my_tag = 5; ctx->votes[0].vote_granted = 1; /* peer0: granted, lsn=99 */ @@ -511,6 +513,8 @@ static void test_safe_committed_lsn_two_peers_at_50_and_51(void) pthread_mutex_init(&ctx->lock, NULL); sem_init(&ctx->done, 0, 0); + ctx->self_lsn = 100; + ctx->self_tag = 5; ctx->votes[0].my_lsn = 100; ctx->votes[0].my_tag = 5; ctx->votes[0].vote_granted = 1; /* peer0: granted, lsn=50 */ @@ -537,30 +541,34 @@ static void test_safe_committed_lsn_two_peers_at_50_and_51(void) /* * Test case C: single-node cluster (n_peers=0, no votes[] entries). - * safe_committed_lsn must equal self's own LSN (self is the only majority). + * safe_committed_lsn must equal self's own applied_lsn (self is the only majority). + * Req 2.3 / task 6.3: returning 0 would truncate all WAL on re-election — wrong. + * The fix (task 6.3): ha_elect_ctx_t gains self_lsn/self_tag fields populated + * unconditionally by ha_start_election_async; ha_compute_safe_committed_lsn + * returns ctx->self_lsn for the n_peers=0 case. */ static void test_safe_committed_lsn_single_node(void) { #ifdef XCE_TESTING - /* n_other=0: no votes[] entries; but we still need my_lsn from... somewhere. - * For the single-node case, ctx->n_peers=0 and votes[] is empty. - * The function should use 0 as safe_lsn (no entries to check). */ ha_elect_ctx_t *ctx = calloc(1, sizeof(*ctx)); TEST_ASSERT_NOT_NULL(ctx); ctx->n_peers = 0; ctx->votes_granted = 1; /* self-vote only */ ctx->n_replied = 0; + /* Task 6.3: self_lsn/self_tag populated unconditionally */ + ctx->self_lsn = 77; + ctx->self_tag = 3; pthread_mutex_init(&ctx->lock, NULL); sem_init(&ctx->done, 0, 0); uint64_t safe_lsn = 0xDEAD, safe_tag = 0xDEAD; ha_compute_safe_committed_lsn(ctx, &safe_lsn, &safe_tag); - /* For single-node, safe_lsn should be 0 (no peer lsns to compare with) */ - TEST_ASSERT_EQUAL_UINT64_MESSAGE(0, safe_lsn, - "single-node (n_peers=0): safe_committed_lsn must be 0 (no peer data)"); - TEST_ASSERT_EQUAL_UINT64_MESSAGE(0, safe_tag, - "single-node (n_peers=0): safe_config_tag must be 0 (no peer data)"); + /* Single-node: safe_committed_lsn must equal self_lsn (not 0) */ + TEST_ASSERT_EQUAL_UINT64_MESSAGE(77, safe_lsn, + "single-node (n_peers=0): safe_committed_lsn must equal self_lsn (77), not 0"); + TEST_ASSERT_EQUAL_UINT64_MESSAGE(3, safe_tag, + "single-node (n_peers=0): safe_config_tag must equal self_tag (3), not 0"); free_ctx(ctx); printf("test_safe_committed_lsn_single_node: PASS (safe_lsn=%" PRIu64 ")\n", safe_lsn); @@ -587,6 +595,8 @@ static void test_safe_committed_lsn_one_peer_denied(void) pthread_mutex_init(&ctx->lock, NULL); sem_init(&ctx->done, 0, 0); + ctx->self_lsn = 200; + ctx->self_tag = 7; ctx->votes[0].my_lsn = 200; ctx->votes[0].my_tag = 7; ctx->votes[0].vote_granted = 0; /* peer denied vote */ @@ -669,6 +679,9 @@ static void test_become_leader_sets_committed_lsn_to_safe_lsn(void) pthread_mutex_init(&ctx->lock, NULL); sem_init(&ctx->done, 0, 0); + /* Self data in self_lsn/self_tag (task 6.3) and votes[0].my_lsn/my_tag */ + ctx->self_lsn = 200; + ctx->self_tag = 5; /* votes[0]: self data (my_lsn=200) + peer0 granted at lsn=100 */ ctx->votes[0].my_lsn = 200; ctx->votes[0].my_tag = 5;