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

fix(controller): checkpoints no longer punch a hole in the replicated log (bug-1870)

Browse source 
persist_checkpoint() and persist_checkpoint_marker_triggered() wrote a
local-only WAL_CHECKPOINT marker that consumed an LSN of the replicated log
(current_lsn++) but was never replicated. A checkpoint taken by one member
only -- the primary's periodic max-wal-records checkpoint, or the one before
a snapshot send -- left a hole no standby had: the leader's next entry
carried prev_log_index = the marker's LSN, logOk failed on every standby
forever, commits timed out and writes stalled ("could not submit 110 jobs"
in the --force-snapshot soak; reproduced on the pre-bug-1867 baseline). Even
the config-marker checkpoint, taken on every member, stamped the marker with
each member's own current_lsn at apply time, which need not agree.

Nothing reads the marker except to skip it, so checkpoints simply stop
writing it. The recovery, streamer and bug-1475 log_len skip paths stay, for
WALs written by older binaries; persist_test_write_legacy_checkpoint_marker()
(XCE_TESTING only) reproduces such a WAL for the tests that cover them.

- new test_bug1870_leader_only_checkpoint_leaves_no_log_hole (RED before):
  a leader-only checkpoint takes no LSN and a standby that never
  checkpointed catches up past it via the real streamer/receive path.
- test_checkpoint_writes_marker_to_wal inverted to ..._writes_no_marker_...
- bug-1475 x2 and the apply-correctness trailing-marker test now build
  their legacy marker explicitly.

Controller suite 1234/0. HA soaks: --force-snapshot PASS (113 jobs, real
snapshot install -- also live-verifies bug-1867), --config-upgrade-wal-trim
PASS, --sequential 6 rounds PASS (SIMVERIFY 7 files, 0 mismatches).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-09-26 15:16:46 +00:00
parent 725301dd92
commit 63fd3d2afd
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
6 changed files with 139 additions and 20 deletions
Download patch file Download diff file Expand all files Collapse all files

6
src/controller/ha.c
View file

