|
|
@@ -60,6 +60,7 @@ using smartbotic::database::writeCascadeWal;
|
|
|
using smartbotic::db::storage::LmdbDocumentStore;
|
|
|
using smartbotic::db::storage::LmdbEnv;
|
|
|
using smartbotic::db::storage::LmdbEnvOpts;
|
|
|
+using smartbotic::db::storage::MissingParentReference;
|
|
|
using smartbotic::db::storage::RelationRef;
|
|
|
|
|
|
namespace {
|
|
|
@@ -1220,6 +1221,292 @@ void test_reinsert_after_delete_converges_in_lmdb() {
|
|
|
freshStore.stop();
|
|
|
}
|
|
|
|
|
|
+// -------------------------------------------------------------------------
|
|
|
+// v2.11.0 T13 — validate_on_write.
|
|
|
+//
|
|
|
+// The mitigation for the restrict race documented in relation_cascade.hpp
|
|
|
+// and the plan's self-review: a `restrict` check on delete runs in its own
|
|
|
+// read, so a child insert racing that check can still commit after the
|
|
|
+// parent is gone (LMDB serialises the two transactions, but the loser just
|
|
|
+// commits second). validate_on_write closes it by re-checking the parent's
|
|
|
+// existence with an mdb_get on the parent's own sub-db INSIDE the child's
|
|
|
+// write transaction - see LmdbDocumentStore::maintainRelations. What makes
|
|
|
+// it a genuine fix rather than a narrower window: the check and the write
|
|
|
+// commit as one unit, so there is no interval between them for the parent
|
|
|
+// to vanish in.
|
|
|
+// -------------------------------------------------------------------------
|
|
|
+
|
|
|
+void test_validate_on_write_rejects_missing_parent() {
|
|
|
+ TmpEnv t("validate-missing");
|
|
|
+ LmdbDocumentStore store(t.env);
|
|
|
+ store.set_relations("executions",
|
|
|
+ {{"exec_wf", "workflowId", "workflows", true}});
|
|
|
+
|
|
|
+ Document d; d.id = "e1"; d.collection = "executions";
|
|
|
+ d.set_data({{"workflowId", "wf-ghost"}});
|
|
|
+
|
|
|
+ bool threw = false;
|
|
|
+ std::string what;
|
|
|
+ try {
|
|
|
+ store.put("executions", "e1", d);
|
|
|
+ } catch (const MissingParentReference& e) {
|
|
|
+ threw = true;
|
|
|
+ what = e.what();
|
|
|
+ }
|
|
|
+ check(threw, "validateOnWrite=true rejects a reference to a nonexistent parent");
|
|
|
+ check(what.find("exec_wf") != std::string::npos, "error names the relation");
|
|
|
+ check(what.find("workflowId") != std::string::npos, "error names the child field");
|
|
|
+ check(what.find("wf-ghost") != std::string::npos, "error names the missing parent id");
|
|
|
+
|
|
|
+ // A rejected write must leave no trace - not the document, not the
|
|
|
+ // reverse index posting.
|
|
|
+ check(!store.get("executions", "e1").has_value(),
|
|
|
+ "rejected insert left no document behind");
|
|
|
+ check(store.relation_index_child_count("exec_wf", "wf-ghost") == 0,
|
|
|
+ "rejected insert left no reverse-index posting behind");
|
|
|
+}
|
|
|
+
|
|
|
+void test_validate_on_write_accepts_existing_parent() {
|
|
|
+ TmpEnv t("validate-present");
|
|
|
+ LmdbDocumentStore store(t.env);
|
|
|
+ store.set_relations("executions",
|
|
|
+ {{"exec_wf", "workflowId", "workflows", true}});
|
|
|
+
|
|
|
+ Document w; w.id = "wf-1"; w.collection = "workflows";
|
|
|
+ w.set_data({{"name", "real workflow"}});
|
|
|
+ store.put("workflows", "wf-1", w);
|
|
|
+
|
|
|
+ Document d; d.id = "e1"; d.collection = "executions";
|
|
|
+ d.set_data({{"workflowId", "wf-1"}});
|
|
|
+ bool threw = false;
|
|
|
+ try {
|
|
|
+ store.put("executions", "e1", d);
|
|
|
+ } catch (const MissingParentReference&) {
|
|
|
+ threw = true;
|
|
|
+ }
|
|
|
+ check(!threw, "validateOnWrite=true accepts a reference to an existing parent");
|
|
|
+ check(store.get("executions", "e1").has_value(), "the child document was actually written");
|
|
|
+ check(store.relation_index_child_count("exec_wf", "wf-1") == 1,
|
|
|
+ "and the reverse index posting was written");
|
|
|
+}
|
|
|
+
|
|
|
+// With validateOnWrite=false (the default), the same missing-parent insert
|
|
|
+// succeeds and check_relation_dangling reports it - the brief's Step 1 case,
|
|
|
+// both halves.
|
|
|
+void test_validate_on_write_false_allows_dangling_and_check_reports_it() {
|
|
|
+ TmpEnv t("validate-off");
|
|
|
+ LmdbDocumentStore store(t.env);
|
|
|
+ store.set_relations("executions",
|
|
|
+ {{"exec_wf", "workflowId", "workflows", false}});
|
|
|
+
|
|
|
+ Document d; d.id = "e1"; d.collection = "executions";
|
|
|
+ d.set_data({{"workflowId", "wf-ghost"}});
|
|
|
+ bool threw = false;
|
|
|
+ try {
|
|
|
+ store.put("executions", "e1", d);
|
|
|
+ } catch (const MissingParentReference&) {
|
|
|
+ threw = true;
|
|
|
+ }
|
|
|
+ check(!threw, "validateOnWrite=false lets the insert through");
|
|
|
+ check(store.get("executions", "e1").has_value(), "the dangling child document was written");
|
|
|
+
|
|
|
+ auto result = store.check_relation_dangling("exec_wf", "workflows", 100);
|
|
|
+ check(result.total == 1, "relations check reports one dangling parent id");
|
|
|
+ check(result.entries.size() == 1 && result.entries[0].parentId == "wf-ghost",
|
|
|
+ "and it is the ghost id");
|
|
|
+ check(result.entries[0].childCount == 1 &&
|
|
|
+ !result.entries[0].sampleChildIds.empty() &&
|
|
|
+ result.entries[0].sampleChildIds[0] == "e1",
|
|
|
+ "naming the dangling child");
|
|
|
+}
|
|
|
+
|
|
|
+// Absent and null references are not references at all (same rule as T3's
|
|
|
+// index maintenance) - they must never be rejected, even under
|
|
|
+// validateOnWrite=true.
|
|
|
+void test_validate_on_write_never_rejects_absent_or_null() {
|
|
|
+ TmpEnv t("validate-absent-null");
|
|
|
+ LmdbDocumentStore store(t.env);
|
|
|
+ store.set_relations("executions",
|
|
|
+ {{"exec_wf", "workflowId", "workflows", true}});
|
|
|
+
|
|
|
+ Document d1; d1.id = "e1"; d1.collection = "executions";
|
|
|
+ d1.set_data({{"other", 1}}); // workflowId absent entirely
|
|
|
+ bool threw1 = false;
|
|
|
+ try {
|
|
|
+ store.put("executions", "e1", d1);
|
|
|
+ } catch (const MissingParentReference&) {
|
|
|
+ threw1 = true;
|
|
|
+ }
|
|
|
+ check(!threw1, "an absent reference field is never rejected");
|
|
|
+
|
|
|
+ Document d2; d2.id = "e2"; d2.collection = "executions";
|
|
|
+ d2.set_data({{"workflowId", nullptr}});
|
|
|
+ bool threw2 = false;
|
|
|
+ try {
|
|
|
+ store.put("executions", "e2", d2);
|
|
|
+ } catch (const MissingParentReference&) {
|
|
|
+ threw2 = true;
|
|
|
+ }
|
|
|
+ check(!threw2, "an explicit null reference is never rejected");
|
|
|
+}
|
|
|
+
|
|
|
+// Array-valued reference decision: ANY missing parent id rejects the WHOLE
|
|
|
+// write, not just that element - fail closed, and no partial state (some
|
|
|
+// elements resolvable, others not) is left behind.
|
|
|
+void test_validate_on_write_array_any_missing_rejects_whole_write() {
|
|
|
+ TmpEnv t("validate-array");
|
|
|
+ LmdbDocumentStore store(t.env);
|
|
|
+ store.set_relations("nodes",
|
|
|
+ {{"node_creds", "config.credentialIds", "credentials", true}});
|
|
|
+
|
|
|
+ Document c1; c1.id = "c1"; c1.collection = "credentials";
|
|
|
+ c1.set_data({{"name", "real cred"}});
|
|
|
+ store.put("credentials", "c1", c1);
|
|
|
+ // c2 is deliberately never created.
|
|
|
+
|
|
|
+ Document n; n.id = "n1"; n.collection = "nodes";
|
|
|
+ n.set_data({{"config", {{"credentialIds", {"c1", "c2"}}}}});
|
|
|
+ bool threw = false;
|
|
|
+ std::string what;
|
|
|
+ try {
|
|
|
+ store.put("nodes", "n1", n);
|
|
|
+ } catch (const MissingParentReference& e) {
|
|
|
+ threw = true;
|
|
|
+ what = e.what();
|
|
|
+ }
|
|
|
+ check(threw, "one missing element in an array reference rejects the whole write");
|
|
|
+ check(what.find("c2") != std::string::npos, "error names the missing element, not the valid one");
|
|
|
+ check(!store.get("nodes", "n1").has_value(),
|
|
|
+ "rejected write left no document behind");
|
|
|
+ check(store.relation_index_child_count("node_creds", "c1") == 0,
|
|
|
+ "rejected write left no posting for the VALID element either - "
|
|
|
+ "no partial index state from a rejected write");
|
|
|
+ check(store.relation_index_child_count("node_creds", "c2") == 0,
|
|
|
+ "and none for the missing one");
|
|
|
+
|
|
|
+ // With every element resolvable, the write goes through and every
|
|
|
+ // element gets its posting.
|
|
|
+ Document c2; c2.id = "c2"; c2.collection = "credentials";
|
|
|
+ c2.set_data({{"name", "the missing one, now created"}});
|
|
|
+ store.put("credentials", "c2", c2);
|
|
|
+ threw = false;
|
|
|
+ try {
|
|
|
+ store.put("nodes", "n1", n);
|
|
|
+ } catch (const MissingParentReference&) {
|
|
|
+ threw = true;
|
|
|
+ }
|
|
|
+ check(!threw, "once every element resolves, the write succeeds");
|
|
|
+ check(store.relation_index_child_count("node_creds", "c1") == 1, "posting for c1");
|
|
|
+ check(store.relation_index_child_count("node_creds", "c2") == 1, "posting for c2");
|
|
|
+}
|
|
|
+
|
|
|
+// An update that does not touch the reference field is not re-validated,
|
|
|
+// even if the previously-written reference has since gone dangling (e.g. the
|
|
|
+// parent was removed out from under a no_action/validateOnWrite=false-era
|
|
|
+// row, or validateOnWrite was turned on after the fact). Only NEWLY
|
|
|
+// introduced references (`to_add`) are checked - see maintainRelations'
|
|
|
+// comment for why: an unchanged reference already existed (or was already
|
|
|
+// dangling) before this write, and this task closes the race for writes
|
|
|
+// that introduce a reference, not for the mere fact that time has passed
|
|
|
+// since one was previously accepted.
|
|
|
+void test_validate_on_write_unrelated_update_not_rechecked() {
|
|
|
+ TmpEnv t("validate-unrelated-update");
|
|
|
+ LmdbDocumentStore store(t.env);
|
|
|
+ store.set_relations("executions",
|
|
|
+ {{"exec_wf", "workflowId", "workflows", true}});
|
|
|
+
|
|
|
+ Document w; w.id = "wf-1"; w.collection = "workflows";
|
|
|
+ w.set_data({{"name", "real"}});
|
|
|
+ store.put("workflows", "wf-1", w);
|
|
|
+
|
|
|
+ Document d1; d1.id = "e1"; d1.collection = "executions";
|
|
|
+ d1.set_data({{"workflowId", "wf-1"}, {"status", "running"}});
|
|
|
+ store.put("executions", "e1", d1); // accepted: parent exists
|
|
|
+
|
|
|
+ store.del("workflows", "wf-1"); // parent now gone; reference dangles
|
|
|
+
|
|
|
+ // Rewrite e1 touching only `status` - workflowId is unchanged, so this
|
|
|
+ // must NOT re-validate it and must NOT throw.
|
|
|
+ Document d2; d2.id = "e1"; d2.collection = "executions";
|
|
|
+ d2.set_data({{"workflowId", "wf-1"}, {"status", "completed"}});
|
|
|
+ bool threw = false;
|
|
|
+ try {
|
|
|
+ store.put("executions", "e1", d2);
|
|
|
+ } catch (const MissingParentReference&) {
|
|
|
+ threw = true;
|
|
|
+ }
|
|
|
+ check(!threw, "an update that leaves the reference field unchanged is not re-validated");
|
|
|
+ check(store.get("executions", "e1")->data()["status"] == "completed",
|
|
|
+ "the update itself still applied");
|
|
|
+}
|
|
|
+
|
|
|
+// v2.11.0 T13 — proves the race is actually closed, not merely narrowed.
|
|
|
+//
|
|
|
+// Models the exact interleaving the plan describes: something (an
|
|
|
+// application-level pre-check, or the old restrict path's own read) observes
|
|
|
+// the parent present, and only AFTER that does the parent get deleted -
|
|
|
+// before the child's write actually lands. A stale check-then-act sequence
|
|
|
+// would let the child insert through anyway, because its answer was decided
|
|
|
+// against the state as of the check, not as of the write.
|
|
|
+//
|
|
|
+// LMDB is single-writer (see try_open_for_read's file comment and
|
|
|
+// document_store_lmdb.cpp's env setup): every write transaction begins only
|
|
|
+// after the previous one has fully committed, so the child's write
|
|
|
+// transaction here necessarily starts strictly after the parent-delete
|
|
|
+// transaction commits. Because validate_on_write's mdb_get runs INSIDE the
|
|
|
+// child's own write transaction rather than in a separate, earlier read, it
|
|
|
+// sees the parent's true state as of the write, not as of whatever was
|
|
|
+// observed before. That is the whole mechanism this task adds: no separate
|
|
|
+// transaction, no interval, nothing that can go stale.
|
|
|
+//
|
|
|
+// What this test does NOT establish: it does not exercise real multi-thread
|
|
|
+// scheduling or prove there is no OTHER race at the LMDB layer. It does not
|
|
|
+// need to - LMDB's single-writer guarantee means transaction ORDER is the
|
|
|
+// only thing that can vary under concurrency, never interleaving within a
|
|
|
+// transaction, so serialising the two operations in program order is the
|
|
|
+// honest, deterministic equivalent of "the delete's transaction commits
|
|
|
+// before the child insert's transaction begins," which is the only
|
|
|
+// interleaving the race actually depends on.
|
|
|
+void test_validate_on_write_closes_the_stale_check_race() {
|
|
|
+ TmpEnv t("validate-race");
|
|
|
+ LmdbDocumentStore store(t.env);
|
|
|
+ store.set_relations("executions",
|
|
|
+ {{"exec_wf", "workflowId", "workflows", true}});
|
|
|
+
|
|
|
+ Document w; w.id = "wf-1"; w.collection = "workflows";
|
|
|
+ w.set_data({{"name", "about to be deleted"}});
|
|
|
+ store.put("workflows", "wf-1", w);
|
|
|
+
|
|
|
+ // The "stale check": some caller observes the parent present. This is
|
|
|
+ // exactly what a restrict check (or an application's own pre-flight
|
|
|
+ // lookup) does - a READ, complete and finished, before the write it is
|
|
|
+ // meant to gate.
|
|
|
+ check(store.get("workflows", "wf-1").has_value(),
|
|
|
+ "pre-check observes the parent present");
|
|
|
+
|
|
|
+ // The parent vanishes AFTER that check returned, in its own committed
|
|
|
+ // transaction - the race window a stale check cannot see across.
|
|
|
+ check(store.del("workflows", "wf-1"), "parent deleted after the check ran");
|
|
|
+
|
|
|
+ // The child write's OWN transaction begins only now, strictly after the
|
|
|
+ // delete's commit (LMDB's single-writer serialisation). If validation
|
|
|
+ // used the stale check's answer (or any state cached before this point)
|
|
|
+ // it would wrongly accept. It must instead re-read the parent's current
|
|
|
+ // state inside its own transaction and reject.
|
|
|
+ Document d; d.id = "e1"; d.collection = "executions";
|
|
|
+ d.set_data({{"workflowId", "wf-1"}});
|
|
|
+ bool threw = false;
|
|
|
+ try {
|
|
|
+ store.put("executions", "e1", d);
|
|
|
+ } catch (const MissingParentReference&) {
|
|
|
+ threw = true;
|
|
|
+ }
|
|
|
+ check(threw, "the child write is rejected against write-time state, "
|
|
|
+ "despite an earlier check having observed the parent present");
|
|
|
+ check(!store.get("executions", "e1").has_value(),
|
|
|
+ "no trace of the write the stale check would have allowed");
|
|
|
+}
|
|
|
+
|
|
|
} // namespace
|
|
|
|
|
|
int main() {
|
|
|
@@ -1241,6 +1528,13 @@ int main() {
|
|
|
test_replayed_cascade_update_remirrors_to_lmdb_after_crash_window();
|
|
|
test_a_failing_row_does_not_stop_recovery();
|
|
|
test_reinsert_after_delete_converges_in_lmdb();
|
|
|
+ test_validate_on_write_rejects_missing_parent();
|
|
|
+ test_validate_on_write_accepts_existing_parent();
|
|
|
+ test_validate_on_write_false_allows_dangling_and_check_reports_it();
|
|
|
+ test_validate_on_write_never_rejects_absent_or_null();
|
|
|
+ test_validate_on_write_array_any_missing_rejects_whole_write();
|
|
|
+ test_validate_on_write_unrelated_update_not_rechecked();
|
|
|
+ test_validate_on_write_closes_the_stale_check_race();
|
|
|
|
|
|
std::cout << "passed: " << g_pass << ", failed: " << g_fail << "\n";
|
|
|
return g_fail == 0 ? 0 : 1;
|