ソースを参照

fix(persistence): include evicted documents in snapshots (v1.7.3)

Pre-v1.7.3 SnapshotManager::serializeStore() walked only the in-memory
documents map (`coll->documents`) and silently omitted everything that
had been evicted to a stub in `evictedDocs_`. Each snapshot rotation
then truncated the WAL past the snapshot's walSequence, deleting the
only remaining copy of the evicted set — so any doc that was evicted
between snapshots was lost forever on the next restart.

Symptoms on Zoe (production): doc count fell from 695,127 to 653,298
across one boot cycle (~42k docs gone). User-visible damage included
profile memory rows reset to empty content, conversation history gaps,
and any rarely-accessed row that the LRU happened to evict before the
next snapshot.

Fix: walk both getAllDocuments and getEvictedDocumentIds in
serializeStore, loading evicted bodies via peekEvictedDocument (same
path read queries already use to fault evicted docs back in). Reserve
8 bytes for the per-collection doc count and back-patch after writing
so failures-to-load don't desynchronise the deserializer's counter.

Adds a regression test that inserts 100 docs into a tiny-memory store,
forces eviction (~90 evicted, 10 in-memory), takes a snapshot, loads
into a fresh store, and verifies all 100 round-trip.
fszontagh 2 ヶ月 前
コミット
03412d9ad3
3 ファイル変更177 行追加9 行削除
  1. 1 1
      VERSION
  2. 52 8
      service/src/persistence/snapshot.cpp
  3. 124 0
      tests/test_snapshot_durability.cpp

+ 1 - 1
VERSION

@@ -1 +1 @@
-1.7.2
+1.7.3

+ 52 - 8
service/src/persistence/snapshot.cpp

@@ -602,17 +602,25 @@ std::vector<uint8_t> SnapshotManager::serializeStore(const MemoryStore& store,
         }
         result.insert(result.end(), optionsJson.begin(), optionsJson.end());
 
-        // Get all documents in collection
+        // Get all documents in collection. Snapshot must include both
+        // in-memory docs AND docs that have been evicted to stubs — otherwise
+        // truncateBefore(walSeq) after this snapshot deletes the WAL entries
+        // that hold the only remaining copy of the evicted docs, and they're
+        // gone forever. Pre-v1.7.3 only wrote in-memory docs; v1.7.0
+        // introduced eviction without a corresponding snapshot path, so any
+        // snapshot taken after eviction silently lost the evicted set.
         auto docs = store.getAllDocuments(name);
-        uint64_t collDocCount = docs.size();
-        docCount += collDocCount;
+        auto evictedIds = store.getEvictedDocumentIds(name);
 
-        // Write document count
-        for (int i = 0; i < 8; ++i) {
-            result.push_back(static_cast<uint8_t>((collDocCount >> (i * 8)) & 0xFF));
-        }
+        // Reserve 8 bytes for the doc count and back-patch after writing,
+        // so we can skip evicted docs that fail to load from WAL without
+        // desynchronising the deserializer's counter.
+        size_t countOffset = result.size();
+        for (int i = 0; i < 8; ++i) result.push_back(0);
 
