Pārlūkot izejas kodu

feat(relations): DUPSORT reverse index with O(1) child counts

fszontagh 1 mēnesi atpakaļ
vecāks
revīzija
718e1eafb7

+ 1 - 0
service/CMakeLists.txt

@@ -58,6 +58,7 @@ set(DATABASE_SERVICE_SOURCES
     src/views/projection.cpp
     src/views/view_manager.cpp
     src/relations/relation_manager.cpp
+    src/relations/relation_index.cpp
     src/config/collection_config_manager.cpp
     src/config/config_loader.cpp
     src/storage/lmdb_env.cpp

+ 20 - 0
service/src/relations/relation_index.cpp

@@ -0,0 +1,20 @@
+// v2.11.0 T2 — reverse index sub-db naming for relations.
+
+#include "relations/relation_index.hpp"
+
+namespace smartbotic::db::storage {
+
+std::string relation_index_subdb(std::string_view relationBareName) {
+    std::string out;
+    out.reserve(kRelIndexPrefix.size() + relationBareName.size());
+    out.append(kRelIndexPrefix);
+    out.append(relationBareName);
+    return out;
+}
+
+bool is_relation_index_subdb(std::string_view subdb) {
+    return subdb.size() > kRelIndexPrefix.size() &&
+           subdb.compare(0, kRelIndexPrefix.size(), kRelIndexPrefix) == 0;
+}
+
+}  // namespace smartbotic::db::storage

+ 50 - 0
service/src/relations/relation_index.hpp

@@ -0,0 +1,50 @@
+// v2.11.0 T2 — reverse index sub-db naming for relations.
+//
+// A relation says "documents in `child` reference documents in `parent`
+// through `childField`". Enforcement (Restrict/Cascade/etc. on delete, and
+// DescribeDelete) needs the inverse question answered fast: "which child
+// documents reference this parent id, and how many?" Scanning `child` for
+// every delete would be a full collection scan per parent delete, which is
+// exactly the cost this repo already paid down once for secondary indexes
+// (see storage/secondary_index.hpp) - so the reverse index reuses that shape.
+//
+// Layout: one LMDB sub-db per relation, opened MDB_DUPSORT.
+//   key  = parent id
+//   data = child id
+// DUPSORT stores a parent's children as a sorted, deduplicated set - exactly
+// a posting list, and mdb_cursor_count answers "how many children" without
+// reading any of them (the operation restrict/DescribeDelete need).
+//
+// This header is intentionally free of relations/relation_manager.hpp - the
+// storage layer does not know about RelationInfo, project qualification, or
+// enforcement policy, the same way secondary_index.hpp does not know about
+// Query or FilterOp. Callers (Task 3+) pass the BARE relation name; a relation
+// sub-db lives inside a per-project LMDB env, so the project is already
+// implied by which env this is - qualifying here would double-encode it.
+
+#pragma once
+
+#include <string>
+#include <string_view>
+
+namespace smartbotic::db::storage {
+
+// Prefix marking a sub-db as a relation reverse index. Leading '_' means
+// is_system_subdb() already treats these as internal, so they stay out of
+// list_collections(). The trailing digit is a KEY FORMAT VERSION, the same
+// convention as kIndexSubdbPrefix: v2.9.1 had to bump _idx_ to _idx2_ when its
+// encoding changed, so a stale index is never read under new rules. Paying
+// that forward here costs nothing and avoids a repeat of that migration pain
+// if the relation index's shape ever needs to change.
+inline constexpr std::string_view kRelIndexPrefix = "_relidx1_";
+
+// Sub-db name for one relation's reverse index. `relationBareName` is the
+// UNQUALIFIED relation name (e.g. "exec_wf", not "myproj:exec_wf") - see the
+// file comment above for why.
+std::string relation_index_subdb(std::string_view relationBareName);
+
+// True if a sub-db name is a relation reverse index. Used to skip these when
+// walking sub-dbs and to open them with MDB_DUPSORT (see prime_dbi_cache).
+bool is_relation_index_subdb(std::string_view subdb);
+
+}  // namespace smartbotic::db::storage

+ 118 - 2
service/src/storage/document_store_lmdb.cpp

@@ -50,6 +50,7 @@
 #include "doc_binary.hpp"
 #include "document.hpp"
 #include "json_parse.hpp"
+#include "relations/relation_index.hpp"
 #include "storage/filter_eval.hpp"
 #include "storage/lmdb_dbi.hpp"
 #include "storage/lmdb_env.hpp"
@@ -449,8 +450,11 @@ size_t LmdbDocumentStore::prime_dbi_cache() {
     for (const auto& n : names) {
         MDB_dbi dbi = 0;
         // Index sub-dbs are MDB_DUPSORT; pass the flag on reopen so the handle
-        // agrees with how the sub-db was created.
-        const unsigned int flags = is_index_subdb(n) ? MDB_DUPSORT : 0u;
+        // agrees with how the sub-db was created. Relation reverse indexes
+        // (v2.11.0 T2) are DUPSORT for the same reason - one parent id key
+        // holding a sorted set of child ids.
+        const unsigned int flags =
+            (is_index_subdb(n) || is_relation_index_subdb(n)) ? MDB_DUPSORT : 0u;
         int rc = mdb_dbi_open(wtxn.raw(), n.c_str(), flags, &dbi);
         if (rc == MDB_NOTFOUND) continue;          // vanished under us; ignore
         if (rc != MDB_SUCCESS) throw_mdb(rc, "dbi_open (prime)");
@@ -996,6 +1000,118 @@ bool LmdbDocumentStore::drop_index(std::string_view collection,
     return true;
 }
 
+// -------------------------------------------------------------------------
+// v2.11.0 T2 — relation reverse index.
+//
+// Same DUPSORT-posting-list shape as secondary indexes above: key = parent
+// id, data = child id. mdb_cursor_count answers "how many children" without
+// reading any of them - the operation restrict/DescribeDelete (Task 4/5)
+// need. No document-write hook calls relation_index_add yet (Task 3); these
+// are pure storage primitives.
+// -------------------------------------------------------------------------
+
+bool LmdbDocumentStore::relation_index_add(std::string_view relation,
+                                            std::string_view parentId,
+                                            std::string_view childId) {
+    const std::string sub = relation_index_subdb(relation);
+    WriteTxn wtxn(env_);
+    const unsigned int dbi = open_for_write(wtxn, sub, MDB_DUPSORT);
+    MDB_val k = to_val(parentId);
+    MDB_val v = to_val(childId);
+    const int rc = mdb_put(wtxn.raw(), dbi, &k, &v, MDB_NODUPDATA);
+    // MDB_KEYEXIST under MDB_NODUPDATA means this exact (key, data) pair is
+    // already present - idempotent re-add, not an error.
+    if (rc != MDB_SUCCESS && rc != MDB_KEYEXIST) throw_mdb(rc, "relation index add");
+    wtxn.commit();
+    // MANDATORY - see the global constraint. A sub-db created at runtime is
+    // invisible to try_open_for_read until its opening transaction commits
+    // AND the handle is cached; skipping this leaves the read path reporting
+    // "no such sub-db" for the life of the process.
+    cacheCommittedDbi(sub, dbi);
+    return true;
+}
+
+bool LmdbDocumentStore::relation_index_remove(std::string_view relation,
+                                               std::string_view parentId,
+                                               std::string_view childId) {
+    const std::string sub = relation_index_subdb(relation);
+    {
+        ReadTxn rtxn(env_);
+        if (!try_open_for_read(rtxn, sub)) return false;
+    }
+    WriteTxn wtxn(env_);
+    const unsigned int dbi = open_for_write(wtxn, sub, MDB_DUPSORT);
+    MDB_val k = to_val(parentId);
+    MDB_val v = to_val(childId);
+    // Passing data narrows the delete to exactly this (key, data) pair -
+    // siblings under the same parent key are untouched.
+    const int rc = mdb_del(wtxn.raw(), dbi, &k, &v);
+    if (rc != MDB_SUCCESS && rc != MDB_NOTFOUND) throw_mdb(rc, "relation index remove");
+    wtxn.commit();
+    cacheCommittedDbi(sub, dbi);
+    return rc == MDB_SUCCESS;
+}
+
+uint64_t LmdbDocumentStore::relation_index_child_count(std::string_view relation,
+                                                         std::string_view parentId) {
+    const std::string sub = relation_index_subdb(relation);
+    ReadTxn rtxn(env_);
+    auto dbi_opt = try_open_for_read(rtxn, sub);
+    if (!dbi_opt) return 0;
+    MDB_cursor* cur = nullptr;
+    mdb_check(mdb_cursor_open(rtxn.raw(), *dbi_opt, &cur), "cursor_open (relidx)");
+    struct CursorGuard {
+        MDB_cursor* c;
+        ~CursorGuard() { if (c) mdb_cursor_close(c); }
+    } guard{cur};
+    MDB_val k = to_val(parentId);
+    MDB_val v{0, nullptr};
+    if (mdb_cursor_get(cur, &k, &v, MDB_SET) != MDB_SUCCESS) return 0;
+    size_t n = 0;
+    mdb_check(mdb_cursor_count(cur, &n), "cursor_count (relidx)");
+    return static_cast<uint64_t>(n);
+}
+
+std::vector<std::string>
+LmdbDocumentStore::relation_index_children(std::string_view relation,
+                                            std::string_view parentId,
+                                            size_t limit) {
+    std::vector<std::string> out;
+    if (limit == 0) return out;
+    const std::string sub = relation_index_subdb(relation);
+    ReadTxn rtxn(env_);
+    auto dbi_opt = try_open_for_read(rtxn, sub);
+    if (!dbi_opt) return out;
+    MDB_cursor* cur = nullptr;
+    mdb_check(mdb_cursor_open(rtxn.raw(), *dbi_opt, &cur), "cursor_open (relidx children)");
+    struct CursorGuard {
+        MDB_cursor* c;
+        ~CursorGuard() { if (c) mdb_cursor_close(c); }
+    } guard{cur};
+    MDB_val k = to_val(parentId);
+    MDB_val v{0, nullptr};
+    // MDB_SET positions the cursor on the key but leaves which dup is current
+    // unspecified across LMDB versions; MDB_FIRST_DUP makes it explicit.
+    int rc = mdb_cursor_get(cur, &k, &v, MDB_SET);
+    if (rc != MDB_SUCCESS) {
+        if (rc == MDB_NOTFOUND) return out;
+        throw_mdb(rc, "cursor_get (relidx children)");
+    }
+    rc = mdb_cursor_get(cur, &k, &v, MDB_FIRST_DUP);
+    if (rc != MDB_SUCCESS) {
+        if (rc == MDB_NOTFOUND) return out;
+        throw_mdb(rc, "cursor first_dup (relidx children)");
+    }
+    while (out.size() < limit) {
+        const std::string_view child = to_sv(v);
+        if (!is_index_meta_key(child)) out.emplace_back(child);
+        rc = mdb_cursor_get(cur, &k, &v, MDB_NEXT_DUP);
+        if (rc == MDB_NOTFOUND) break;
+        if (rc != MDB_SUCCESS) throw_mdb(rc, "cursor next_dup (relidx children)");
+    }
+    return out;
+}
+
 std::optional<smartbotic::database::Document>
 LmdbDocumentStore::get(std::string_view collection, std::string_view id) {
     // An empty id is a zero-length LMDB key, which mdb_get rejects with

+ 29 - 0
service/src/storage/document_store_lmdb.hpp

@@ -182,6 +182,35 @@ public:
     // Remove an index entirely.
     bool drop_index(std::string_view collection, const std::string& field);
 
+    // v2.11.0 T2 — relation reverse index: parent id -> child ids, DUPSORT.
+    // `relation` is the BARE relation name (see relations/relation_index.hpp);
+    // callers strip the project qualifier before reaching here. No document-
+    // write hooks call these yet (Task 3); no enforcement reads them yet
+    // (Task 4). This is storage only.
+
+    // Record that `childId` references `parentId` through `relation`.
+    // Idempotent: re-adding an existing pair is a no-op (MDB_NODUPDATA), not a
+    // duplicate posting.
+    bool relation_index_add(std::string_view relation, std::string_view parentId,
+                            std::string_view childId);
+
+    // Remove exactly the (parentId, childId) pair. Other children of the same
+    // parent are untouched. Returns false if the pair was not present.
+    bool relation_index_remove(std::string_view relation, std::string_view parentId,
+                               std::string_view childId);
+
+    // Number of children `parentId` has under `relation`, without reading them
+    // (mdb_cursor_count) — the O(1)-ish count restrict/DescribeDelete need. 0
+    // for an unreferenced parent or a relation with no index yet; neither is
+    // an error.
+    uint64_t relation_index_child_count(std::string_view relation,
+                                        std::string_view parentId);
+
+    // Up to `limit` child ids of `parentId` under `relation`. Empty if none.
+    std::vector<std::string> relation_index_children(std::string_view relation,
+                                                      std::string_view parentId,
+                                                      size_t limit);
+
     // Outcome of the constructor's priming pass, for the owner to report.
     // prime_error() is empty on success.
     size_t primed_count() const noexcept { return primed_count_; }

+ 39 - 0
tests/CMakeLists.txt

@@ -353,6 +353,7 @@ add_executable(test_document_store
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/lmdb_txn.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/lmdb_dbi.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/document_store_lmdb.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/relations/relation_index.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/secondary_index.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/subdb_identity.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/json_parse.cpp
@@ -392,6 +393,7 @@ add_executable(test_migrate_v1_to_v2
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/lmdb_txn.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/lmdb_dbi.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/document_store_lmdb.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/relations/relation_index.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/secondary_index.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/subdb_identity.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/migrate_v1_to_v2.cpp
@@ -448,6 +450,7 @@ add_executable(test_dual_write_mirror
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/lmdb_txn.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/lmdb_dbi.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/document_store_lmdb.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/relations/relation_index.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/secondary_index.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/subdb_identity.cpp
 )
@@ -533,6 +536,7 @@ add_executable(test_subdb_identity
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/lmdb_dbi.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/subdb_identity.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/document_store_lmdb.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/relations/relation_index.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/secondary_index.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/json_parse.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/doc_binary.cpp
@@ -555,6 +559,41 @@ endif()
 
 add_test(NAME test_subdb_identity COMMAND test_subdb_identity)
 
+# v2.11.0 T2 — relation reverse index (parent id -> child ids, DUPSORT).
+# Only the sources relation_index_{add,remove,child_count,children} actually
+# need: the LMDB store itself plus its direct dependencies. No relations/
+# relation_manager.cpp here - the storage layer is deliberately free of that
+# dependency (see relations/relation_index.hpp).
+add_executable(test_relation_index
+    test_relation_index.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/lmdb_env.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/lmdb_txn.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/lmdb_dbi.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/subdb_identity.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/document_store_lmdb.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/storage/secondary_index.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/relations/relation_index.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/json_parse.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../service/src/doc_binary.cpp
+)
+
+target_include_directories(test_relation_index PRIVATE
+    ${CMAKE_CURRENT_SOURCE_DIR}/../service/src
+    ${LMDB_INCLUDE_DIR}
+    ${yyjson_INCLUDE_DIRS}
+)
+
+target_link_libraries(test_relation_index PRIVATE ${LMDB_LIBRARY})
+target_link_libraries(test_relation_index PRIVATE ${yyjson_LIBRARIES})
+
+if(TARGET nlohmann_json::nlohmann_json)
+    target_link_libraries(test_relation_index PRIVATE nlohmann_json::nlohmann_json)
+else()
+    target_include_directories(test_relation_index PRIVATE ${NLOHMANN_JSON_INCLUDE_DIRS})
+endif()
+
+add_test(NAME test_relation_index COMMAND test_relation_index)
+
 # v2.9.0 — secondary index key encoding. Pins the one property that matters:
 # an index key comparison must mean the same thing as the scan's comparison,
 # because two paths answering one question that disagree return wrong data

+ 142 - 0
tests/test_relation_index.cpp

@@ -0,0 +1,142 @@
+// v2.11.0 T2 — relation reverse index tests.
+//
+// Storage-only: this exercises LmdbDocumentStore::relation_index_{add,remove,
+// child_count,children} directly. No document-write hooks (Task 3), no
+// enforcement (Task 4) are involved.
+//
+// test_index_created_at_runtime_is_visible_after_reopen is THE regression
+// test for this task: since v2.8.1 the read path serves only from the primed
+// dbi cache, so a sub-db created at runtime is invisible to every later read
+// in the process unless cacheCommittedDbi() runs after commit. Removing that
+// call from relation_index_add must fail this test.
+
+#include <algorithm>
+#include <atomic>
+#include <cstdio>
+#include <filesystem>
+#include <iostream>
+#include <string>
+#include <unistd.h>
+#include <vector>
+
+#include "storage/document_store_lmdb.hpp"
+#include "storage/lmdb_env.hpp"
+
+namespace fs = std::filesystem;
+
+using smartbotic::db::storage::LmdbDocumentStore;
+using smartbotic::db::storage::LmdbEnv;
+using smartbotic::db::storage::LmdbEnvOpts;
+
+namespace {
+
+int g_pass = 0;
+int g_fail = 0;
+
+void check(bool cond, const char* msg) {
+    if (cond) {
+        ++g_pass;
+    } else {
+        ++g_fail;
+        std::cerr << "FAIL: " << msg << "\n";
+    }
+}
+
+std::string make_tmpdir(const char* tag) {
+    static std::atomic<int> counter{0};
+    std::string path = "/tmp/relidx-test-" + std::to_string(::getpid()) + "-" +
+                       std::to_string(counter.fetch_add(1)) + "-" + tag;
+    std::error_code ec;
+    fs::remove_all(path, ec);
+    return path;
+}
+
+struct TmpEnv {
+    std::string path;
+    LmdbEnv env;
+    explicit TmpEnv(const char* tag)
+        : path(make_tmpdir(tag)),
+          env(LmdbEnvOpts{path, 64ULL << 20, 256, 126, false}) {}
+    ~TmpEnv() {
+        std::error_code ec;
+        fs::remove_all(path, ec);
+    }
+    TmpEnv(const TmpEnv&) = delete;
+    TmpEnv& operator=(const TmpEnv&) = delete;
+};
+
+// -------------------------------------------------------------------------
+
+// DUPSORT shape, per the re-validated design.
+void test_children_of_a_parent_are_a_dup_set() {
+    TmpEnv t("relidx");
+    LmdbDocumentStore store(t.env);
+
+    check(store.relation_index_add("r1", "wf-1", "exec-a"), "added a child");
+    check(store.relation_index_add("r1", "wf-1", "exec-b"), "and another");
+    check(store.relation_index_add("r1", "wf-2", "exec-c"), "under a second parent");
+
+    // THE operation restrict and DescribeDelete need: a count without reading
+    // the children. mdb_cursor_count makes it O(1)-ish.
+    check(store.relation_index_child_count("r1", "wf-1") == 2, "two children of wf-1");
+    check(store.relation_index_child_count("r1", "wf-2") == 1, "one child of wf-2");
+    check(store.relation_index_child_count("r1", "wf-none") == 0,
+          "an unreferenced parent has none, and that is not an error");
+
+    auto kids = store.relation_index_children("r1", "wf-1", 10);
+    std::sort(kids.begin(), kids.end());
+    check(kids == std::vector<std::string>{"exec-a", "exec-b"}, "children listed");
+
+    // Removing one pair must not remove the sibling.
+    check(store.relation_index_remove("r1", "wf-1", "exec-a"), "removed one pair");
+    check(store.relation_index_child_count("r1", "wf-1") == 1, "the sibling survives");
+
+    // Idempotent: re-adding the same pair is a no-op, not a duplicate.
+    store.relation_index_add("r1", "wf-1", "exec-b");
+    check(store.relation_index_child_count("r1", "wf-1") == 1, "no duplicate posting");
+
+    // A distinct relation is a distinct sub-db: same parent id, no crosstalk.
+    check(store.relation_index_child_count("r2", "wf-1") == 0,
+          "a different relation's index is independent");
+
+    // Removing a pair that was never present is reported, not thrown.
+    check(!store.relation_index_remove("r1", "wf-1", "exec-does-not-exist"),
+          "removing an absent pair returns false rather than throwing");
+    check(!store.relation_index_remove("r1", "wf-none", "exec-a"),
+          "removing under an absent parent returns false rather than throwing");
+}
+
+// The failure this repo is most likely to reproduce. Since v2.8.1 reads serve
+// only from the primed dbi cache, so a sub-db created at runtime is invisible
+// unless registered after commit - and relations would silently not enforce.
+void test_index_created_at_runtime_is_visible_after_reopen() {
+    const std::string path = make_tmpdir("relidx-visible");
+    const LmdbEnvOpts opts{path, 64ULL << 20, 256, 126, false};
+    {
+        LmdbEnv env(opts);
+        LmdbDocumentStore store(env);
+        store.relation_index_add("r1", "wf-1", "exec-a");
+        // Same process, no reopen: must be visible immediately.
+        check(store.relation_index_child_count("r1", "wf-1") == 1,
+              "visible in the process that created it - this is what "
+              "cacheCommittedDbi() buys");
+    }
+    LmdbEnv env2(opts);
+    LmdbDocumentStore store2(env2);      // priming runs in the constructor
+    check(store2.relation_index_child_count("r1", "wf-1") == 1,
+          "and after a restart, via prime_dbi_cache()");
+
+    std::error_code ec;
+    fs::remove_all(path, ec);
+}
+
+}  // namespace
+
+int main() {
+    std::cout << "=== test_relation_index ===\n";
+    test_children_of_a_parent_are_a_dup_set();
+    test_index_created_at_runtime_is_visible_after_reopen();
+
+    std::cout << "passed: " << g_pass << ", failed: " << g_fail << "\n";
+    return g_fail == 0 ? 0 : 1;
+}