ソースを参照

fix(storage): dropCollection must purge the vector sub-db (no leak)

dropCollection cleared the in-memory collection and the docs sub-db but left
the parallel _vectors_<collection> sub-db intact, and MemoryStore::dropCollection
never mirrored the drop to the LMDB doc-store at all. Net effect: dropped vectors
survived and leaked into a recreated same-name collection — search returned them
even though document_count read 0, and DELETE-by-id 404'd them.

- LmdbDocumentStore::drop_collection now drops BOTH the docs sub-db and the
  _vectors_<collection> sub-db atomically (in one write txn).
- MemoryStore::dropCollection now mirrors the drop into the doc-store
  (mirrorDropToDocStore), matching the per-doc remove() mirror path.
- Adds a regression test (drop_collection purges vectors).

Bumps VERSION 2.3.2 -> 2.3.3.
fszontagh 1 ヶ月 前
コミット
da7a43613f

+ 1 - 1
VERSION

@@ -1 +1 @@
-2.3.2
+2.3.3

+ 18 - 0
service/src/memory_store.cpp

@@ -147,6 +147,13 @@ bool MemoryStore::dropCollection(const std::string& name) {
         historyStore_->dropCollection(name);
     }
 
+    // v2.3 fix — also drop the collection from the LMDB doc-store mirror.
+    // Erasing it from memory above is not enough: the doc-store is the read
+    // source post-v2.0, so leftover rows there resurface (and leak into a
+    // recreated same-name collection). Per-doc remove() already mirrors a
+    // DELETE; dropCollection must do the equivalent for the whole collection.
+    mirrorDropToDocStore(name);
+
     {
         std::lock_guard<std::mutex> statsLock(statsMutex_);
         stats_.totalCollections--;
@@ -2317,6 +2324,17 @@ void MemoryStore::mirrorVectorToDocStore(const std::string& collection, const st
         pc.collection, id, vec, eventType);
 }
 
+void MemoryStore::mirrorDropToDocStore(const std::string& collection) {
+    if (!docStoreResolver_) return;
+    auto pc = smartbotic::database::parseProjectCollection(collection);
+    auto* ds = docStoreResolver_(pc.project);
+    if (!ds) return;
+    // Drop the collection's docs + vectors from the LMDB doc-store mirror.
+    // Without this, dropCollection only clears the in-memory state and the
+    // persisted rows leak into a recreated same-name collection.
+    ds->drop_collection(pc.collection);
+}
+
 void MemoryStore::addToExpirationIndex(CollectionData& coll, const std::string& id, uint64_t expiresAt) {
     coll.expirationIndex[expiresAt].insert(id);
 }

+ 8 - 0
service/src/memory_store.hpp

@@ -751,6 +751,14 @@ private:
     void mirrorVectorToDocStore(const std::string& collection, const std::string& id,
                                  const std::vector<float>* vec, EventType eventType);
 
+    // v2.3 fix — mirror a collection DROP into the LMDB DocumentStore.
+    // dropCollection() erases the in-memory collection, but the doc-store
+    // mirror is the read source post-v2.0; without dropping it there too the
+    // documents/vectors survive and leak into a recreated same-name collection
+    // (search returns them though document_count reads 0). No-op when the
+    // mirror isn't configured (legacy / test bootstrap path).
+    void mirrorDropToDocStore(const std::string& collection);
+
     // Update expiration index
     void addToExpirationIndex(CollectionData& coll, const std::string& id, uint64_t expiresAt);
     void removeFromExpirationIndex(CollectionData& coll, const std::string& id, uint64_t expiresAt);

+ 23 - 8
service/src/storage/document_store_lmdb.cpp

