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

fix(controller): resume at checkpoint+1 after loading a snapshot (bug-1870 follow-up)

Browse source 
Regression from 63fd3d2a (bug-1870), found by the --config-upgrade HA soak
(force-elect: "submit on {ctrl-2,ctrl-1} failed", ~640 standby NACKs;
bisected: 725301dd passes, 88c4310e fails).

load_snapshot() seeded current_lsn = checkpoint_lsn, but current_lsn is
the NEXT lsn to write and the snapshot covers 1..checkpoint_lsn. The
off-by-one was always there, hidden because every checkpoint also wrote a
WAL_CHECKPOINT marker ABOVE the checkpoint, so the WAL scan moved
current_lsn past it. With the marker gone, a standby whose WAL held
nothing above its config-marker checkpoint (LSN 11) recovered with
current_lsn=11: its raft layer counted entry 11 as held (ae_already_have,
not written), and persist_wal_write_replicated() then refused entry 12 as
a gap (ae_append_persist_fail) on every resend -- stuck at 11 forever.

Both of load_snapshot's commit points now set current_lsn =
checkpoint_lsn + 1, matching the apply-side catch-up path (lsn + 1).

New test_recover_from_snapshot_without_wal_resumes_after_it (RED before:
current_lsn 3, not 4). Two snapshot-entity tests asserted the old
current_lsn == checkpoint_lsn and now expect +1. Controller suite 1245/0.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-09-27 06:47:30 +00:00
parent 154a169ff3
commit cb1415d7c1
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
3 changed files with 69 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

14
src/controller/persist.c
View file

@ -3981,7 +3981,9 @@ static int load_snapshot(persist_t* p, const char* path, uint64_t lsn,
if (st.st_size <= consumed) {
close(fd);
p->checkpoint_lsn = stored_lsn;
p->current_lsn = stored_lsn;
/* current_lsn is the NEXT lsn to write: the snapshot covers
* 1..stored_lsn (bug-1870 follow-up -- see below). */
p->current_lsn = stored_lsn + 1;
if (cfgref_present) {
p->cfgref_present = 1;
p->cfgref_tag = cfgref_tag;
@ -4018,7 +4020,15 @@ static int load_snapshot(persist_t* p, const char* path, uint64_t lsn,
/* Single commit point (Requirement 2.1, 2.2): reached ONLY after every
* job and queue frame restored successfully above. */
p->checkpoint_lsn = stored_lsn;
p->current_lsn = stored_lsn;
/* bug-1870 follow-up: current_lsn is the NEXT lsn to write, and the
* snapshot covers 1..stored_lsn, so it is stored_lsn + 1 (the apply-side
* catch-up path already advances it to lsn + 1). Seeding stored_lsn was
* hidden while every checkpoint also wrote a WAL_CHECKPOINT marker above
* it (the WAL scan then moved current_lsn past); without the marker a
* member whose WAL held nothing above the checkpoint recovered one
* short, took the next replicated entry for a gap and refused it
* forever. */
p->current_lsn = stored_lsn + 1;
if (cfgref_present) {
p->cfgref_present = 1;
p->cfgref_tag = cfgref_tag;