From c419ece2c3ad5802cdc83c75d52669e728231dd5 Mon Sep 17 00:00:00 2001 From: Darek Date: Fri, 23 Oct 2026 00:45:58 +0200 Subject: [PATCH 1/3] fix(wal): throw on a checksum-setting mismatch instead of dropping the WAL With throwOnWalReplayFailure false, the engine default, opening a database with enableChecksums set the other way from its WAL's header deleted the WAL and every commit in it. dryReplay checked the flag inside the catch that treats read errors as a torn tail, so the mismatch looked like a WAL without a complete commit. The flag is now read with the plain reader before that try, the way attached-graph recovery already reads it, and a mismatch throws in both modes, leaving the WAL untouched. A header too short to hold the flag still takes the lenient path. This covers the main and the frozen checkpoint WAL, and read-only opens. Co-Authored-By: Claude Opus 5.5 --- docs/checkpoint_recovery.md | 4 +- src/include/main/database.h | 3 +- src/storage/wal/wal_replayer.cpp | 26 +++++++----- test/transaction/wal_test.cpp | 68 +++++++++++++++++++++++++++++++- 4 files changed, 89 insertions(+), 12 deletions(-) diff --git a/docs/checkpoint_recovery.md b/docs/checkpoint_recovery.md index bed775f34d..7d17b8840f 100644 --- a/docs/checkpoint_recovery.md +++ b/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 diff --git a/src/include/main/database.h b/src/include/main/database.h index f3553d7a11..92d6d833a7 100644 --- a/src/include/main/database.h +++ b/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 diff --git a/src/storage/wal/wal_replayer.cpp b/src/storage/wal/wal_replayer.cpp index c924a7824a..c4e5cbfe43 100644 --- a/src/storage/wal/wal_replayer.cpp +++ b/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 checkWALChecksumFlag(bool walEnableChecksums, bool enableChecksums) { + if (enableChecksums != walEnableChecksums) { 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(walEnableChecksums), TypeUtils::toString(enableChecksums))); } } @@ -253,10 +253,12 @@ 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. +// 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, so strict recovery rethrows +// and non-strict recovery may retire the WAL. static std::optional tryReadWALChecksumFlag(FileInfo& fileInfo, main::ClientContext& clientContext, bool throwOnWalReplayFailure) { try { @@ -528,7 +530,7 @@ void WALReplayer::replayGraphWAL(StorageManager& storageManager, auto deserializer = initDeserializer(*fileInfo, clientContext, *walEnableChecksums); deserializer.getReader()->onObjectBegin(); const auto walHeader = readWALHeader(deserializer); - checkWALHeader(walHeader, *walEnableChecksums); + checkWALChecksumFlag(walHeader.enableChecksums, *walEnableChecksums); FileDBIDUtils::verifyDatabaseID(*fileInfo, storageManager.getOrInitDatabaseID(clientContext), walHeader.databaseID); deserializer.getReader()->onObjectEnd(); @@ -928,13 +930,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 walEnableChecksums = tryReadWALChecksumFlag(fileInfo, clientContext, false); + if (walEnableChecksums.has_value()) { + checkWALChecksumFlag(*walEnableChecksums, 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(); diff --git a/test/transaction/wal_test.cpp b/test/transaction/wal_test.cpp index a7bde256a5..abb2343264 100644 --- a/test/transaction/wal_test.cpp +++ b/test/transaction/wal_test.cpp @@ -35,6 +35,7 @@ class WalTest : public ApiTest { void testStrayWALFile(const std::function& setupNewDBFunc); void setupChecksumMismatchTest(std::function corruptFunc); + void expectChecksumSettingError(); }; class FailingSyncFileSystem final : public FileSystem, public DirectorySyncFileSystem { @@ -864,6 +865,26 @@ TEST_F(WalTest, WALChecksumConfigMismatch) { lbug::common::RuntimeException); } +static std::vector readFileBytes(const std::string& path) { + std::ifstream stream{path, std::ios::binary}; + return {std::istreambuf_iterator{stream}, std::istreambuf_iterator{}}; +} + +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,22 @@ 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). +TEST_F(FrozenWALRecoveryTest, FrozenWALWithOtherChecksumSettingIsKept) { + 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) { From ff453d51c1bc75bf1686da1d5a65695cbbf6e25f Mon Sep 17 00:00:00 2001 From: Darek Date: Fri, 23 Oct 2026 01:12:15 +0200 Subject: [PATCH 2/3] test(wal): cover a plain WAL opened with checksums on The frozen-WAL mismatch test now writes its WAL with the checksum setting flipped, so together with the active-WAL test both directions of a mismatch run in CI. The comment on tryReadWALChecksumFlag now says which caller setting rethrows a torn header. Co-Authored-By: Claude Opus 5.5 --- src/storage/wal/wal_replayer.cpp | 4 ++-- test/transaction/wal_test.cpp | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/storage/wal/wal_replayer.cpp b/src/storage/wal/wal_replayer.cpp index c4e5cbfe43..315e962701 100644 --- a/src/storage/wal/wal_replayer.cpp +++ b/src/storage/wal/wal_replayer.cpp @@ -257,8 +257,8 @@ static uint64_t getReadOffset(Deserializer& deSer, bool enableChecksums) { // 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, so strict recovery rethrows -// and non-strict recovery may retire the WAL. +// 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 tryReadWALChecksumFlag(FileInfo& fileInfo, main::ClientContext& clientContext, bool throwOnWalReplayFailure) { try { diff --git a/test/transaction/wal_test.cpp b/test/transaction/wal_test.cpp index abb2343264..6107ad2ad1 100644 --- a/test/transaction/wal_test.cpp +++ b/test/transaction/wal_test.cpp @@ -1414,8 +1414,11 @@ TEST_F(FrozenWALRecoveryTest, FrozenWALWithoutCheckpointRecordSurvivesSecondReco } // A frozen WAL written with the other checksum setting is kept as well, also with -// throwOnWalReplayFailure false (the fixture's setting). +// 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); From e1210ecce29b9dfa7f2ef7a7740a8c022b8fefd1 Mon Sep 17 00:00:00 2001 From: Darek Date: Sat, 24 Oct 2026 19:45:42 +0200 Subject: [PATCH 3/3] refactor(wal): pass the WAL header to the checksum-setting check Two bare bools could be swapped at a call site without a compile error; checkWALHeader takes the header again and tryReadWALHeader returns it. Co-Authored-By: Claude Opus 5.5 --- src/storage/wal/wal_replayer.cpp | 34 +++++++++++++++++--------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/storage/wal/wal_replayer.cpp b/src/storage/wal/wal_replayer.cpp index 315e962701..46591319b4 100644 --- a/src/storage/wal/wal_replayer.cpp +++ b/src/storage/wal/wal_replayer.cpp @@ -234,14 +234,14 @@ static Deserializer initDeserializer(FileInfo& fileInfo, main::ClientContext& cl } } -static void checkWALChecksumFlag(bool walEnableChecksums, bool enableChecksums) { - if (enableChecksums != walEnableChecksums) { +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(walEnableChecksums), TypeUtils::toString(enableChecksums))); + TypeUtils::toString(walHeader.enableChecksums), TypeUtils::toString(enableChecksums))); } } @@ -259,14 +259,14 @@ static uint64_t getReadOffset(Deserializer& deSer, bool enableChecksums) { // 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 tryReadWALChecksumFlag(FileInfo& fileInfo, +static std::optional 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; @@ -383,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; } @@ -522,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); - checkWALChecksumFlag(walHeader.enableChecksums, *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); } @@ -933,9 +935,9 @@ WALReplayer::WALReplayInfo WALReplayer::dryReplay(FileInfo& fileInfo, bool throw // 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 walEnableChecksums = tryReadWALChecksumFlag(fileInfo, clientContext, false); - if (walEnableChecksums.has_value()) { - checkWALChecksumFlag(*walEnableChecksums, enableChecksums); + const auto persistedHeader = tryReadWALHeader(fileInfo, clientContext, false); + if (persistedHeader.has_value()) { + checkWALHeader(*persistedHeader, enableChecksums); } WALReplayInfo replayInfo; try {