@@ -509,25 +509,40 @@ void LmdbDocumentStore::scan_vectors(
 }
 
 bool LmdbDocumentStore::drop_collection(std::string_view collection) {
-    // Probe with a read txn first; if the sub-db doesn't exist, return
-    // false without opening a write txn at all.
+    // A collection's data spans two sub-dbs: the docs sub-db `<collection>`
+    // and the parallel vectors sub-db `_vectors_<collection>`. Both must be
+    // dropped, else vectors survive a collection drop and resurface in search
+    // / leak into a recreated same-name collection.
+    const std::string vsubdb = vector_subdb_name(collection);
+
+    // Probe with a read txn first; if neither sub-db exists, return false
+    // without opening a write txn at all.
+    bool has_docs = false, has_vecs = false;
     {
         ReadTxn rtxn(env_);
-        auto dbi_opt = try_open_for_read(rtxn, collection);
-        if (!dbi_opt) return false;
+        has_docs = try_open_for_read(rtxn, collection).has_value();
+        has_vecs = try_open_for_read(rtxn, vsubdb).has_value();
     }
+    if (!has_docs && !has_vecs) return false;
 
     WriteTxn wtxn(env_);
-    unsigned int dbi = open_for_write(wtxn, collection);
     // mdb_drop with del=1 removes the sub-db itself (not just its entries).
-    mdb_check(mdb_drop(wtxn.raw(), dbi, 1), "drop");
+    if (has_docs) {
+        unsigned int dbi = open_for_write(wtxn, collection);
+        mdb_check(mdb_drop(wtxn.raw(), dbi, 1), "drop");
+    }
+    if (has_vecs) {
+        unsigned int vdbi = open_for_write(wtxn, vsubdb);
+        mdb_check(mdb_drop(wtxn.raw(), vdbi, 1), "drop_vectors");
+    }
     wtxn.commit();
 
-    // Drop invalidates the dbi handle — evict from cache so subsequent ops
-    // on this collection re-open with MDB_CREATE.
+    // Drop invalidates the dbi handles — evict from cache so subsequent ops
+    // re-open with MDB_CREATE.
     {
         std::lock_guard<std::mutex> lock(cache_mutex_);
         dbi_cache_.erase(std::string(collection));
+        dbi_cache_.erase(vsubdb);
     }
     return true;
 }

+ 26 - 0
tests/test_document_store.cpp

@@ -341,6 +341,31 @@ void test_drop_nonexistent_returns_false() {
           "drop_collection returns false for absent collection");
 }
 
+// Regression: drop_collection must purge the parallel _vectors_<coll> sub-db
+// too, not just the docs sub-db. Otherwise vectors survive a drop and leak
+// into a recreated same-name collection (search returns them though
+// document_count reads 0).
+void test_drop_collection_purges_vectors() {
+    TmpEnv t("drop-vectors");
+    LmdbDocumentStore store(t.env);
+
+    store.put("c", "a", make_doc("a", "c", nlohmann::json{{"v", 1}}));
+    store.put_vector("c", "a", std::vector<float>{1.0f, 0.0f, 0.0f, 0.0f});
+    check(store.get_vector("c", "a").has_value(), "precondition: vector present");
+
+    check(store.drop_collection("c"), "drop_collection returns true");
+    check(store.count("c") == 0, "post-drop doc count is 0");
+    check(!store.get_vector("c", "a").has_value(),
+          "post-drop get_vector returns nullopt (vector sub-db purged)");
+
+    // A drop with only a vectors sub-db (no docs) must still report success.
+    store.put_vector("vecsonly", "x", std::vector<float>{0.0f, 1.0f});
+    check(store.drop_collection("vecsonly"),
+          "drop_collection returns true when only the vectors sub-db exists");
+    check(!store.get_vector("vecsonly", "x").has_value(),
+          "vectors-only collection fully purged");
+}
+
 void test_scan_returns_all_for_empty_query() {
     TmpEnv t("scan-empty-query");
     LmdbDocumentStore store(t.env);
@@ -739,6 +764,7 @@ int main() {
     test_list_collections_after_inserts();
     test_drop_collection_removes_all_docs();
     test_drop_nonexistent_returns_false();
+    test_drop_collection_purges_vectors();
     test_scan_returns_all_for_empty_query();
     test_scan_returns_empty_for_nonexistent_collection();