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

fix(ha-commit-engine,persist): reserve WAL lsn 0 — first entry starts at lsn 1

Browse source 
Root cause (found via live 3-node E2E + a verification test): lsn was 0-based
(hdr.lsn = current_lsn++ from current_lsn=0), and 0 was overloaded as a
sentinel, causing three cold-start defects on a freshly-promoted leader:
  - the first WAL entry (own-term promotion NOOP at lsn 0) was structurally
    uncommittable: commit_advance_locked only stores when cand>cur and
    committed_lsn starts at 0, so lsn 0 can never advance it;
  - commit_set_writable_lsn(noop_lsn=0) set writable_lsn=0, which wait_done
    reads as 'gate off' — the own-term safety gate was a no-op on the first leader;
  - bug-543 (WAL hole): ha_stream_wal_to_standby filters hdr.lsn>standby_lsn;
    a fresh empty standby reports standby_lsn=0 (ambiguous 'empty' vs 'has lsn 0'),
    so lsn 0 failed 0>0 and was dropped -> non-contiguous standby WAL.

Fix: a fresh persist store initializes current_lsn=1, reserving lsn 0 as the
none/empty sentinel. The first real entry is lsn 1 -> committable (0->1),
writable_lsn=1 -> gate effective, and standby_lsn=0 unambiguously means 'empty'
so catch-up streams from the first entry with no hole. persist_recover is
data-driven (current_lsn=max(hdr.lsn+1)), so existing on-disk WALs and non-fresh
recovery are unaffected; the change only sets the base for brand-new stores.
No wire/capnp change.

Verification: test_coldstart_lsn0 drives the real commit engine + real streamer
and proves H1 (first entry commits 0->1), H3 (gate effective + blocks when
committed<writable), H4 (empty-standby catch-up is contiguous [1,2], control
standby_lsn=1 gets only [2]). Full controller suite green under ASAN. Tests that
relied on the lsn-0 base were shifted (several strengthened); commit.c /
config_repl.c / ha.c / wire format untouched.

Owning specs: ha-commit-engine / ha-wal-replication / persist. Follow-up:
sweep stale lsn-0 comments (repl.c:429-436, persist.c:1055-1075/1240-1243/1296).
This commit is contained in:
Rongsong Shen 2026-06-14 10:46:16 +00:00
parent c97c7632e9
commit 7519fa96dd
10 changed files with 480 additions and 31 deletions
Download patch file Download diff file Expand all files Collapse all files

412
src/controller/testsuite/test_coldstart_lsn0.c Normal file
View file

