fix(wal): throw on a checksum-setting mismatch instead of dropping the WAL

docs/checkpoint_recovery.md

@@ -74,7 +74,9 @@ its own again.
 
 Recovery reads the checksum flag of an attached-graph WAL from the WAL's own header, so a graph
 opened on its own may be opened with either checksum setting: its committed changes are replayed
-or its committed checkpoint recovered when the parent database is reopened.
+or its committed checkpoint recovered when the parent database is reopened. The database's own
+WAL must instead match the configured setting. A mismatch is a configuration error, not a torn
+tail, so it fails the open even with `throwOnWalReplayFailure` false and leaves the WAL untouched.
 
 ## Downgrades
 

src/include/main/database.h

@@ -68,7 +68,8 @@ struct LBUG_API SystemConfig {
      * Default is false: on a torn tail (e.g. after a crash), the database opens with all data
      * up to the last checkpoint, discarding only the unreplayable tail.
      * @param enableChecksums If true, the database will use checksums to detect corruption in the
-     * WAL file.
+     * WAL file. It must match the setting the database's WAL was written with: a mismatch fails
+     * the open whatever throwOnWalReplayFailure says.
      * @param enableMultiWrites If true, multiple concurrent write transactions are allowed.
      * Default to false.
      * @param enableDefaultHashIndex If true, node tables create the default primary-key hash

src/storage/wal/wal_replayer.cpp

@@ -234,14 +234,14 @@ static Deserializer initDeserializer(FileInfo& fileInfo, main::ClientContext& cl
     }
 }
 
-static void checkWALHeader(const WALHeader& header, bool enableChecksums) {
-    if (enableChecksums != header.enableChecksums) {
+static void checkWALHeader(const WALHeader& walHeader, bool enableChecksums) {
+    if (enableChecksums != walHeader.enableChecksums) {
         throw RuntimeException(std::format(
             "The database you are trying to open was serialized with enableChecksums={} but you "
             "are trying to open it with enableChecksums={}. Please open your database using the "
             "correct enableChecksums config. If you wish to change this for your database, please "
             "use the export/import functionality.",
-            TypeUtils::toString(header.enableChecksums), TypeUtils::toString(enableChecksums)));
+            TypeUtils::toString(walHeader.enableChecksums), TypeUtils::toString(enableChecksums)));
     }
 }
 
@@ -253,18 +253,20 @@ static uint64_t getReadOffset(Deserializer& deSer, bool enableChecksums) {
     }
 }
 
