|
|
@@ -1440,33 +1440,155 @@ void test_validate_on_write_unrelated_update_not_rechecked() {
|
|
|
"the update itself still applied");
|
|
|
}
|
|
|
|
|
|
-// v2.11.0 T13 — proves the race is actually closed, not merely narrowed.
|
|
|
+// v2.11.0 T13 round 2 (review finding) — rollback of an ALREADY-APPLIED
|
|
|
+// index mutation within the same write, not merely "the mutation never
|
|
|
+// happened because validation ran first."
|
|
|
//
|
|
|
-// 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.
|
|
|
+// The earlier "left no posting behind" assertions are structurally weak on
|
|
|
+// their own: validate_on_write's check runs before that RELATION's own
|
|
|
+// index sub-db is even opened, so on rejection the sub-db often never
|
|
|
+// exists and relation_index_child_count returns 0 through the nullopt path
|
|
|
+// regardless of whether LMDB actually rolled anything back. This test
|
|
|
+// forces a real rollback to matter: `executions` declares TWO relations.
|
|
|
+// The first (wf_rel, validateOnWrite=false) has its posting WRITTEN - a
|
|
|
+// real mdb_put against a real sub-db, inside the loop, before the second
|
|
|
+// relation is even considered. The second (owner_rel, validateOnWrite=true)
|
|
|
+// then rejects. Both relations share the one write transaction `put()`
|
|
|
+// opens, so the abort must undo wf_rel's already-applied mdb_put along with
|
|
|
+// everything else - there is no sub-db-never-existed shortcut available
|
|
|
+// here, because it demonstrably did exist and did get written to.
|
|
|
+void test_validate_on_write_rolls_back_an_already_applied_sibling_relation() {
|
|
|
+ TmpEnv t("validate-rollback-sibling");
|
|
|
+ LmdbDocumentStore store(t.env);
|
|
|
+ store.set_relations("executions", {
|
|
|
+ {"wf_rel", "workflowId", "workflows", false}, // validateOnWrite=false
|
|
|
+ {"owner_rel", "ownerId", "users", true}, // validateOnWrite=true
|
|
|
+ });
|
|
|
+
|
|
|
+ Document w; w.id = "wf-1"; w.collection = "workflows";
|
|
|
+ w.set_data({{"name", "real workflow"}});
|
|
|
+ store.put("workflows", "wf-1", w);
|
|
|
+ // Deliberately no "users/u-ghost" - owner_rel's parent never exists.
|
|
|
+
|
|
|
+ Document e; e.id = "e1"; e.collection = "executions";
|
|
|
+ e.set_data({{"workflowId", "wf-1"}, {"ownerId", "u-ghost"}});
|
|
|
+ bool threw = false;
|
|
|
+ try {
|
|
|
+ store.put("executions", "e1", e);
|
|
|
+ } catch (const MissingParentReference&) {
|
|
|
+ threw = true;
|
|
|
+ }
|
|
|
+ check(threw, "owner_rel's missing parent rejects the write");
|
|
|
+ check(!store.get("executions", "e1").has_value(),
|
|
|
+ "the document itself was rolled back");
|
|
|
+ check(store.relation_index_child_count("owner_rel", "u-ghost") == 0,
|
|
|
+ "owner_rel (the relation that rejected) has no posting");
|
|
|
+ check(store.relation_index_child_count("wf_rel", "wf-1") == 0,
|
|
|
+ "wf_rel (the EARLIER, already-applied sibling relation) was rolled "
|
|
|
+ "back too - LMDB's transaction abort undid a real mdb_put, not "
|
|
|
+ "just 'the sub-db never got created'");
|
|
|
+
|
|
|
+ // Once owner_rel's parent exists, the identical write succeeds and BOTH
|
|
|
+ // relations end up with their postings - confirming the rollback above
|
|
|
+ // was real and not a side effect of some other bug losing wf_rel's
|
|
|
+ // posting permanently.
|
|
|
+ Document u; u.id = "u-ghost"; u.collection = "users";
|
|
|
+ u.set_data({{"name", "real user, now created"}});
|
|
|
+ store.put("users", "u-ghost", u);
|
|
|
+ threw = false;
|
|
|
+ try {
|
|
|
+ store.put("executions", "e1", e);
|
|
|
+ } catch (const MissingParentReference&) {
|
|
|
+ threw = true;
|
|
|
+ }
|
|
|
+ check(!threw, "once owner_rel's parent exists too, the write succeeds");
|
|
|
+ check(store.relation_index_child_count("wf_rel", "wf-1") == 1, "wf_rel posted");
|
|
|
+ check(store.relation_index_child_count("owner_rel", "u-ghost") == 1, "owner_rel posted");
|
|
|
+}
|
|
|
+
|
|
|
+// v2.11.0 T13 round 2 (review finding 1) — relationsEnforced is the
|
|
|
+// documented escape hatch (config/collection_config_manager.hpp) for "a
|
|
|
+// bulk import, or a collection under write pressure." Before this, the
|
|
|
+// only consumer was RelationEnforcer::canDelete (restrict/no_action on
|
|
|
+// delete); validate_on_write did not read it at all, so the only way to
|
|
|
+// stop a validate_on_write rejection was to drop and re-declare the
|
|
|
+// relation without validateOnWrite - not what the switch is for. Disabling
|
|
|
+// enforcement must also let a bulk-import-shaped write through even though
|
|
|
+// its parent is not loaded yet.
|
|
|
+void test_validate_on_write_relations_enforced_false_is_the_escape_hatch() {
|
|
|
+ TmpEnv t("validate-enforced-off");
|
|
|
+ LmdbDocumentStore store(t.env);
|
|
|
+ // relationsEnforced=false alongside validateOnWrite=true - the exact
|
|
|
+ // combination an operator reaches for mid-bulk-import.
|
|
|
+ store.set_relations("executions",
|
|
|
+ {{"exec_wf", "workflowId", "workflows", true, 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, "relationsEnforced=false lets a validateOnWrite=true write through");
|
|
|
+ check(store.get("executions", "e1").has_value(),
|
|
|
+ "the escape-hatch write actually landed");
|
|
|
+
|
|
|
+ // Re-enabling enforcement does not retroactively touch what was already
|
|
|
+ // written (documented behaviour, mirrors canDelete's own log message) -
|
|
|
+ // but a NEW write with a missing parent is rejected again.
|
|
|
+ store.set_relations("executions",
|
|
|
+ {{"exec_wf", "workflowId", "workflows", true, true}});
|
|
|
+ Document d2; d2.id = "e2"; d2.collection = "executions";
|
|
|
+ d2.set_data({{"workflowId", "wf-ghost-2"}});
|
|
|
+ threw = false;
|
|
|
+ try {
|
|
|
+ store.put("executions", "e2", d2);
|
|
|
+ } catch (const MissingParentReference&) {
|
|
|
+ threw = true;
|
|
|
+ }
|
|
|
+ check(threw, "re-enabling enforcement rejects a new write with a missing parent");
|
|
|
+ check(store.get("executions", "e1").has_value(),
|
|
|
+ "the earlier escape-hatch write was not retroactively undone");
|
|
|
+}
|
|
|
+
|
|
|
+// v2.11.0 T13 — demonstrates validation answers against write-time state,
|
|
|
+// not a stale earlier observation.
|
|
|
+//
|
|
|
+// Models one honest slice of the 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 - in its own committed transaction - before the child's write
|
|
|
+// happens. A check that trusted the earlier observation would let the
|
|
|
+// child insert through anyway.
|
|
|
//
|
|
|
-// 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 distinguish "the mdb_get
|
|
|
+// runs inside the child's own write transaction" (the actual fix - see
|
|
|
+// maintainRelations, before the child's own mdb_put, same transaction the
|
|
|
+// caller commits) from "the mdb_get runs in a separate read transaction
|
|
|
+// opened immediately before the child's write transaction" (a narrowed
|
|
|
+// window, not a closed one). This test's sequence - get, then a
|
|
|
+// committed del, then put - would reject identically either way, because
|
|
|
+// the del is fully committed before either kind of check would run. That
|
|
|
+// placement is inside the transaction, not merely adjacent to it, is
|
|
|
+// established by reading the code (document_store_lmdb.cpp: the mdb_get is
|
|
|
+// at maintainRelations, called from put() before put()'s own mdb_put,
|
|
|
+// against the same wtxn the caller commits), not by this test.
|
|
|
//
|
|
|
-// 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.
|
|
|
+// What this test DOES show: the validation's answer tracks the parent's
|
|
|
+// state as of when the check actually runs, not whatever an earlier,
|
|
|
+// separate read happened to observe - which is the necessary condition for
|
|
|
+// the fix to work at all, even though it is not sufficient to prove
|
|
|
+// placement by itself. It is deliberately single-threaded and
|
|
|
+// deterministic, not a real multi-thread stress test: LMDB is single-writer
|
|
|
+// (see try_open_for_read's file comment and document_store_lmdb.cpp's env
|
|
|
+// setup), so under real concurrency the only thing that can vary is
|
|
|
+// transaction ORDER, never interleaving within a transaction - serialising
|
|
|
+// "the delete's transaction commits, then the child write's transaction
|
|
|
+// begins" in program order is the deterministic equivalent of that
|
|
|
+// ordering, which is as much of the race as a single-process test can
|
|
|
+// exercise.
|
|
|
void test_validate_on_write_closes_the_stale_check_race() {
|
|
|
TmpEnv t("validate-race");
|
|
|
LmdbDocumentStore store(t.env);
|
|
|
@@ -1534,6 +1656,8 @@ int main() {
|
|
|
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_rolls_back_an_already_applied_sibling_relation();
|
|
|
+ test_validate_on_write_relations_enforced_false_is_the_escape_hatch();
|
|
|
test_validate_on_write_closes_the_stale_check_race();
|
|
|
|
|
|
std::cout << "passed: " << g_pass << ", failed: " << g_fail << "\n";
|
