lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

test(config-upgrade-snapshot): prove WAL-trim safety + add fault-injection seam for marker trigger

Browse source 
Confirmed task 5's existing 6 tests already satisfy "exactly once",
"safe under forced retry", and "does not re-home entities"; confirmed
test_snapshot_recv_old_format_version_rejected_fail_loud (task 1)
already covers format-mismatch rejection by proxy through
load_snapshot's two shared callers (network-install, boot replay).

Adds the two genuine gaps: (1)
test_marker_triggered_checkpoint_trim_preserves_standby_mid_catchup_segments
proves trim_wal_segments/repl_trim_lsn's standby-cursor retention
guard (bug-1147) still holds when reached via the new marker-triggered
checkpoint path, not just the old periodic/direct-call path -- guard
itself untouched. (2) persist_marker_checkpoint_fault_hook
(#ifdef XCE_TESTING, NULL default, true no-op in production) gates
whether persist_checkpoint_marker_triggered runs, for task 8.7's later
crash-before-checkpoint-durable injection; minimally proven here.
Task 8.1 of 19.

Requirements: 1.1, 1.5, 1.8, 5.2, 5.3, 5.6, 5.7, 6.3, 7.3, 7.4
This commit is contained in:
Rongsong Shen 2026-07-25 06:33:11 +00:00
parent 41af05402d
commit 4aeae6e282
3 changed files with 291 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

2
.kiro/specs/config-upgrade-snapshot/tasks.md
View file

@ -78,7 +78,7 @@
- _Depends: 5_
- [ ] 8. Validation: regression coverage for every new mechanism
- [ ] 8.1 (P) Unit-test the marker-apply trigger and its interaction with WAL retention
- [x] 8.1 (P) Unit-test the marker-apply trigger and its interaction with WAL retention
- Assert the configuration-marker case fires a checkpoint exactly once per apply and is safe under a forced retry
- Assert the trigger does not itself re-home entities
- Add a deterministic fault-injection seam at the marker-apply-to-checkpoint-durable boundary, mirroring the existing fault-hook pattern already used elsewhere in this codebase, for later chaotic-scenario use

42
src/controller/persist.c
View file

@ -3369,6 +3369,33 @@ int apply_wal_entry(persist_t* p, uint32_t type, uint64_t lsn,
* SYNCHRONOUSLY on this exact call stack, from this exact function. */
static _Thread_local bool t_apply_seam_notifying = false;
#ifdef XCE_TESTING
/* config-upgrade-snapshot task 8.1 (Req 6.3): deterministic fault-injection
* seam at the marker-apply-to-checkpoint-durable boundary, for later use by
* a crash/restart regression (task 8.7): "force a controller crash/restart
* between a configuration-change marker's commit and that member's own
* snapshot write becoming durable, and assert materialization resumes
* correctly from the marker on restart." When non-NULL AND it returns
* nonzero for the marker's own (p, lsn), the WAL_CONFIG_MARKER case below
* skips its call into persist_checkpoint_marker_triggered() entirely --
* simulating "the process crashes here, before the checkpoint write becomes
* durable" (a real crash simply stops the process; the checkpoint call
* never runs either way). The skip is deliberately routed through the SAME
* rc!=0 handling an actual checkpoint failure already takes below (not a
* new branch): last_applied_lsn is not advanced past this marker, so the
* next kick/retry -- or, after a genuine restart, boot replay's independent
* call into this same case (already proven idempotent by
* test_marker_apply_retry_is_idempotent) -- resumes materialization from
* the marker fresh, exactly matching Requirement 5.2's crash-safety
* contract. NULL in production -- no behaviour change. Mirrors
* ha_stream_body_read_fault_hook's shape (config_repl.c) and this same
* spec's own xce_upgrade_marker_committed_hook placement convention
* (defined here at file scope, declared extern in whichever test file
* needs it -- never in a header). */
int (*persist_marker_checkpoint_fault_hook)(persist_t *p,
uint64_t marker_lsn) = NULL;
#endif
int apply_committed_entry(persist_t* p, uint32_t wal_type, uint64_t lsn,
const void* payload, uint32_t len)
{
@ -3780,7 +3807,20 @@ int apply_committed_entry(persist_t* p, uint32_t wal_type, uint64_t lsn,
* checkpoint. `lsn` (this function's own parameter) is
* already the marker's exact position, so it is passed
* through explicitly instead of re-derived. */
int rc = persist_checkpoint_marker_triggered(p, lsn);
int rc;
#ifdef XCE_TESTING
if (persist_marker_checkpoint_fault_hook != NULL &&
persist_marker_checkpoint_fault_hook(p, lsn)) {
/* Simulated crash (task 8.1, Req 6.3): skip the checkpoint call
* entirely and fall into the SAME genuine-failure handling below
* a real checkpoint I/O error already takes -- see the fault
* hook's own doc comment above for why that is the correct,
* not merely convenient, simulation of "the process never got
* to checkpoint". */
rc = -ECANCELED;
} else
#endif
rc = persist_checkpoint_marker_triggered(p, lsn);
if (rc == -EBUSY) {
/* Expected, self-resolving (Req 4.4/5.3): a concurrent
* WAL-threshold-triggered or earlier marker-triggered

249
src/controller/testsuite/test_upgrade_snapshot_apply.c
View file

@ -66,6 +66,7 @@
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
#include <unistd.h>
#include <unity/unity.h>
@ -74,6 +75,11 @@
#include "hook.h"
#include "jobj.h"
#include "persist.h"
#include "repl.h" /* repl_add_standby / repl_on_ack -- Gap-1 WAL-trim-
* under-marker-trigger test (task 8.1, Req 7.4),
* mirrors test_e2e_checkpoint.c's own include */
#include "typemap.h" /* wal_catchup_entry_t -- repl_broadcast_send_hook's
* signature */
#include "wal_types.h"
#include "xce/cluster_def.h"
#include "xce/types.h"
@ -185,6 +191,37 @@ static int tusa_count_snapshots(persist_t* p)
return n;
}
/* Drop hook for repl_broadcast: a fixture that registers a standby with a
* dummy (int) endpoint via repl_add_standby must install this so
* persist_wal_async -> repl_broadcast does not crash trying to send to the
* fake endpoint. Pattern from test_e2e_checkpoint.c (itself from
* test_commit_engine.c / test_p2_integration.c) -- reused here, not
* duplicated logic, for the Gap-1 WAL-trim-under-marker-trigger test
* (task 8.1, Req 7.4). */
static int tusa_drop_broadcast(void* ep, wal_catchup_entry_t** entries,
int count)
{
(void)ep;
(void)entries;
(void)count;
return 0;
}
extern int (*repl_broadcast_send_hook)(void* ep,
wal_catchup_entry_t** entries,
int count);
/* Checks for a WAL segment file "seg-<lsn16>.wal" at seg_start_lsn inside
* wal_dir -- mirrors test_e2e_checkpoint.c's own seg_file_exists helper
* exactly (same naming convention the production trim/rotate code uses). */
static bool tusa_seg_file_exists(const char* wal_dir, uint64_t seg_start_lsn)
{
char path[4096];
snprintf(path, sizeof(path), "%s/seg-%016" PRIx64 ".wal",
wal_dir, seg_start_lsn);
return access(path, F_OK) == 0;
}
static int tusa_count_entries(var mgr)
{
if (!mgr) return 0;
@ -605,6 +642,216 @@ static void test_marker_apply_via_replay_checkpoints_at_marker_own_lsn(void)
printf("test_marker_apply_via_replay_checkpoints_at_marker_own_lsn: PASS\n");
}
/* ── 7: WAL-trim safety under the marker-triggered path (Req 7.4) ───────
*
* Six existing tests already prove trim_wal_segments()'s standby-cursor
* retention guard (repl_trim_lsn(), bug-1147) holds under the OLD trigger
* -- a direct/periodic persist_checkpoint() call:
* test_e2e_checkpoint.c's test_trim_guard_preserves_standby_segments, plus
* repl_trim_lsn()'s own unit coverage in test_repl_unit.c /
* test_p2_integration.c. None of them drive that SAME guard through this
* spec's NEW trigger -- a WAL_CONFIG_MARKER committed and applied via the
* real apply_committed_entry()/persist_apply_pump_run() path, which
* reaches the identical, unmodified trim_wal_segments()/repl_trim_lsn()
* code via persist_checkpoint_marker_triggered() instead of a bare
* persist_checkpoint() call. That is a real, previously-flagged coverage
* gap (task 5's own implementation notes), not something covered by proxy
* -- this test closes it.
*
* Uses this file's own real-cluster tusa_t fixture, not
* test_e2e_checkpoint.c's cluster-less make_ckpt_e2e() -- apply_committed_
* entry() short-circuits to a no-op whenever p->cluster is NULL, so a
* cluster-less fixture would never actually reach persist_checkpoint_
* marker_triggered() at all and could not exercise this path. WAL_NOOP is
* used for every filler entry (rather than test_e2e_checkpoint.c's
* WAL_JOB_UPDATE) because that file's fixture has no cluster to apply a
* job update against; WAL_NOOP is safe against a real cluster (default:
* no-op case), exactly as tusa_append_noop_then_marker() already relies on
* elsewhere in this file.
*
* Layout mirrors test_trim_guard_preserves_standby_segments' own LSN
* numbering exactly, for direct comparability:
* noop @ lsn1=1 → force segment (seg-1 closes, seg-2 opens @ lsn2=2)
* noop @ lsn2=2 → standby acks lsn2 (mid-catch-up: this is the LAST
* position it has acked -- in particular it has NOT
* acked the marker below)
* → force segment (seg-2 closes, seg-3 opens @ lsn3=3,
* stays active -- no further force call)
* noop @ lsn3=3 → keeps last_applied_lsn genuinely behind the marker's
* own lsn at dispatch time (same discipline this file's
* other tests already document as Hazard 2)
* WAL_CONFIG_MARKER @ marker_lsn=4 → committed → applied via
* persist_apply_pump_run(), which dispatches into apply_committed_
* entry()'s WAL_CONFIG_MARKER case → persist_checkpoint_marker_
* triggered() → trim_wal_segments().
*
* floor = repl_trim_lsn(active_seg_lsn=3, standby acked=2) = min(3,2) = 2:
* seg-1 (start=1) < floor → MUST be trimmed
* seg-2 (start=2) == floor → MUST be preserved (the standby still needs
* it for catch-up -- exactly bug-1147's guard)
* seg-3 (start=3, active) → MUST always be preserved
*/
static void
test_marker_triggered_checkpoint_trim_preserves_standby_mid_catchup_segments(void)
{
tusa_t t;
tusa_setup(&t);
repl_broadcast_send_hook = tusa_drop_broadcast;
int dummy_ep = 0;
int standby_idx = repl_add_standby(&t.p.repl, (void*)&dummy_ep,
"standby1", 7001);
int rc_noop1 = persist_wal_async(&t.p, WAL_NOOP, NULL, 0);
uint64_t lsn1 = t.p.current_lsn - 1;
int rc_force1 = persist_test_force_new_segment(&t.p);
int rc_noop2 = persist_wal_async(&t.p, WAL_NOOP, NULL, 0);
uint64_t lsn2 = t.p.current_lsn - 1;
repl_on_ack(&t.p.repl, (void*)&dummy_ep, lsn2);
int rc_force2 = persist_test_force_new_segment(&t.p);
uint64_t active_seg_lsn = t.p.current_lsn; /* seg-3 opens here */
int rc_noop3 = persist_wal_async(&t.p, WAL_NOOP, NULL, 0);
uint8_t sha[32];
memset(sha, 0xA7, sizeof(sha));
uint8_t marker_payload[WAL_CONFIG_MARKER_PAYLOAD_LEN];
size_t out_len = 0;
int rc_encode = config_marker_encode(0x1122334455667788ULL ^ 0xA7, sha,
marker_payload,
WAL_CONFIG_MARKER_PAYLOAD_LEN,
&out_len);
int rc_marker = persist_wal_async(&t.p, WAL_CONFIG_MARKER, marker_payload,
WAL_CONFIG_MARKER_PAYLOAD_LEN);
uint64_t marker_lsn = t.p.current_lsn - 1;
uint64_t committed_after =
persist_committed_lsn_advance_atomic(&t.p, marker_lsn);
int applied = persist_apply_pump_run(&t.p);
bool seg1_exists = tusa_seg_file_exists(t.p.wal_path, lsn1);
bool seg2_exists = tusa_seg_file_exists(t.p.wal_path, lsn2);
bool seg3_exists = tusa_seg_file_exists(t.p.wal_path, active_seg_lsn);
bool snap_at_marker = tusa_snapshot_exists(&t.p, marker_lsn);
repl_broadcast_send_hook = NULL;
tusa_teardown(&t);
TEST_ASSERT_TRUE(standby_idx >= 0);
TEST_ASSERT_EQUAL_INT(0, rc_noop1);
TEST_ASSERT_EQUAL_INT(0, rc_force1);
TEST_ASSERT_EQUAL_INT(0, rc_noop2);
TEST_ASSERT_EQUAL_INT(0, rc_force2);
TEST_ASSERT_EQUAL_INT(0, rc_noop3);
TEST_ASSERT_EQUAL_INT(0, rc_encode);
TEST_ASSERT_EQUAL_INT(0, rc_marker);
TEST_ASSERT_EQUAL_UINT64(1, lsn1);
TEST_ASSERT_EQUAL_UINT64(2, lsn2);
TEST_ASSERT_EQUAL_UINT64(3, active_seg_lsn);
TEST_ASSERT_EQUAL_UINT64(4, marker_lsn);
TEST_ASSERT_EQUAL_UINT64(marker_lsn, committed_after);
TEST_ASSERT_EQUAL_INT_MESSAGE(4, applied,
"all four entries (3 noops + the marker) must apply successfully");
TEST_ASSERT_TRUE_MESSAGE(snap_at_marker,
"the marker-triggered checkpoint must still fire and produce a "
"snapshot even with a standby mid-catch-up");
TEST_ASSERT_FALSE_MESSAGE(seg1_exists,
"seg-1: below the standby's mid-catch-up acked_lsn=2, MUST be "
"trimmed by the marker-triggered checkpoint");
TEST_ASSERT_TRUE_MESSAGE(seg2_exists,
"seg-2: at the standby's mid-catch-up acked_lsn=2, MUST be "
"preserved (bug-1147 guard) even when trim runs via the NEW "
"marker-triggered path");
TEST_ASSERT_TRUE_MESSAGE(seg3_exists,
"seg-3: the active segment, MUST always be preserved");
printf("test_marker_triggered_checkpoint_trim_preserves_standby_"
"mid_catchup_segments: PASS\n");
}
/* ── 8: fault-injection seam at the marker-apply→checkpoint-durable
* boundary (Req 6.3) ───────────────────────────────────────────────
*
* Proves the new persist_marker_checkpoint_fault_hook seam (persist.c,
* added by this task) fires exactly once for the marker's own (p, lsn) and
* correctly gates whether persist_checkpoint_marker_triggered() is called
* at all. This is deliberately ONLY the seam itself, not the full
* crash/restart regression -- that is task 8.7's job, which depends on
* this seam landing first.
*/
typedef struct {
int calls;
persist_t* last_p;
uint64_t last_lsn;
} tusa_fault_obs_t;
static tusa_fault_obs_t g_tusa_fault_obs;
static int tusa_marker_fault_hook(persist_t* p, uint64_t marker_lsn)
{
g_tusa_fault_obs.calls++;
g_tusa_fault_obs.last_p = p;
g_tusa_fault_obs.last_lsn = marker_lsn;
return 1; /* simulate "the process never got to checkpoint" */
}
static void test_marker_checkpoint_fault_hook_gates_checkpoint_call(void)
{
tusa_t t;
extern int (*persist_marker_checkpoint_fault_hook)(persist_t* p,
uint64_t marker_lsn);
tusa_setup(&t);
memset(&g_tusa_fault_obs, 0, sizeof(g_tusa_fault_obs));
persist_marker_checkpoint_fault_hook = tusa_marker_fault_hook;
tusa_append_result_t ar = tusa_append_noop_then_marker(&t.p, 0x17);
int apply_rc = persist_apply_pump_run(&t.p);
bool snap_at_marker = tusa_snapshot_exists(&t.p, ar.marker_lsn);
uint64_t last_applied = atomic_load_explicit(&t.p.last_applied_lsn,
memory_order_acquire);
int fault_calls = g_tusa_fault_obs.calls;
uint64_t fault_lsn = g_tusa_fault_obs.last_lsn;
bool fault_p_matches = (g_tusa_fault_obs.last_p == &t.p);
persist_marker_checkpoint_fault_hook = NULL;
tusa_teardown(&t);
TEST_ASSERT_EQUAL_INT(0, ar.rc_encode);
TEST_ASSERT_EQUAL_INT(0, ar.rc_noop);
TEST_ASSERT_EQUAL_INT(0, ar.rc_marker);
TEST_ASSERT_EQUAL_INT_MESSAGE(1, fault_calls,
"the fault hook must fire exactly once, for the marker's own "
"checkpoint attempt");
TEST_ASSERT_EQUAL_UINT64(ar.marker_lsn, fault_lsn);
TEST_ASSERT_TRUE_MESSAGE(fault_p_matches,
"the fault hook must observe the SAME persist_t the marker is "
"being applied to");
TEST_ASSERT_EQUAL_INT_MESSAGE(-ECANCELED, apply_rc,
"a nonzero fault-hook return must propagate as the marker's apply "
"failure, taking the SAME halt-and-retry path a genuine checkpoint "
"I/O error already takes");
TEST_ASSERT_FALSE_MESSAGE(snap_at_marker,
"a nonzero fault-hook return must skip the checkpoint call "
"entirely -- no snapshot file at the marker's lsn");
TEST_ASSERT_EQUAL_UINT64_MESSAGE(ar.noop_lsn, last_applied,
"last_applied_lsn must NOT advance past the marker when the "
"checkpoint is skipped by the fault hook -- materialization must "
"resume from the marker on the next attempt");
printf("test_marker_checkpoint_fault_hook_gates_checkpoint_call: "
"PASS\n");
}
/* ── Dispatch ────────────────────────────────────────────────────────── */
void test_upgrade_snapshot_apply(void)
@ -618,4 +865,6 @@ void test_upgrade_snapshot_apply(void)
RUN_TEST(test_marker_apply_busy_checkpoint_handled_gracefully);
RUN_TEST(test_marker_apply_does_not_reinterpret_entities);
RUN_TEST(test_marker_apply_via_replay_checkpoints_at_marker_own_lsn);
RUN_TEST(test_marker_triggered_checkpoint_trim_preserves_standby_mid_catchup_segments);
RUN_TEST(test_marker_checkpoint_fault_hook_gates_checkpoint_call);
}