-// An attached-graph WAL may have been written by a different session (e.g. a standalone
-// graph session) whose checksum setting differs from the recovering session's, so the
-// checksum flag must come from the WAL's own header. A torn header holds no acknowledged
-// commits, so strict recovery rethrows and non-strict recovery may retire the WAL.
-static std::optional<bool> tryReadWALChecksumFlag(FileInfo& fileInfo,
+// Reads the header with the plain reader whatever the checksum setting: ChecksumWriter writes an
+// object's bytes before its checksum, so the WAL starts with the header in both modes. An
+// attached-graph WAL may have been written by a different session (e.g. a standalone graph
+// session) whose checksum setting differs from the recovering session's, so its checksum flag
+// must come from here. A torn header holds no acknowledged commits: with throwOnWalReplayFailure
+// the read error is rethrown, otherwise nullopt lets non-strict recovery retire the WAL.
+static std::optional<WALHeader> tryReadWALHeader(FileInfo& fileInfo,
     main::ClientContext& clientContext, bool throwOnWalReplayFailure) {
     try {
         auto deserializer = initDeserializer(fileInfo, clientContext, false);
         deserializer.getReader()->onObjectBegin();
         const auto walHeader = readWALHeader(deserializer);
         deserializer.getReader()->onObjectEnd();
-        return walHeader.enableChecksums;
+        return walHeader;
     } catch (...) {
         if (throwOnWalReplayFailure) {
             throw;
@@ -381,14 +383,14 @@ WALReplayer::GraphRecoveryState WALReplayer::prepareGraphCheckpoint(StorageManag
         if (!storageManager.isReadOnly()) {
             syncWALFile(*fileInfo);
         }
-        const auto walEnableChecksums = tryReadWALChecksumFlag(*fileInfo, clientContext,
+        const auto persistedHeader = tryReadWALHeader(*fileInfo, clientContext,
             clientContext.getDBConfig()->throwOnWalReplayFailure);
-        if (!walEnableChecksums.has_value()) {
+        if (!persistedHeader.has_value()) {
             // The header is torn, so the WAL holds no acknowledged commits.
             return std::nullopt;
         }
         auto replayInfo = dryReplay(*fileInfo, clientContext.getDBConfig()->throwOnWalReplayFailure,
-            *walEnableChecksums);
+            persistedHeader->enableChecksums);
         if (!replayInfo.isLastRecordCheckpoint && replayInfo.offsetDeserialized == 0) {
             return std::nullopt;
         }
@@ -520,19 +522,21 @@ void WALReplayer::replayGraphWAL(StorageManager& storageManager,
             }
             auto fileInfo = VirtualFileSystem::GetUnsafe(clientContext)
                                 ->openFile(range.path, FileOpenFlags(flags), &clientContext);
-            const auto walEnableChecksums = tryReadWALChecksumFlag(*fileInfo, clientContext, true);
-            if (!walEnableChecksums.has_value()) {
+            const auto persistedHeader = tryReadWALHeader(*fileInfo, clientContext, true);
+            if (!persistedHeader.has_value()) {
                 throw RuntimeException(std::format(
                     "Cannot replay graph WAL {}: the WAL header could not be read.", range.path));
             }
-            auto deserializer = initDeserializer(*fileInfo, clientContext, *walEnableChecksums);
+            auto deserializer =
+                initDeserializer(*fileInfo, clientContext, persistedHeader->enableChecksums);
             deserializer.getReader()->onObjectBegin();
             const auto walHeader = readWALHeader(deserializer);
-            checkWALHeader(walHeader, *walEnableChecksums);
+            checkWALHeader(walHeader, persistedHeader->enableChecksums);
             FileDBIDUtils::verifyDatabaseID(*fileInfo,
                 storageManager.getOrInitDatabaseID(clientContext), walHeader.databaseID);
             deserializer.getReader()->onObjectEnd();
-            while (getReadOffset(deserializer, *walEnableChecksums) < range.endOffset) {
+            while (
+                getReadOffset(deserializer, persistedHeader->enableChecksums) < range.endOffset) {
                 auto walRecord = WALRecord::deserialize(deserializer, clientContext);
                 replayWALRecord(*walRecord);
             }
@@ -928,13 +932,19 @@ void WALReplayer::replayCommittedCheckpoint(Checkpointer& checkpointer,
 
 WALReplayer::WALReplayInfo WALReplayer::dryReplay(FileInfo& fileInfo, bool throwOnWalReplayFailure,
     bool enableChecksums) const {
+    // A readable header with the other checksum setting is a configuration error, not a torn
+    // tail: the non-strict path below would drop every commit in the WAL, so it throws in both
+    // modes. A header too short to read is left to the path below.
+    const auto persistedHeader = tryReadWALHeader(fileInfo, clientContext, false);
+    if (persistedHeader.has_value()) {
+        checkWALHeader(*persistedHeader, enableChecksums);
+    }
     WALReplayInfo replayInfo;
     try {
         Deserializer deserializer = initDeserializer(fileInfo, clientContext, enableChecksums);
 
         deserializer.getReader()->onObjectBegin();
         const auto walHeader = readWALHeader(deserializer);
-        checkWALHeader(walHeader, enableChecksums);
         replayInfo.walDatabaseID = walHeader.databaseID;
         deserializer.getReader()->onObjectEnd();
 

test/transaction/wal_test.cpp

@@ -35,6 +35,7 @@ class WalTest : public ApiTest {
 
     void testStrayWALFile(const std::function<void()>& setupNewDBFunc);
     void setupChecksumMismatchTest(std::function<void(std::ofstream&)> corruptFunc);
+    void expectChecksumSettingError();
 };
 
 class FailingSyncFileSystem final : public FileSystem, public DirectorySyncFileSystem {
@@ -864,6 +865,26 @@ TEST_F(WalTest, WALChecksumConfigMismatch) {
         lbug::common::RuntimeException);
 }
 
+static std::vector<uint8_t> readFileBytes(const std::string& path) {
+    std::ifstream stream{path, std::ios::binary};
+    return {std::istreambuf_iterator<char>{stream}, std::istreambuf_iterator<char>{}};
+}
+
+void WalTest::expectChecksumSettingError() {
+    EXPECT_THROW(
+        {
+            try {
+                createDBAndConn();
+            } catch (std::exception& e) {
+                EXPECT_THAT(e.what(),
+                    testing::HasSubstr(
+                        "Please open your database using the correct enableChecksums config."));
+                throw;
+            }
+        },
+        lbug::common::RuntimeException);
+}
+
 TEST_F(WalTest, WALChecksumConfigMismatchNoThrow) {
     if (inMemMode || systemConfig->checkpointThreshold == 0) {
         GTEST_SKIP();
@@ -872,9 +893,38 @@ TEST_F(WalTest, WALChecksumConfigMismatchNoThrow) {
     ASSERT_TRUE(conn->query("call auto_checkpoint=false")->isSuccess());
     ASSERT_TRUE(conn->query("create node table test1(id int64 primary key)")->isSuccess());
     ASSERT_TRUE(conn->query("create node table test2(id int64 primary key)")->isSuccess());
+    database.reset();
+    const auto walFilePath = lbug::storage::StorageUtils::getWALFilePath(databasePath);
+    const auto walBytes = readFileBytes(walFilePath);
+    ASSERT_FALSE(walBytes.empty());
     systemConfig->enableChecksums = !systemConfig->enableChecksums;
     systemConfig->throwOnWalReplayFailure = false;
-    // We have throwOnWalReplayFailure=false so we essentially skip the replay
+    systemConfig->readOnly = true;
+    expectChecksumSettingError();
+    systemConfig->readOnly = false;
+    expectChecksumSettingError();
+    EXPECT_EQ(readFileBytes(walFilePath), walBytes);
+    systemConfig->enableChecksums = !systemConfig->enableChecksums;
+    createDBAndConn();
+    auto res = conn->query("CALL show_tables() WHERE name STARTS WITH 'test' RETURN *;");
+    ASSERT_TRUE(res->isSuccess());
+    ASSERT_EQ(res->getNumTuples(), 2);
+}
+
+// A header too short to hold the checksum flag holds no commits, so non-strict recovery still
+// drops the WAL whatever the configured flag.
+TEST_F(WalTest, TornWALHeaderNoThrow) {
+    if (inMemMode || systemConfig->checkpointThreshold == 0) {
+        GTEST_SKIP();
+    }
+    ASSERT_TRUE(conn->query("call force_checkpoint_on_close=false")->isSuccess());
+    ASSERT_TRUE(conn->query("call auto_checkpoint=false")->isSuccess());
+    ASSERT_TRUE(conn->query("create node table test1(id int64 primary key)")->isSuccess());
+    database.reset();
+    const auto walFilePath = lbug::storage::StorageUtils::getWALFilePath(databasePath);
+    // Shorter than the 16-byte database ID that starts the header.
+    std::filesystem::resize_file(walFilePath, 10);
+    systemConfig->enableChecksums = !systemConfig->enableChecksums;
     createDBAndConn();
     auto res = conn->query("CALL show_tables() WHERE name STARTS WITH 'test' RETURN *;");
     ASSERT_TRUE(res->isSuccess());
@@ -1363,6 +1413,25 @@ TEST_F(FrozenWALRecoveryTest, FrozenWALWithoutCheckpointRecordSurvivesSecondReco
     EXPECT_EQ(countTestNodes(), 150);
 }
 
+// A frozen WAL written with the other checksum setting is kept as well, also with
+// throwOnWalReplayFailure false (the fixture's setting). The WAL is written with the setting
+// flipped, the other direction from WALChecksumConfigMismatchNoThrow.
+TEST_F(FrozenWALRecoveryTest, FrozenWALWithOtherChecksumSettingIsKept) {
+    systemConfig->enableChecksums = !systemConfig->enableChecksums;
+    createDBAndConn();
+    insertRange(0, 100);
+    closeDB();
+    std::filesystem::rename(walFilePath, frozenWALFilePath);
+    const auto walBytes = readFileBytes(frozenWALFilePath);
+    ASSERT_FALSE(walBytes.empty());
+    systemConfig->enableChecksums = !systemConfig->enableChecksums;
+    expectChecksumSettingError();
+    EXPECT_EQ(readFileBytes(frozenWALFilePath), walBytes);
+    systemConfig->enableChecksums = !systemConfig->enableChecksums;
+    createDBAndConn();
+    EXPECT_EQ(countTestNodes(), 100);
+}
+
 // Same as above, but writes committed after the interrupted checkpoint are in an active WAL next
 // to the frozen one. Recovery must apply the frozen WAL first and keep both durable.
 TEST_F(FrozenWALRecoveryTest, FrozenAndActiveWALSurviveSecondRecovery) {