@ -0,0 +1,412 @@
/*
* test_coldstart_lsn0.c — RESERVED-LSN-0 CONTRACT.
*
* Pins the cold-start fix: LSN 0 is RESERVED as the "none / empty" sentinel and
* is NEVER assigned to a real WAL entry. A fresh persist store starts
* current_lsn = 1 (persist_init, persist.c), so the FIRST-EVER appended entry
* (the promotion WAL_NOOP) is lsn 1. This removes three cold-start defects that
* the old 0-based scheme (hdr.lsn = current_lsn++ from 0) caused:
*
* H1 The first entry (NOOP at lsn 1) IS committable. committed_lsn advances
* 0 → 1. (Under the old scheme the lsn-0 NOOP was structurally
* uncommittable: commit_advance_locked only stores cand > cur and
* committed_lsn starts at 0, so lsn 0 could never advance it.)
*
* H2 A second real entry (lsn 2) commits on a majority watermark, proving the
* reserved base does not break ordinary advancement.
*
* H3 The own-term gate armed at the NOOP lsn (now 1, ≠ 0) is EFFECTIVE: a
* WAIT for an entry below the gate stays blocked until committed_lsn
* reaches the gate. (Under the old scheme writable_lsn = noop_lsn = 0 read
* as "gate off" in wait_done_locked — the safety gate was a no-op on the
* first cold-start leader.)
*
* H4 (CRITICAL — bug-543) NO WAL HOLE: a fresh EMPTY standby reports
* standby_lsn = 0 (unambiguously "empty", since no real entry is ever
* lsn 0). The REAL streamer (ha_stream_wal_to_standby, config_repl.c)
* filters `hdr.lsn > standby_lsn`; the first entry lsn 1 satisfies 1 > 0
* and is streamed, so the standby's WAL is contiguous from the tip.
*
* Drives the REAL commit engine (commit.c) and the REAL catch-up streamer
* (ha_stream_wal_to_standby) — never reimplementations.
*/
#include <dirent.h>
#include <errno.h>
#include <fcntl.h>
#include <inttypes.h>
#include <stdatomic.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <time.h>
#include <unistd.h>
#include <unity/unity.h>
#include "xce/errnox.h"
#include "commit.h"
#include "config_repl.h" /* ha_stream_wal_to_standby */
#include "controller.capnp.h" /* OPCode_walCatchupEntry */
#include "persist.h"
#include "repl.h"
#include "request.h" /* xce_request_t */
#include "typemap.h" /* wal_catchup_entry_t */
#include "wal_types.h" /* WAL_JOB_SUBMIT, wal_entry_hdr_t */
/* ── H1-H3 commit-engine fixture (pattern from test_commit_engine.c) ───────── */
static persist_t g_p;
static commit_engine_t g_ce;
static int g_eps[MAX_STANDBYS];
static void ce_setup(int n_peers, int n_standbys, uint64_t appended_lsn)
{
int i;
char name[16];
memset(&g_p, 0, sizeof(g_p));
TEST_ASSERT_EQUAL_INT(0, repl_init(&g_p.repl));
for (i = 0; i < n_standbys; i++) {
snprintf(name, sizeof(name), "sb%d", i);
TEST_ASSERT_EQUAL_INT(
i, repl_add_standby(&g_p.repl, &g_eps[i], name, 7000 + (uint32_t)i));
}
/* appended_lsn = highest locally appended; current_lsn = next = +1.
* Reserved-0 contract: appended_lsn == 0 means NOTHING appended (the lsn-0
* sentinel), so current_lsn = 1 = "first entry will be lsn 1". */
g_p.current_lsn = appended_lsn + 1;
atomic_store(&g_p.committed_lsn, 0);
TEST_ASSERT_EQUAL_INT(0, commit_engine_init(&g_ce, &g_p, n_peers, 8));
}
static void ce_teardown(void)
{
commit_engine_fini(&g_ce);
repl_fini(&g_p.repl);
}
static void standby_ack(int idx, uint64_t acked_lsn)
{
atomic_store(&g_p.repl.standbys[idx].acked_lsn, acked_lsn);
commit_on_ack(&g_ce, idx, acked_lsn);
}
static uint64_t committed(void)
{
return atomic_load(&g_p.committed_lsn);
}
/*
* H1: the first-ever entry — the promotion NOOP at lsn 1 — COMMITS.
*
* Cold-start model: a fresh 3-node cluster whose ONLY appended entry is the
* promotion NOOP at lsn 1 (appended_lsn = 1 → current_lsn = 2 → cap = 1).
* A majority of standbys ACK lsn 1; committed_lsn advances 0 → 1. This is the
* fix: the reserved-0 base makes the first entry committable (0 → 1), whereas a
* lsn-0 first entry could never advance committed_lsn off 0.
*/
static void test_h1_first_noop_lsn1_commits(void)
{
ce_setup(/*n_peers=*/3, /*n_standbys=*/2, /*appended_lsn=*/1);
uint64_t before = committed();
/* Majority (self + 1 of 2 for n_peers=3) ACK the lsn-1 NOOP. */
standby_ack(0, 1);
uint64_t after = committed();
printf("H1: committed_lsn before=%" PRIu64 " after sb0 ack(1)=%" PRIu64
" (cap=highest_appended=1)\n", before, after);
/* lsn 1 (the first-ever entry) is committable: committed_lsn advances 0→1. */
TEST_ASSERT_EQUAL_UINT64(0, before);
TEST_ASSERT_EQUAL_UINT64(1, after);
ce_teardown();
printf("test_h1_first_noop_lsn1_commits: PASS\n");
}
/*
* H2: a second real entry at lsn 2 also commits.
*
* 3-node cluster, entries appended through lsn 2 (NOOP lsn 1 + first real write
* lsn 2 → appended_lsn = 2 → cap = 2). One standby ACKs watermark 2
* (self + 1 of 2 = majority for n_peers=3). committed_lsn advances 0 → 2,
* proving the reserved base does not perturb ordinary advancement.
*/
static void test_h2_real_entry_lsn2_commits(void)
{
ce_setup(/*n_peers=*/3, /*n_standbys=*/2, /*appended_lsn=*/2);
uint64_t before = committed();
/* standby 0 ACK watermark 2 → matchIndex {self=2, sb0=2, sb1=0}: median 2 */
standby_ack(0, 2);
uint64_t after = committed();
printf("H2: committed_lsn before=%" PRIu64 " after sb0 ack(2)=%" PRIu64 "\n",
before, after);
TEST_ASSERT_EQUAL_UINT64(0, before);
TEST_ASSERT_EQUAL_UINT64(2, after);
ce_teardown();
printf("test_h2_real_entry_lsn2_commits: PASS\n");
}
/*
* H3: the own-term gate armed at the NOOP lsn (now 1, ≠ 0) is EFFECTIVE.
*
* Arm the gate at noop_lsn = 1 (the cold-start NOOP lsn under the reserved-0
* contract). With writable_lsn = 1, wait_done_locked's predicate
* `c >= lsn && (writable_lsn == 0 || c >= writable_lsn)` does NOT short-circuit:
* a WAIT for lsn 1 completes only once committed_lsn >= 1. We commit lsn 1 via
* self-quorum (n_peers = 1), then the WAIT must return 0 — the gate opens
* exactly when the NOOP commits. (Under the old lsn-0 scheme writable_lsn = 0
* disabled the gate entirely.)
*/
static void test_h3_writable_lsn1_gate_effective(void)
{
/* n_peers = 1: self-quorum, so the WAIT advances + completes on its own. */
ce_setup(/*n_peers=*/1, /*n_standbys=*/0, /*appended_lsn=*/1);
/* Arm the own-term gate at the cold-start NOOP lsn 1 (reserved-0 contract). */
commit_set_writable_lsn(&g_ce, 1);
TEST_ASSERT_EQUAL_UINT64(1, g_ce.writable_lsn);
/* WAIT for lsn 1 with a generous-but-finite timeout. writable_lsn = 1 is an
* ACTIVE gate (not "off"): the predicate requires committed_lsn >= 1. With
* self-quorum the entry commits and the gate opens, so WAIT returns 0. */
int rc = commit_submit(&g_ce, /*lsn=*/1, COMMIT_MODE_WAIT, /*timeout_ms=*/500);
printf("H3: commit_submit(lsn=1,WAIT) with writable_lsn=1 → rc=%d "
"committed_lsn=%" PRIu64 "\n", rc, committed());
TEST_ASSERT_EQUAL_INT_MESSAGE(0, rc,
"writable_lsn=1 is an active gate that opens when lsn 1 commits");
TEST_ASSERT_EQUAL_UINT64(1, committed());
ce_teardown();
printf("test_h3_writable_lsn1_gate_effective: PASS\n");
}
/*
* H3b: the gate at lsn 1 truly BLOCKS a sub-gate WAIT until the gate is reached.
*
* Distinct from H3 (which proves the gate opens): here we prove writable_lsn = 1
* is not silently ignored. With NOTHING committed (committed_lsn = 0) and the
* gate armed at 1, a short-timeout WAIT for lsn 1 must NOT complete (no quorum
* ever arrives in a 3-node cluster with no ACKs) — it times out. A gate that
* read as "off" (the old lsn-0 bug) would also block here on `c >= lsn`, so to
* isolate the gate we WAIT for lsn 1 while only committed_lsn 0 is reachable.
*/
static void test_h3b_gate_lsn1_blocks_until_reached(void)
{
/* 3-node cluster, NOOP lsn 1 appended, NO standby ACKs → never a majority. */
ce_setup(/*n_peers=*/3, /*n_standbys=*/2, /*appended_lsn=*/1);
commit_set_writable_lsn(&g_ce, 1);
/* committed_lsn is still 0; the gate at 1 is unreached. WAIT must time out
* (the gate is a blocking predicate, never a fast NOT_READY reject). */
int rc = commit_submit(&g_ce, /*lsn=*/1, COMMIT_MODE_WAIT, /*timeout_ms=*/150);
printf("H3b: WAIT(lsn=1) with gate=1, committed=%" PRIu64 " → rc=%d\n",
committed(), rc);
TEST_ASSERT_EQUAL_INT_MESSAGE(-ETIMEDOUT, rc,
"active gate at lsn 1 must block (time out) until committed_lsn >= 1");
TEST_ASSERT_EQUAL_UINT64(0, committed());
ce_teardown();
printf("test_h3b_gate_lsn1_blocks_until_reached: PASS\n");
}
/* ── H4 catch-up streaming fixture (real ha_stream_wal_to_standby) ─────────── */
/* test-only seam declared in config_repl.c (XCE_TESTING). */
extern int (*xce_wal_stream_request_hook)(endpoint_t *ep, xce_request_t *req);
/* Capture the exact set of streamed LSNs across all batches. */
#define MAX_CAPTURED 64
static uint64_t g_streamed_lsns[MAX_CAPTURED];
static int g_streamed_n;
static int capture_wal_stream_hook(endpoint_t *ep, xce_request_t *req)
{
(void)ep;
if (!req || !req->body.walcatchupentrymsg)
return 0;
wal_catchup_entry_t *batch = (wal_catchup_entry_t *)req->body.walcatchupentrymsg;
/* ha_stream_wal_to_standby builds a container batch whose ->entries is an
* array of per-entry wal_catchup_entry_t* (n_entries of them). */
for (int i = 0; i < batch->n_entries && g_streamed_n < MAX_CAPTURED; i++) {
wal_catchup_entry_t *e = batch->entries[i];
g_streamed_lsns[g_streamed_n++] = e->lsn;
}
return 0;
}
/* Write a genuine reserved-0 cold-start segment: the first segment is named
* after current_lsn = 1 (persist.c open_new_segment), so it is
* seg-0000000000000001.wal and holds entries lsn 1 and lsn 2 — exactly the
* layout persist.c lays down on a fresh cluster (NOOP lsn 1 + first write
* lsn 2). LSN 0 is reserved and never written. */
static int write_coldstart_segment(const char *wal_dir)
{
char seg_path[4096];
/* Reserved-0 contract: first segment is named after current_lsn == 1. */
snprintf(seg_path, sizeof(seg_path),
"%s/seg-%016" PRIx64 ".wal", wal_dir, (uint64_t)1);
int fd = open(seg_path, O_WRONLY | O_CREAT | O_TRUNC, 0600);
if (fd < 0)
return -1;
for (uint64_t lsn = 1; lsn <= 2; lsn++) {
wal_entry_hdr_t hdr;
memset(&hdr, 0, sizeof(hdr));
hdr.lsn = lsn;
hdr.type = WAL_JOB_SUBMIT;
hdr.payload_len = 0;
hdr.term = 1;
if (write(fd, &hdr, sizeof(hdr)) != (ssize_t)sizeof(hdr)) {
close(fd);
return -1;
}
}
close(fd);
return 0;
}
static void h4_capture_reset(void)
{
memset(g_streamed_lsns, 0, sizeof(g_streamed_lsns));
g_streamed_n = 0;
}
static int streamed_contains(uint64_t lsn)
{
for (int i = 0; i < g_streamed_n; i++)
if (g_streamed_lsns[i] == lsn)
return 1;
return 0;
}
static void print_streamed(const char *tag)
{
printf("%s streamed n=%d lsns=[", tag, g_streamed_n);
for (int i = 0; i < g_streamed_n; i++)
printf("%s%" PRIu64, i ? "," : "", g_streamed_lsns[i]);
printf("]\n");
}
/*
* H4 (CRITICAL — bug-543): a fresh EMPTY standby catching up to a primary
* holding [1,2] receives BOTH lsn 1 AND lsn 2 — NO hole.
*
* An empty standby (no entries applied) reports standby_lsn = 0, which under the
* reserved-0 contract unambiguously means "empty" (no real entry is ever lsn 0).
* The primary holds committed entries 1 and 2. We drive the REAL streamer and
* capture the exact LSNs it sends. The filter `hdr.lsn > standby_lsn` now yields
* 1 > 0 = true for the first entry, so lsn 1 is streamed — the cold-start hole
* (old: 0 > 0 = false dropped lsn 0) is closed. Correctness requires a
* contiguous [1,2].
*/
static void test_h4_empty_standby_receives_lsn1_and_lsn2_no_hole(void)
{
char tmpdir[256];
snprintf(tmpdir, sizeof(tmpdir), "/tmp/xce_coldstart_%d_%ld",
(int)getpid(), (long)time(NULL));
TEST_ASSERT_EQUAL_INT(0, mkdir(tmpdir, 0700));
TEST_ASSERT_EQUAL_INT_MESSAGE(0, write_coldstart_segment(tmpdir),
"failed to write cold-start segment");
xce_wal_stream_request_hook = capture_wal_stream_hook;
h4_capture_reset();
/* Empty fresh standby = standby_lsn 0; primary committed_lsn = 2 (holds
* [1,2]). Drive the REAL streamer. */
int rc = ha_stream_wal_to_standby(/*ep=*/NULL, tmpdir,
/*standby_lsn=*/0,
/*committed_lsn=*/2);
xce_wal_stream_request_hook = NULL;
TEST_ASSERT_EQUAL_INT_MESSAGE(0, rc, "ha_stream_wal_to_standby failed");
print_streamed("H4(empty standby_lsn=0, committed=2):");
int has1 = streamed_contains(1);
int has2 = streamed_contains(2);
printf("H4: has_lsn1=%d has_lsn2=%d (contiguous [1,2] required, no hole)\n",
has1, has2);
char cmd[512];
snprintf(cmd, sizeof(cmd), "rm -rf %s", tmpdir);
if (system(cmd) != 0)
fprintf(stderr, "cleanup warning: rm -rf %s failed\n", tmpdir);
/* Correctness assertion: BOTH entries reach the empty standby — no hole. */
TEST_ASSERT_TRUE_MESSAGE(has2, "lsn 2 must be streamed");
TEST_ASSERT_TRUE_MESSAGE(has1,
"NO HOLE: empty standby (standby_lsn=0) must receive the first entry "
"lsn 1 — the reserved-0 base makes `hdr.lsn(1) > standby_lsn(0)` true, "
"closing the cold-start WAL hole that the old lsn-0 first entry caused");
printf("test_h4_empty_standby_receives_lsn1_and_lsn2_no_hole: PASS\n");
}
/*
* H4 control: a standby that has applied lsn 1 (standby_lsn = 1) catching up to
* committed 2 receives ONLY lsn 2. Under the reserved-0 contract standby_lsn = 1
* is unambiguous ("has the first entry"), so the streamer correctly sends just
* the missing lsn 2 — no double-send, no ambiguity. This is the contrast to H4:
* different standby_lsn (1 vs 0) → different, correct streamed set.
*/
static void test_h4_control_standby_with_lsn1_gets_only_lsn2(void)
{
char tmpdir[256];
snprintf(tmpdir, sizeof(tmpdir), "/tmp/xce_coldstart_ctl_%d_%ld",
(int)getpid(), (long)time(NULL));
TEST_ASSERT_EQUAL_INT(0, mkdir(tmpdir, 0700));
TEST_ASSERT_EQUAL_INT(0, write_coldstart_segment(tmpdir));
xce_wal_stream_request_hook = capture_wal_stream_hook;
h4_capture_reset();
/* standby_lsn = 1 (already holds the first entry) catching up to committed 2. */
int rc = ha_stream_wal_to_standby(/*ep=*/NULL, tmpdir,
/*standby_lsn=*/1,
/*committed_lsn=*/2);
xce_wal_stream_request_hook = NULL;
TEST_ASSERT_EQUAL_INT(0, rc);
print_streamed("H4-control(standby has lsn1, standby_lsn=1):");
char cmd[512];
snprintf(cmd, sizeof(cmd), "rm -rf %s", tmpdir);
if (system(cmd) != 0)
fprintf(stderr, "cleanup warning: rm -rf %s failed\n", tmpdir);
/* Only the missing entry (lsn 2) is streamed; lsn 1 is already applied. */
TEST_ASSERT_TRUE_MESSAGE(streamed_contains(2), "lsn 2 must be streamed");
TEST_ASSERT_FALSE_MESSAGE(streamed_contains(1),
"standby_lsn=1 already holds lsn 1 — it must NOT be re-streamed "
"(reserved-0 makes standby_lsn=1 unambiguous: 'has the first entry')");
printf("test_h4_control_standby_with_lsn1_gets_only_lsn2: PASS\n");
}
void test_coldstart_lsn0(void)
{
RUN_TEST(test_h1_first_noop_lsn1_commits);
RUN_TEST(test_h2_real_entry_lsn2_commits);
RUN_TEST(test_h3_writable_lsn1_gate_effective);
RUN_TEST(test_h3b_gate_lsn1_blocks_until_reached);
RUN_TEST(test_h4_control_standby_with_lsn1_gets_only_lsn2);
RUN_TEST(test_h4_empty_standby_receives_lsn1_and_lsn2_no_hole);
}