fix(storage): recover pinned checkpoints that a crash interrupts

docs/checkpoint_recovery.md

@@ -32,6 +32,34 @@ reached the OS — but power-loss durability of those directory entries is not g
 If a step fails after the commit point, the database refuses further writes until it is reopened,
 and recovery finishes the checkpoint.
 
+## Pinned checkpoints
+
+A checkpoint taken while a read-only transaction is open keeps the WAL that transaction may still
+need: it does not rotate the WAL, so its `CHECKPOINT` record stays in the active WAL and later
+commits follow it. Its cleanup writes the marker file `<db>.pinned.tmp` before removing any shadow
+file, and every `CHECKPOINT` record logged into the active WAL first removes the marker. The marker
+therefore means that every `CHECKPOINT` record in the active WAL has been applied.
+
+A transaction committed after a `CHECKPOINT` record proves that checkpoint completed, since the
+write gate is held until a checkpoint's cleanup ends and a failure after its record poisons the
+WAL. Recovery then replays only the records after the last `CHECKPOINT` and removes a remaining
+shadow file, which belongs to a later checkpoint that never logged its record. A `COMMIT` after a
+`CHECKPOINT` without its `BEGIN`, which the engine never writes, leaves the earlier rule in place:
+with a shadow file, only the records before the first `CHECKPOINT` replay. Records that end in no
+`COMMIT`, such as a torn commit, are a torn tail of the last `CHECKPOINT`, and the WAL counts as
+ending in that record.
+
+Recovering a WAL that ends in a `CHECKPOINT` record writes: it applies or removes the shadow files
+and retires the WAL. A read-only open therefore refuses such a database with `Cannot open database
+in read-only mode while checkpoint recovery is pending`, also when the `CHECKPOINT` is followed by
+a torn commit; opening it read-write once recovers it.
+
+When nothing was committed after the last `CHECKPOINT` record, the marker decides. With it,
+recovery discards the shadow files of the main database and partition children. Without it, a
+remaining shadow is replayed as the committed checkpoint's, and a missing one is an error. A marker
+written by an older build holds `1` instead of `2` and only proves that a missing shadow was
+applied. Named graphs still recover through their own shadow files.
+
 ## Checkpoint bundle format
 
 `CheckpointRecord::bundleFormatVersion` is 2 for checkpoints written by current builds, including

src/include/storage/checkpointer.h

