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

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>

src/controller/persist.c

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

src/controller/testsuite/test_persist_checkpoint.c

@@ -199,6 +199,56 @@ static void test_checkpoint_writes_no_marker_to_wal(void)
     printf("test_checkpoint_writes_no_marker_to_wal: PASS\n");
 }
 
+/* ------------------------------------------------------------------ */
+/* bug-1870 follow-up: recovering from a snapshot at X with no WAL entry */
+/* above it must resume at LSN X+1. load_snapshot seeded current_lsn = X */
+/* (the NEXT lsn to write) -- hidden while every checkpoint also wrote a */
+/* WAL_CHECKPOINT marker above X; without the marker a standby recovered */
+/* one short, took entry X+1 for a gap and refused it forever (the       */
+/* --config-upgrade soak's force-elect stall).                           */
+/* ------------------------------------------------------------------ */
+
+static void test_recover_from_snapshot_without_wal_resumes_after_it(void)
+{
+    char tmpdir[256];
+    persist_t* p = make_ckpt_persist(tmpdir, sizeof(tmpdir));
+    TEST_ASSERT_NOT_NULL(p);
+
+    for (int i = 0; i < 3; i++)
+        persist_wal_async(p, WAL_JOB_UPDATE, "z", 1);        /* lsn 1..3 */
+    TEST_ASSERT_EQUAL_UINT64(4, p->current_lsn);
+    atomic_store_explicit(&p->last_applied_lsn, 3, memory_order_relaxed);
+    TEST_ASSERT_EQUAL_INT(0, persist_checkpoint(p));
+    TEST_ASSERT_EQUAL_UINT64(3, p->checkpoint_lsn);
+    TEST_ASSERT_EQUAL_INT(0, persist_flush(p));
+
+    char walpath[4096];
+    snprintf(walpath, sizeof(walpath), "%s", p->wal_path);
+    cleanup_ckpt_persist(p);
+
+    /* No WAL entry above (or at) the checkpoint survives. */
+    char cmd[4200];
+    snprintf(cmd, sizeof(cmd), "rm -f %s/seg-*.wal", walpath);
+    TEST_ASSERT_EQUAL_INT(0, system(cmd));
+
+    persist_t* p2 = calloc(1, sizeof(persist_t));
+    TEST_ASSERT_NOT_NULL(p2);
+    persist_config_t cfg = persist_config_defaults();
+    cfg.wal_sync_ms = 5000;
+    cfg.snap_keep_n = 2;
+    persist_set_workdir(p2, tmpdir, "ctrl-test");
+    TEST_ASSERT_EQUAL_INT(0, persist_init(p2, &cfg, NULL));
+    TEST_ASSERT_EQUAL_INT(0, persist_start(p2));
+    TEST_ASSERT_EQUAL_INT(0, persist_recover(p2));
+
+    TEST_ASSERT_EQUAL_UINT64(3, p2->checkpoint_lsn);
+    TEST_ASSERT_EQUAL_UINT64_MESSAGE(4, p2->current_lsn,
+        "after a snapshot covering 1..3, the next lsn to write is 4");
+    TEST_ASSERT_EQUAL_UINT64(3, persist_durable_floor(p2));
+
+    cleanup_ckpt_persist(p2);
+}
+
 /* ------------------------------------------------------------------ */
 /* Test 4: concurrent checkpoint returns -EBUSY                        */
 /* ------------------------------------------------------------------ */
@@ -257,6 +307,7 @@ void test_persist_checkpoint(void)
     RUN_TEST(test_checkpoint_sets_checkpoint_lsn);
     RUN_TEST(test_checkpoint_lsn_tracks_last_applied_not_current_lsn);
     RUN_TEST(test_checkpoint_writes_no_marker_to_wal);
+    RUN_TEST(test_recover_from_snapshot_without_wal_resumes_after_it);
     RUN_TEST(test_checkpoint_busy_guard);
     RUN_TEST(test_checkpoint_rotation_keeps_snap_keep_n);
 }

src/controller/testsuite/test_persist_snapshot_entities.c

@@ -1960,7 +1960,8 @@ static void test_snapent_load_snapshot_restores_entities_before_advancing_positi
      * TOGETHER with the entities actually being present -- both checked
      * above must be simultaneously true of the end state. */
     TEST_ASSERT_EQUAL_UINT64(expected_lsn, dst->c->persist.checkpoint_lsn);
-    TEST_ASSERT_EQUAL_UINT64(expected_lsn, dst->c->persist.current_lsn);
+    /* current_lsn is the NEXT lsn to write (bug-1870 follow-up). */
+    TEST_ASSERT_EQUAL_UINT64(expected_lsn + 1, dst->c->persist.current_lsn);
 
     snapent_teardown(dst);
     snapent_teardown(src);
@@ -2265,10 +2266,11 @@ static void test_snapent_load_snapshot_legacy_file_reports_position_only(void)
         dst->c->persist.checkpoint_lsn,
         "a legacy checkpoint still records the committed position it was "
         "taken at -- existing, preserved behavior");
-    TEST_ASSERT_EQUAL_UINT64_MESSAGE(SNAPENT_LEGACY_CHECKPOINT_LSN,
+    TEST_ASSERT_EQUAL_UINT64_MESSAGE(SNAPENT_LEGACY_CHECKPOINT_LSN + 1,
         dst->c->persist.current_lsn,
-        "a legacy checkpoint still advances current_lsn to its recorded "
+        "a legacy checkpoint still advances current_lsn past its recorded "
-        "position -- existing, preserved behavior");
+        "position (current_lsn is the next lsn to write, bug-1870 "
+        "follow-up)");
 
     snapent_teardown(dst);
     printf("test_snapent_load_snapshot_legacy_file_reports_position_only: PASS\n");