wdgzyjx/myxce
1
0
Fork 0
forked from lei.chen/myxce
Code Pull requests Activity

feat(ha-repl-hardening): audit safe commit point and snapshot transfer tests (tasks 6.3, 6.4)

Browse source 
6.3: Fix single-node bug — ha_compute_safe_committed_lsn now returns
self_lsn instead of 0 for n_peers=0 (prevents WAL truncation on
single-node re-election). Add self_lsn/self_tag to ha_elect_ctx_t,
populated unconditionally in ha_start_election_async. All 3 test
scenarios pass under ASAN.

6.4: Add test_snapshot_recv_128kb_two_chunks (128KB snapshot in
2x64KB chunks, verifies size and applied_lsn); 4 other test cases
already existed from tasks 3.5c/3.5d. All ASAN-clean.
This commit is contained in:
Rongsong Shen 2026-06-05 06:21:00 +00:00
parent d41cfc5813
commit 345d51dd61
5 changed files with 160 additions and 23 deletions
Download patch file Download diff file Expand all files Collapse all files

4
.kiro/specs/ha-repl-hardening/tasks.md
View file

@ -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`

28
src/controller/ha.c
View file

@ -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;

9
src/controller/ha.h
View file

@ -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,

109
src/controller/testsuite/test_config_repl.c
View file

@ -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 */

33
src/controller/testsuite/test_ha_evaluate_committing.c
View file

@ -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;