-        // Write each document
+        uint64_t collDocCount = 0;
+
+        // Write each in-memory document
         for (const auto& doc : docs) {
             std::string docJson = doc.toJson().dump();
             uint32_t docLen = static_cast<uint32_t>(docJson.size());
@@ -620,8 +628,44 @@ std::vector<uint8_t> SnapshotManager::serializeStore(const MemoryStore& store,
                 result.push_back(static_cast<uint8_t>((docLen >> (i * 8)) & 0xFF));
             }
             result.insert(result.end(), docJson.begin(), docJson.end());
+            collDocCount++;
+        }
+
+        // Write each evicted document, loading from WAL via the document
+        // load callback. peekEvictedDocument is const and uses the same
+        // path that read queries use to fault evicted docs back in.
+        uint64_t evictedFailed = 0;
+        for (const auto& id : evictedIds) {
+            auto docOpt = store.peekEvictedDocument(name, id);
+            if (!docOpt) {
+                evictedFailed++;
+                continue;
+            }
+            std::string docJson = docOpt->toJson().dump();
+            uint32_t docLen = static_cast<uint32_t>(docJson.size());
+            for (int i = 0; i < 4; ++i) {
+                result.push_back(static_cast<uint8_t>((docLen >> (i * 8)) & 0xFF));
+            }
+            result.insert(result.end(), docJson.begin(), docJson.end());
+            collDocCount++;
+        }
+
+        if (evictedFailed > 0) {
+            spdlog::warn(
+                "Snapshot: {} evicted documents in collection '{}' could not "
+                "be loaded from WAL — they will be missing from the snapshot. "
+                "Likely a pre-v1.7.3 snapshot already lost them.",
+                evictedFailed, name);
+        }
+
+        // Back-patch the doc count
+        for (int i = 0; i < 8; ++i) {
+            result[countOffset + i] =
+                static_cast<uint8_t>((collDocCount >> (i * 8)) & 0xFF);
         }
 
+        docCount += collDocCount;
+
         // Write version history (v2)
         auto allHistory = store.getAllVersionHistory(name);
         uint64_t historyEntryCount = allHistory.size();

+ 124 - 0
tests/test_snapshot_durability.cpp

@@ -26,6 +26,7 @@
 #include <nlohmann/json.hpp>
 #include <thread>
 #include <unistd.h>  // getpid
+#include <unordered_set>
 
 namespace fs = std::filesystem;
 using smartbotic::database::MemoryStore;
@@ -400,6 +401,128 @@ void test_wal_sequence_floor_anchors_after_truncate() {
     std::cout << "PASS: WAL sequence floor anchors after snapshot truncate\n";
 }
 
