diff --git a/src/controller/persist.c b/src/controller/persist.c index 3e596967..539765b5 100644 --- a/src/controller/persist.c +++ b/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; diff --git a/src/controller/testsuite/test_persist_checkpoint.c b/src/controller/testsuite/test_persist_checkpoint.c index 2d5ee89c..746c852a 100644 --- a/src/controller/testsuite/test_persist_checkpoint.c +++ b/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); } diff --git a/src/controller/testsuite/test_persist_snapshot_entities.c b/src/controller/testsuite/test_persist_snapshot_entities.c index e32b35a0..19d543ee 100644 --- a/src/controller/testsuite/test_persist_snapshot_entities.c +++ b/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 " - "position -- existing, preserved behavior"); + "a legacy checkpoint still advances current_lsn past its recorded " + "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");