@@ -81,6 +81,8 @@ class Checkpointer {
     // Runs only when WAL retirement actually executes; skipped alongside retirement while
     // the horizon pins the WAL.
     virtual void onWALRetired(bool walRotated);
+    // Runs after the pinned-checkpoint marker is written, before cleanup removes any shadow file.
+    virtual void beforeShadowRemoval();
     virtual void beforeGraphShadowApply(StorageManager& storageManager);
     void applyShadowPagesForPartitionChildren();
     void flushGraphShadowFiles();

src/include/storage/storage_utils.h

@@ -77,15 +77,21 @@ class StorageUtils {
     static std::string getShadowFilePath(const std::string& path) {
         return std::format("{}.{}", path, common::StorageConstants::SHADOWING_SUFFIX);
     }
-    // Marker proving a pinned checkpoint (CHECKPOINT record in the active WAL with no WAL
-    // rotation) completed and its shadow was applied before cleanup removed the shadow.
-    // Recovery only treats a trailing/mid-file active-WAL CHECKPOINT with no shadow as
-    // durable when this marker exists; otherwise a missing shadow is an error, never a
-    // silent WAL delete (out-of-band shadow loss must stay loud). Uses a `.tmp` suffix so
-    // the file-deletion allowlist permits cleanup.
+    // Marker proving that every CHECKPOINT record in the active WAL (left there by pinned
+    // checkpoints, which skip WAL rotation) has been applied: WAL::logAndFlushCheckpoint removes
+    // it before logging a record, and a pinned checkpoint's cleanup writes it again after
+    // applying its shadows, before removing them. With the marker, any shadow file recovery finds
+    // belongs to a later checkpoint that never logged its record, or to an interrupted cleanup.
+    // A marker from an older build (see below) only proves that a missing shadow was applied.
+    // Without any marker, CHECKPOINT records with no shadow are an error, never a silent WAL
+    // delete (out-of-band shadow loss must stay loud). Uses a `.tmp` suffix so the file-deletion
+    // allowlist permits cleanup.
     static std::string getPinnedCheckpointMarkerPath(const std::string& path) {
         return std::format("{}.pinned.tmp", path);
     }
+    // Content of the marker above. Older builds wrote 1 and kept the marker while logging a new
+    // CHECKPOINT record, so their marker does not prove a trailing record was applied.
+    static constexpr uint8_t PINNED_CHECKPOINT_MARKER_APPLIED = 2;
     static std::string getCheckpointIntentLockFilePath(const std::string& path) {
         return std::format("{}.checkpoint.intent.lock", path);
     }

src/include/storage/wal/wal.h

@@ -39,6 +39,8 @@ class WAL {
 
     void logCommittedWAL(LocalWAL& localWAL, main::ClientContext* context,
         uint64_t& commitSequence);
+    // Removes the pinned-checkpoint marker first, so it never vouches for a record whose shadow is
+    // not applied yet.
     void logAndFlushCheckpoint(main::ClientContext* context);
 
     // Renames the active WAL to the frozen checkpoint WAL. Returns false if there is nothing to
@@ -62,6 +64,9 @@ class WAL {
     // source for WAL pin observability alongside getFileSize() (active).
     uint64_t getFrozenWALFileSize();
     bool mayHaveCheckpointRecord();
+    // See StorageUtils::getPinnedCheckpointMarkerPath.
+    void writePinnedCheckpointMarker(main::ClientContext* context);
+    void removePinnedCheckpointMarker(main::ClientContext* context);
     void throwIfPoisoned();
     void poison(const std::string& reason);
 
@@ -76,11 +81,13 @@ class WAL {
     void waitForDurabilityNoLock(uint64_t commitSequence, std::unique_lock<std::mutex>& lck);
     void flushAndSyncNoLock();
     void writeHeader(main::ClientContext& context);
+    void removePinnedCheckpointMarkerNoLock(main::ClientContext* context);
 
 private:
     std::mutex mtx;
     std::string walPath;
     std::string checkpointWalPath;
+    std::string pinnedCheckpointMarkerPath;
     bool inMemory;
     [[maybe_unused]] bool readOnly;
     common::VirtualFileSystem* vfs;

src/include/storage/wal/wal_replayer.h

@@ -51,15 +51,15 @@ class WALReplayer {
         bool isLastRecordCheckpoint = false;
         // End offset of the last CHECKPOINT record, or 0 when the file holds none. A
         // pinned checkpoint skips WAL rotation, so its record can sit mid-file (with
-        // later commits after it); only the suffix past this offset replays, and only
-        // when no shadow file remains (see below).
+        // later commits after it); usually only the suffix past this offset replays.
         uint64_t lastCheckpointOffset = 0;
         // offsetDeserialized as of just before the first CHECKPOINT record (meaningful
-        // only when lastCheckpointOffset != 0). When a shadow file remains, some
-        // checkpoint may never have completed, so recovery replays exactly the
-        // pre-checkpoint prefix ending here and discards everything from the first
-        // CHECKPOINT record onward.
+        // only when lastCheckpointOffset != 0). A WAL whose CHECKPOINT records are not
+        // shown to be complete replays exactly the prefix ending here.
         uint64_t offsetBeforeFirstCheckpoint = 0;
+        // A BEGIN..COMMIT transaction follows the last CHECKPOINT record, which proves every
+        // CHECKPOINT in the file completed (see WALReplayer::replayActiveWAL).
+        bool hasTransactionAfterLastCheckpoint = false;
         common::uuid walDatabaseID{};
         uint64_t checkpointFormatVersion = 0;
     };

src/storage/checkpointer.cpp

@@ -443,6 +443,13 @@ void Checkpointer::postCheckpointCleanup(bool canResetPageManagerToCurrent) try
         }
         onWALRetired(mainWALRotated);
     }
+    // A pinned checkpoint keeps its CHECKPOINT record in the active WAL. The marker must be
+    // durable before any shadow is removed: recovery then discards the shadows instead of
+    // replaying a half-removed one.
+    if (pinWALRetirement) {
+        mainStorageManager->getWAL().writePinnedCheckpointMarker(&clientContext);
+    }
+    beforeShadowRemoval();
     for (const auto& target : checkpointTargets) {
         target.storageManager->getShadowFile().clear(*bufferManager, false);
         target.storageManager->getShadowFile().reset();
@@ -458,29 +465,9 @@ void Checkpointer::postCheckpointCleanup(bool canResetPageManagerToCurrent) try
     common::VirtualFileSystem::GetUnsafe(clientContext)
         ->syncParentDirectory(
             StorageUtils::getShadowFilePath(mainStorageManager->getDatabasePath()));
-    // Durability marker for pinned checkpoints: the active WAL holds CHECKPOINT records
-    // with no rotation, and cleanup above removed the shadow. Recovery treats such
-    // CHECKPOINTs as durable only when this marker exists; a missing shadow without the
-    // marker is an out-of-band loss and must throw, never silently delete the WAL.
-    if (!isInMemory) {
-        auto* vfs = common::VirtualFileSystem::GetUnsafe(clientContext);
-        const auto markerPath =
-            StorageUtils::getPinnedCheckpointMarkerPath(mainStorageManager->getDatabasePath());
-        if (pinWALRetirement) {
-            auto markerFile = vfs->openFile(markerPath,
-                common::FileOpenFlags(
-                    common::FileFlags::WRITE | common::FileFlags::CREATE_IF_NOT_EXISTS),
-                &clientContext);
-            const uint8_t done = 1;
-            markerFile->writeFile(&done, 1, 0);
-            markerFile->truncate(1);
-            markerFile->syncFile();
-            markerFile.reset();
-            vfs->syncParentDirectory(markerPath);
-        } else {
-            vfs->removeFileIfExists(markerPath, &clientContext);
-            vfs->syncParentDirectory(markerPath);
-        }
+    // An unpinned checkpoint retired the WAL above, so no CHECKPOINT record is left to vouch for.
+    if (!pinWALRetirement) {
+        mainStorageManager->getWAL().removePinnedCheckpointMarker(&clientContext);
     }
     bufferManager->removeEvictedCandidates();
     releaseCheckpointLocks();
@@ -576,6 +563,8 @@ void Checkpointer::beforeWALRetirement(bool) {}
 
 void Checkpointer::onWALRetired(bool) {}
 
+void Checkpointer::beforeShadowRemoval() {}
+
 void Checkpointer::beforeGraphShadowApply(StorageManager&) {}
 
 void Checkpointer::logCheckpointAndApplyShadowPages(bool walRotated_) {

src/storage/wal/wal.cpp

@@ -33,6 +33,7 @@ WAL::WAL(const std::string& dbPath, bool readOnly, bool enableChecksums, Virtual
     StorageManager* storageManager)
     : walPath{StorageUtils::getWALFilePath(dbPath)},
       checkpointWalPath{StorageUtils::getCheckpointWALFilePath(dbPath)},
+      pinnedCheckpointMarkerPath{StorageUtils::getPinnedCheckpointMarkerPath(dbPath)},
       inMemory{main::DBConfig::isDBPathInMemory(dbPath)}, readOnly{readOnly}, vfs{vfs},
       storageManager{storageManager}, enableChecksums(enableChecksums) {}
 
@@ -56,6 +57,7 @@ void WAL::logCommittedWAL(LocalWAL& localWAL, main::ClientContext* context,
 void WAL::logAndFlushCheckpoint(main::ClientContext* context) {
     std::unique_lock lck{mtx};
     throwIfPoisonedNoLock();
+    removePinnedCheckpointMarkerNoLock(context);
     initWriter(context);
     CheckpointRecord walRecord{CHECKPOINT_BUNDLE_FORMAT_VERSION};
     checkpointRecordMayExist = true;
@@ -362,6 +364,31 @@ bool WAL::mayHaveCheckpointRecord() {
     return checkpointRecordMayExist;
 }
 
+void WAL::writePinnedCheckpointMarker(main::ClientContext* context) {
+    std::unique_lock lck{mtx};
+    auto markerFile = vfs->openFile(pinnedCheckpointMarkerPath,
+        FileOpenFlags(FileFlags::WRITE | FileFlags::CREATE_IF_NOT_EXISTS), context);
+    const uint8_t applied = StorageUtils::PINNED_CHECKPOINT_MARKER_APPLIED;
+    markerFile->writeFile(&applied, 1, 0);
+    markerFile->truncate(1);
+    markerFile->syncFile();
+    markerFile.reset();
+    vfs->syncParentDirectory(pinnedCheckpointMarkerPath);
+}
+
+void WAL::removePinnedCheckpointMarker(main::ClientContext* context) {
+    std::unique_lock lck{mtx};
+    removePinnedCheckpointMarkerNoLock(context);
+}
+
+void WAL::removePinnedCheckpointMarkerNoLock(main::ClientContext* context) {
+    if (!vfs->fileOrPathExists(pinnedCheckpointMarkerPath, context)) {
+        return;
+    }
+    vfs->removeFileIfExists(pinnedCheckpointMarkerPath, context);
+    vfs->syncParentDirectory(pinnedCheckpointMarkerPath);
+}
+
 void WAL::throwIfPoisoned() {
     std::unique_lock lck{mtx};
     throwIfPoisonedNoLock();