Przeglądaj źródła

refactor(storage): drop defensive MemoryStore fallback from Exists/Get

The under-lock mirror in 85e0158 closes the race window: from any
reader's perspective, MemoryStore and LMDB are in sync at every
observable boundary. The defensive MemoryStore probe on LMDB-not-found
was insurance against a race that no longer exists.

Behaviour: LMDB-first remains gated on mirror_healthy_ + zero drift +
non-system collection + doc_store_ available. On LMDB throw, still
fall back to MemoryStore once. On LMDB absent, return absent — that
IS the answer.

Stress-validated: load_test_mixed still PASSes with zero hard failures
and zero LMDB errors. Throughput effectively unchanged (5146 reads/s)
since the LMDB-hit path was always the dominant cost.
fszontagh 2 miesięcy temu
rodzic
commit
f1ff6a7c62
1 zmienionych plików z 17 dodań i 21 usunięć
  1. 17 21
      service/src/database_grpc_impl.cpp

+ 17 - 21
service/src/database_grpc_impl.cpp

@@ -163,11 +163,12 @@ grpc::Status DatabaseGrpcImpl::Get(
         targetCollection = view->collection;
     }
 
-    // v2.0 Stage 4 — LMDB-first read. Same gate as Exists; on LMDB
-    // not-found, fall back to MemoryStore.get() to close the dual-write
-    // race window. On LMDB throw, fall back and log WARN. The fallback
-    // is a single in-memory probe (cheap), so the hot path is one LMDB
-    // get + one MemoryStore probe in the worst case.
+    // v2.0 Stage 4 — LMDB-first read. Gated on mirror_healthy_ + zero
+    // drift + non-system collection + doc_store_ available. The
+    // defensive MemoryStore-on-LMDB-absent fallback was removed once the
+    // dual-write moved under MemoryStore's per-collection lock — readers
+    // can no longer observe MemoryStore-has-but-LMDB-doesn't, so LMDB
+    // absent is now ground truth. On LMDB throw, fall back to MemoryStore.
     std::optional<Document> doc;
     bool used_lmdb = false;
     if (!targetCollection.empty() && targetCollection[0] != '_'
@@ -183,12 +184,8 @@ grpc::Status DatabaseGrpcImpl::Get(
             }
         }
     }
-    if (!used_lmdb || !doc) {
-        // LMDB blocked / threw / returned absent — try MemoryStore. For
-        // the LMDB-blocked case it's the only path. For the absent case
-        // it catches the mirror-write race for fresh inserts.
-        auto mem_doc = store_.get(targetCollection, request->id());
-        if (mem_doc) doc = std::move(mem_doc);
+    if (!used_lmdb) {
+        doc = store_.get(targetCollection, request->id());
     }
 
     if (!doc) {
@@ -479,11 +476,13 @@ grpc::Status DatabaseGrpcImpl::Exists(
     pb::ExistsResponse* response
 ) {
     // v2.0 Stage 4 — LMDB-first read. Gated on mirror_healthy_ + zero
-    // drift + non-system collection + doc_store_ available. On LMDB
-    // not-found, fall back to MemoryStore.exists() to close the small
-    // race window between MemoryStore lock release and dual-write commit
-    // (see memory_store.cpp:294 / 313). On LMDB throw, fall back to
-    // MemoryStore and log WARN.
+    // drift + non-system collection + doc_store_ available. The
+    // defensive MemoryStore-on-LMDB-miss fallback was removed once the
+    // dual-write mirror moved under MemoryStore's per-collection lock
+    // (see MemoryStore::mirrorWriteToDocStore + setDocumentStoreMirror).
+    // From any reader's perspective, MemoryStore and LMDB are now in
+    // sync at every observable boundary, so an LMDB not-found IS the
+    // ground truth. On LMDB throw, fall back to MemoryStore once.
     bool exists = false;
     bool used_lmdb = false;
     if (!request->collection().empty() && request->collection()[0] != '_'
@@ -499,11 +498,8 @@ grpc::Status DatabaseGrpcImpl::Exists(
             }
         }
     }
-    if (!used_lmdb || !exists) {
-        // Either gates blocked LMDB, LMDB threw, or LMDB said no — in the
-        // not-found case we also probe MemoryStore so a freshly-inserted
-        // doc whose mirror hasn't committed yet still reports exists=true.
-        exists = exists || store_.exists(request->collection(), request->id());
+    if (!used_lmdb) {
+        exists = store_.exists(request->collection(), request->id());
     }
 
     response->set_exists(exists);