@ -3295,7 +3295,11 @@ static int ha_apply_term_entry_via_library(ha_state_t *ha, persist_t *p,
cluster_t *c, struct endpoint_t *ep,
wal_catchup_entry_t *e)
{
/* bug-1475 / bug-1466: resync the raft library's log_len from this node's
/* bug-1870: checkpoints no longer write WAL_CHECKPOINT markers, so new
* logs have no such holes; this resync is kept for WALs written by older
* binaries, whose markers still sit in the log.
*
* bug-1475 / bug-1466: resync the raft library's log_len from this node's
* OWN recovered/durable WAL tip before consuming the AE, exactly mirroring
* ha_sync_raft_log_from_recovery's boot-time computation (persist_durable_
* floor), but applied live, JUST-IN-TIME, on this same single AE-receive

33
src/controller/persist.c
View file

@ -2275,9 +2275,24 @@ uint64_t persist_test_durable_lsn_on_disk(persist_t* p)
/* Checkpoint internals */
/* ------------------------------------------------------------------ */
/* Write a WAL_CHECKPOINT marker to the current segment.
* Never calls repl_broadcast() — checkpoint markers are local only. */
static int persist_write_checkpoint_marker(persist_t* p)
/* bug-1870: checkpoints no longer write a WAL_CHECKPOINT marker. The marker
* was local-only (never replicated) yet consumed an LSN of the replicated log
* (current_lsn++), so a checkpoint taken by one member only -- the periodic
* max-wal-records one, or the one before a snapshot send -- left a hole the
* other members never had: the leader's next entry carried prev_log_index =
* the marker's LSN, logOk failed on every standby and writes stalled. Even a
* config-marker-triggered checkpoint, taken on every member, stamped the
* marker with each member's own current_lsn at apply time, which need not
* agree. Nothing reads the marker back except to skip it (recovery,
* catch-up streaming), so it is simply not written any more; those skip
* paths stay for markers already present in existing WALs. */
#ifdef XCE_TESTING
/* bug-1870: the WAL_CHECKPOINT marker writer checkpoints used before the fix,
* kept for tests only, to produce the on-disk state of a WAL written by an
* older binary (a local marker that consumed an LSN) -- recovery and catch-up
* streaming must still handle such WALs. Never called in production. */
int persist_test_write_legacy_checkpoint_marker(persist_t* p)
{
pthread_mutex_lock(&p->lock);
@ -2320,6 +2335,7 @@ static int persist_write_checkpoint_marker(persist_t* p)
pthread_mutex_unlock(&p->lock);
return 0;
}
#endif /* XCE_TESTING */
/* Entity-bearing checkpoint payload (ha-entity-snapshots task 2.2/3.2, Req
* 1.1-1.4). Physical layout appended after the existing legacy header
@ -3581,10 +3597,8 @@ int persist_checkpoint(persist_t* p)
int rc = 0;
/* 1. Write WAL_CHECKPOINT marker (local only — never replicated) */
rc = persist_write_checkpoint_marker(p);
if (rc != 0)
goto done;
/* 1. (bug-1870) no WAL_CHECKPOINT marker any more -- see the bug-1870
* note above the checkpoint payload layout comment. */
/* 2. Record checkpoint_lsn = the APPLIED cursor (last_applied_lsn), not
* the physical WAL append tip (current_lsn). write_snapshot() below
@ -3666,9 +3680,8 @@ static int persist_checkpoint_marker_triggered(persist_t* p,
if (atomic_exchange(&p->checkpoint_in_progress, 1) != 0)
return -EBUSY;
int rc = persist_write_checkpoint_marker(p);
if (rc == 0)
rc = persist_checkpoint_write_locked(p, marker_lsn);
/* bug-1870: no WAL_CHECKPOINT marker (see persist_checkpoint). */
int rc = persist_checkpoint_write_locked(p, marker_lsn);
if (rc == 0) {
trim_wal_segments(p);

4
src/controller/testsuite/test_apply_correctness_verification.c
View file

@ -365,6 +365,10 @@ static void test_checkpoint_tail_with_no_snapshot_is_apply_clean(void)
* seam to stop persist_checkpoint() midway, so the snapshot is
* deleted immediately below to reach the identical post-crash
* on-disk state. */
/* bug-1870: checkpoints no longer write a marker; reproduce a WAL
* written by an older binary (marker, then snapshot). */
TEST_ASSERT_EQUAL_INT(0,
persist_test_write_legacy_checkpoint_marker(p));
TEST_ASSERT_EQUAL_INT(0, persist_checkpoint(p));
marker_checkpoint_lsn = p->checkpoint_lsn;

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

@ -5497,9 +5497,15 @@ static void test_bug1475_catchup_survives_midstream_checkpoint(void)
* synchronous flush right after so persist_durable_floor() (which the
* ha.c bug-1475 fix reads) reflects it immediately rather than waiting on
* the background fsync_worker's own timer (cfg.wal_sync_ms=5000). */
TEST_ASSERT_EQUAL_INT(0, persist_checkpoint(&leader->persist)); /* lsn=2 */
/* bug-1870: checkpoints no longer write a marker; this test now covers a
* WAL written by an older binary, whose checkpoint marker took lsn=2. */
TEST_ASSERT_EQUAL_INT(0,
persist_test_write_legacy_checkpoint_marker(&leader->persist)); /* lsn=2 */
TEST_ASSERT_EQUAL_INT(0, persist_checkpoint(&leader->persist));
TEST_ASSERT_EQUAL_INT(0, persist_flush(&leader->persist));
TEST_ASSERT_EQUAL_INT(0, persist_checkpoint(&recv->persist)); /* lsn=2 */
TEST_ASSERT_EQUAL_INT(0,
persist_test_write_legacy_checkpoint_marker(&recv->persist)); /* lsn=2 */
TEST_ASSERT_EQUAL_INT(0, persist_checkpoint(&recv->persist));
TEST_ASSERT_EQUAL_INT(0, persist_flush(&recv->persist));
/* bug-1466 (unfixed, deliberately NOT touched by this fix): the checkpoint
@ -5558,6 +5564,79 @@ static void test_bug1475_catchup_survives_midstream_checkpoint(void)
printf("test_bug1475_catchup_survives_midstream_checkpoint: PASS\n");
}
/* bug-1870: a checkpoint taken by ONE member only -- the primary's periodic
* max-wal-records checkpoint, or the one before a snapshot send -- must not
* leave a hole in the replicated log. persist_write_checkpoint_marker used to
* consume an LSN for its local-only WAL_CHECKPOINT marker, so the leader's
* next real entry carried prev_log_index = the marker's LSN, which no standby
* holds: logOk failed forever and writes stalled (--force-snapshot soak). */
static void test_bug1870_leader_only_checkpoint_leaves_no_log_hole(void)
{
char ltmp[256], rtmp[256];
strcpy(ltmp, "/tmp/xce_cr_b1870_leader_XXXXXX");
strcpy(rtmp, "/tmp/xce_cr_b1870_recv_XXXXXX");
TEST_ASSERT_NOT_NULL(mkdtemp(ltmp));
TEST_ASSERT_NOT_NULL(mkdtemp(rtmp));
cluster_t *leader = t42_make_cluster(ltmp);
cluster_t *recv = t42_make_cluster(rtmp);
TEST_ASSERT_NOT_NULL(leader);
TEST_ASSERT_NOT_NULL(recv);
enum { TERM = 3 };
t42_make_leader(leader, TERM, /*n_peers=*/3);
t_bug1475_make_follower(recv, TERM, /*n_peers=*/3);
/* identical prefix through lsn=1 on both */
persist_set_term(&leader->persist, TERM);
TEST_ASSERT_EQUAL_INT(0,
persist_wal_async(&leader->persist, WAL_JOB_UPDATE, NULL, 0)); /* lsn=1 */
leader->ha.raft.log_len = 1;
persist_set_term(&recv->persist, TERM);
TEST_ASSERT_EQUAL_INT(0,
persist_wal_async(&recv->persist, WAL_JOB_UPDATE, NULL, 0)); /* lsn=1 */
recv->ha.raft.log_len = 1;
atomic_store_explicit(&recv->ha.applied_lsn, 1, memory_order_release);
/* ONLY the leader checkpoints; it must not take a log slot */
uint64_t before = (uint64_t)leader->persist.current_lsn;
TEST_ASSERT_EQUAL_INT(0, persist_checkpoint(&leader->persist));
TEST_ASSERT_EQUAL_INT(0, persist_flush(&leader->persist));
TEST_ASSERT_EQUAL_UINT64_MESSAGE(before,
(uint64_t)leader->persist.current_lsn,
"bug-1870: a checkpoint must not consume a replicated-log LSN");
/* leader appends two real entries after its checkpoint */
persist_set_term(&leader->persist, TERM);
TEST_ASSERT_EQUAL_INT(0,
persist_wal_async(&leader->persist, WAL_JOB_UPDATE, NULL, 0));
TEST_ASSERT_EQUAL_INT(0,
persist_wal_async(&leader->persist, WAL_JOB_UPDATE, NULL, 0));
uint64_t tip = (uint64_t)leader->persist.current_lsn - 1;
atomic_store_explicit(&leader->persist.committed_lsn, tip,
memory_order_release);
/* real catch-up streamer -> real receive path */
g_bug1475_recv = recv;
xce_wal_stream_request_hook = bug1475_replay_hook;
int rc = ha_stream_wal_to_standby(/*ep=*/NULL, leader->persist.wal_path,
&leader->persist, /*leader_term=*/TERM,
/*standby_lsn=*/1, /*committed_lsn=*/tip);
xce_wal_stream_request_hook = NULL;
g_bug1475_recv = NULL;
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_UINT64_MESSAGE(tip,
atomic_load_explicit(&recv->ha.applied_lsn, memory_order_acquire),
"bug-1870: a standby that never checkpointed must still catch up "
"past the leader's checkpoint");
TEST_ASSERT_EQUAL_UINT64(tip, recv->ha.raft.log_len);
TEST_ASSERT_EQUAL_UINT64(TERM, persist_entry_term(&recv->persist, tip));
t42_teardown(leader);
t42_teardown(recv);
}
/*
* bug-1028 — the catch-up streamer must source each streamed entry's term and
* the AppendEntries prev_log_term from the AUTHORITATIVE per-LSN term index
@ -5809,7 +5888,10 @@ static void test_bug1475_finding2_snapshot_install_checkpoint_term_absent(void)
TEST_ASSERT_EQUAL_UINT64(2, (uint64_t)leader->persist.current_lsn);
atomic_store_explicit(&leader->persist.last_applied_lsn, 2,
memory_order_release);
TEST_ASSERT_EQUAL_INT(0, persist_checkpoint(&leader->persist)); /* lsn=2 */
/* bug-1870: legacy (pre-fix) marker at lsn=2, as an older binary wrote. */
TEST_ASSERT_EQUAL_INT(0,
persist_test_write_legacy_checkpoint_marker(&leader->persist)); /* lsn=2 */
TEST_ASSERT_EQUAL_INT(0, persist_checkpoint(&leader->persist));
TEST_ASSERT_EQUAL_INT(0, persist_flush(&leader->persist));
TEST_ASSERT_EQUAL_UINT64_MESSAGE(2, leader->persist.checkpoint_lsn,
"precondition: checkpoint_lsn must coincide with the checkpoint "
@ -7447,6 +7529,7 @@ void test_config_repl(void)
RUN_TEST(test_wal_stream_empty_range_no_batches);
RUN_TEST(test_wal_stream_skips_checkpoint_marker);
RUN_TEST(test_bug1475_catchup_survives_midstream_checkpoint); /* bug-1475 */
RUN_TEST(test_bug1870_leader_only_checkpoint_leaves_no_log_hole); /* bug-1870 */
RUN_TEST(test_wal_stream_term_from_index_not_raw_hdr); /* bug-1028 */
/* Task 3.5: snapshot chunk transfer (sender side) */
RUN_TEST(test_snapshot_transfer_two_chunks);

21
src/controller/testsuite/test_persist_checkpoint.c
View file

@ -171,23 +171,32 @@ static void test_checkpoint_lsn_tracks_last_applied_not_current_lsn(void)
}
/* ------------------------------------------------------------------ */
/* Test 3: checkpoint writes a WAL_CHECKPOINT marker (advances lsn) */
/* Test 3 (bug-1870): a checkpoint writes no WAL_CHECKPOINT marker and */
/* consumes no LSN -- a local-only marker in the replicated log left a */
/* hole standbys never had and stalled replication. */
/* ------------------------------------------------------------------ */
static void test_checkpoint_writes_marker_to_wal(void)
static void test_checkpoint_writes_no_marker_to_wal(void)
{
char tmpdir[256];
persist_t* p = make_ckpt_persist(tmpdir, sizeof(tmpdir));
TEST_ASSERT_NOT_NULL(p);
persist_wal_async(p, WAL_JOB_UPDATE, "z", 1);
uint64_t lsn_before = p->current_lsn;
atomic_store_explicit(&p->last_applied_lsn, lsn_before - 1,
memory_order_relaxed);
TEST_ASSERT_EQUAL_INT(0, persist_checkpoint(p));
/* current_lsn must have advanced — WAL_CHECKPOINT marker was written */
TEST_ASSERT_TRUE(p->current_lsn > lsn_before);
TEST_ASSERT_EQUAL_UINT64_MESSAGE(lsn_before, p->current_lsn,
"a checkpoint must not consume a replicated-log LSN");
/* the next real entry takes the very next LSN -- no hole */
persist_wal_async(p, WAL_JOB_UPDATE, "z", 1);
TEST_ASSERT_EQUAL_UINT64(lsn_before + 1, p->current_lsn);
cleanup_ckpt_persist(p);
printf("test_checkpoint_writes_marker_to_wal: PASS\n");
printf("test_checkpoint_writes_no_marker_to_wal: PASS\n");
}
/* ------------------------------------------------------------------ */
@ -247,7 +256,7 @@ void test_persist_checkpoint(void)
RUN_TEST(test_checkpoint_creates_snapshot_file);
RUN_TEST(test_checkpoint_sets_checkpoint_lsn);
RUN_TEST(test_checkpoint_lsn_tracks_last_applied_not_current_lsn);
RUN_TEST(test_checkpoint_writes_marker_to_wal);
RUN_TEST(test_checkpoint_writes_no_marker_to_wal);
RUN_TEST(test_checkpoint_busy_guard);
RUN_TEST(test_checkpoint_rotation_keeps_snap_keep_n);
}

6
src/include/persist.h
View file

@ -1092,6 +1092,12 @@ extern void (*persist_checkpoint_test_hook)(persist_t*);
/* Checkpoint */
int persist_checkpoint(persist_t* p);
#ifdef XCE_TESTING
/* bug-1870: test-only -- write a pre-fix WAL_CHECKPOINT marker (consumes an
* LSN) to reproduce a WAL written by an older binary. */
int persist_test_write_legacy_checkpoint_marker(persist_t* p);
#endif
/*
* persist_set_checkpoint_offload_for_test / persist_checkpoint_offload_enabled:
* Feature-flag accessors (xce-audit-invariants controller pass, dimension 1