+// ──────────────────────────────────────────────────────────
+// Test: snapshot must include evicted documents (v1.7.3 fix)
+// ──────────────────────────────────────────────────────────
+//
+// Pre-v1.7.3: serializeStore() walked only `coll->documents` (in-memory),
+// silently omitting docs that had been evicted to stubs. Each snapshot
+// rotation truncated the WAL past the snapshot's sequence, deleting the
+// only remaining copy of those evicted docs — they were lost forever
+// across the next restart.
+void test_snapshot_includes_evicted_documents() {
+    using smartbotic::database::WriteAheadLog;
+    auto snapDir = makeSnapshotDir("evict-snap");
+    auto walDir  = makeSnapshotDir("evict-wal");
+
+    WriteAheadLog::Config walCfg;
+    walCfg.walDir = walDir;
+    WriteAheadLog wal(walCfg);
+    if (!wal.open()) {
+        std::cerr << "FAIL: wal.open() returned false\n";
+        std::exit(2);
+    }
+
+    // Tiny memory limit so eviction fires under load.
+    MemoryStore::Config storeCfg;
+    storeCfg.nodeId = "test";
+    storeCfg.maxMemoryBytes = 32ULL * 1024;
+    storeCfg.evictionThresholdPercent = 50;
+    storeCfg.evictionTargetPercent = 30;
+    storeCfg.hotWriteFloorMs = 0;     // disable hot-write protection
+    storeCfg.evictionChunkSize = 1000;
+
+    MemoryStore store(storeCfg);
+    store.start();
+    store.createCollection("docs", CollectionOptions{});
+
+    // Wire load callback: scan WAL for the requested doc, same path
+    // production uses (database_service.cpp installs a similar lambda
+    // backed by PersistenceManager::loadDocument).
+    store.setDocumentLoadCallback(
+        [&wal](const std::string& coll, const std::string& id,
+               uint64_t /*walSeq*/) -> std::optional<Document> {
+            std::optional<Document> found;
+            wal.replay(0, [&](const smartbotic::database::WalEntry& e) {
+                if (e.collection == coll && e.documentId == id && e.data.has_value()) {
+                    Document d = Document::fromJson(*e.data);
+                    d.id = id;
+                    d.collection = coll;
+                    found = d;
+                }
+            });
+            return found;
+        });
+
+    // Insert N docs, mirroring to WAL so the load callback can resolve them.
+    constexpr int N = 100;
+    for (int i = 0; i < N; ++i) {
+        Document d = makeDoc("doc-" + std::to_string(i));
+        d.data["padding"] = std::string(300, 'x');  // make eviction meaningful
+        wal.append(WriteAheadLog::makeInsertEntry("docs", d, "test"));
+        store.insert("docs", d);
+    }
+
+    // Force eviction.
+    store.evictDocuments();
+    auto evictedIds = store.getEvictedDocumentIds("docs");
+    auto inMemoryDocs = store.getAllDocuments("docs");
+    assert(!evictedIds.empty());
+    assert(evictedIds.size() + inMemoryDocs.size() == static_cast<size_t>(N));
+
+    // Flush WAL so the load callback (which reads via ifstream) sees every
+    // entry. In production the WAL is synced periodically by the persistence
+    // manager; tests need to force it.
+    wal.sync();
+
+    // Take snapshot.
+    SnapshotManager::Config scfg;
+    scfg.snapshotDir = snapDir;
+    SnapshotManager mgr(scfg);
+    auto path = mgr.createSnapshot(store, wal.currentSequence());
+
+    // Load into fresh store with a huge memory budget so eviction doesn't
+    // fire during verification — we want to see every doc in-memory.
+    MemoryStore::Config newCfg = storeCfg;
+    newCfg.maxMemoryBytes = 256ULL * 1024 * 1024;
+    MemoryStore newStore(newCfg);
+    newStore.start();
+    mgr.loadSnapshot(path, newStore);
+
+    auto loadedDocs = newStore.getAllDocuments("docs");
+    auto loadedEvicted = newStore.getEvictedDocumentIds("docs");
+    const size_t totalLoaded = loadedDocs.size() + loadedEvicted.size();
+
+    // Real check (assert is a no-op in Release builds with NDEBUG).
+    if (totalLoaded != static_cast<size_t>(N)) {
+        std::cerr << "FAIL: snapshot lost docs — loaded "
+                  << loadedDocs.size() << " in-memory + "
+                  << loadedEvicted.size() << " evicted = "
+                  << totalLoaded << " of " << N << " inserted\n";
+        std::exit(1);
+    }
+
+    std::unordered_set<std::string> loadedIdSet;
+    for (const auto& d : loadedDocs) loadedIdSet.insert(d.id);
+    for (const auto& id : loadedEvicted) loadedIdSet.insert(id);
+    for (const auto& id : evictedIds) {
+        if (loadedIdSet.count(id) != 1) {
+            std::cerr << "FAIL: evicted doc " << id
+                      << " missing from loaded snapshot\n";
+            std::exit(1);
+        }
+    }
+
+    wal.close();
+    fs::remove_all(snapDir);
+    fs::remove_all(walDir);
+
+    std::cout << "PASS: snapshot includes evicted documents ("
+              << inMemoryDocs.size() << " in-memory + "
+              << evictedIds.size() << " evicted, all "
+              << N << " round-tripped)\n";
+}
+
 int main() {
     test_atomic_write_leaves_no_tmp();
     test_snapshot_round_trip();
@@ -409,6 +532,7 @@ int main() {
     test_loadWithFallback_throws_when_all_corrupt();
     test_v4_format_roundtrip_at_scale();
     test_wal_sequence_floor_anchors_after_truncate();
+    test_snapshot_includes_evicted_documents();
     std::cout << "\nAll snapshot durability tests PASSED!\n";
     return 0;
 }