Skip to content

Commit 32c07f9

Browse files
committed
feat(catalog): add create-only PutCollectionIfAbsent entry variant
Adds a CatalogEntry variant that upserts a collection only when absent, threading it through apply, descriptor versioning, post-apply sync/async dispatch, gateway cache invalidation, drain lease descriptor resolution, and integrity verification. Lets CRDT sync materialize an announced collection without racing or clobbering a locally-authored definition of the same name.
1 parent 0beb9f1 commit 32c07f9

11 files changed

Lines changed: 277 additions & 0 deletions

File tree

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// SPDX-License-Identifier: BUSL-1.1
2+
//! End-to-end cluster tests for the create-only
3+
//! `CatalogEntry::PutCollectionIfAbsent` primitive.
4+
//!
5+
//! `PutCollectionIfAbsent` materializes a collection through the
6+
//! metadata raft group ONLY when no collection of the same
7+
//! `(database_id, tenant_id, name)` already exists — it never
8+
//! clobbers an existing schema. This is the durable primitive CRDT
9+
//! sync will use to announce collections without racing a
10+
//! locally-authored definition.
11+
//!
12+
//! No SQL DDL emits this variant yet, so the tests propose the entry
13+
//! directly through `metadata_proposer::propose_catalog_entry` (which
14+
//! forwards to the metadata-group leader) and assert idempotency +
15+
//! no-clobber by reading the replicated record on every node.
16+
17+
mod common;
18+
19+
use std::time::Duration;
20+
21+
use common::cluster_harness::{TestCluster, wait_for};
22+
23+
use nodedb::control::catalog_entry::CatalogEntry;
24+
use nodedb::control::metadata_proposer::propose_catalog_entry;
25+
use nodedb::control::security::catalog::StoredCollection;
26+
use nodedb_types::DatabaseId;
27+
28+
const TENANT: u64 = 1;
29+
const COLL: &str = "if_absent_coll";
30+
31+
/// Read the distinguishing fields `(bitemporal, declared_primary_key)`
32+
/// of a collection from a node's local `SystemCatalog` redb — the same
33+
/// record every node's applier writes.
34+
fn coll_fields(
35+
node: &common::cluster_harness::TestClusterNode,
36+
name: &str,
37+
) -> Option<(bool, Option<String>)> {
38+
node.shared
39+
.credentials
40+
.catalog()
41+
.as_ref()
42+
.and_then(|c| {
43+
c.get_collection(DatabaseId::DEFAULT, TENANT, name)
44+
.ok()
45+
.flatten()
46+
})
47+
.map(|c| (c.bitemporal, c.declared_primary_key))
48+
}
49+
50+
/// Propose a `PutCollectionIfAbsent` for `coll`, trying each node
51+
/// until one accepts (the proposer forwards to the metadata leader,
52+
/// so any node works — the loop mirrors `exec_ddl_on_any_leader`).
53+
fn propose_if_absent(cluster: &TestCluster, coll: StoredCollection) -> Result<(), String> {
54+
let entry = CatalogEntry::PutCollectionIfAbsent(Box::new(coll));
55+
let mut last_err = String::new();
56+
for node in &cluster.nodes {
57+
match propose_catalog_entry(&node.shared, &entry) {
58+
Ok(_) => return Ok(()),
59+
Err(e) => last_err = e.to_string(),
60+
}
61+
}
62+
Err(format!(
63+
"no node accepted PutCollectionIfAbsent: {last_err}"
64+
))
65+
}
66+
67+
#[tokio::test(flavor = "multi_thread", worker_threads = 6)]
68+
async fn put_if_absent_creates_then_no_clobbers_then_idempotent() {
69+
let cluster = TestCluster::spawn_three().await.expect("3-node cluster");
70+
71+
// A: the winning definition — bitemporal + a distinct PRIMARY KEY.
72+
let mut a = StoredCollection::new(TENANT, COLL, "tester");
73+
a.bitemporal = true;
74+
a.declared_primary_key = Some("a_key".to_string());
75+
76+
// 1. Create via PutCollectionIfAbsent (collection is absent).
77+
propose_if_absent(&cluster, a.clone()).expect("propose A");
78+
79+
// Assert A materialized on all three nodes with A's fields.
80+
wait_for(
81+
"all 3 nodes see collection with A's distinguishing fields",
82+
Duration::from_secs(10),
83+
Duration::from_millis(50),
84+
|| {
85+
cluster
86+
.nodes
87+
.iter()
88+
.all(|n| coll_fields(n, COLL) == Some((true, Some("a_key".to_string()))))
89+
},
90+
)
91+
.await;
92+
93+
// 2. Propose a DIFFERENT definition B under the same tenant+name.
94+
// Because the collection already exists, this must be a no-op.
95+
let mut b = StoredCollection::new(TENANT, COLL, "tester");
96+
b.bitemporal = false;
97+
b.declared_primary_key = Some("b_key".to_string());
98+
propose_if_absent(&cluster, b).expect("propose B");
99+
100+
// Wait for B's proposal to have applied cluster-wide, then assert
101+
// every node STILL shows A's fields — B was skipped, no clobber.
102+
wait_for(
103+
"no-clobber: every node still shows A after B proposal",
104+
Duration::from_secs(10),
105+
Duration::from_millis(50),
106+
|| {
107+
cluster
108+
.nodes
109+
.iter()
110+
.all(|n| coll_fields(n, COLL) == Some((true, Some("a_key".to_string()))))
111+
},
112+
)
113+
.await;
114+
for node in &cluster.nodes {
115+
assert_eq!(
116+
coll_fields(node, COLL),
117+
Some((true, Some("a_key".to_string()))),
118+
"B must not clobber A on node {}",
119+
node.node_id
120+
);
121+
}
122+
123+
// 3. Re-propose A verbatim — idempotent no-op. Still exactly one
124+
// collection, unchanged.
125+
propose_if_absent(&cluster, a).expect("re-propose A");
126+
wait_for(
127+
"idempotent: still exactly one collection with A's fields",
128+
Duration::from_secs(10),
129+
Duration::from_millis(50),
130+
|| {
131+
cluster.nodes.iter().all(|n| {
132+
n.cached_collection_count() == 1
133+
&& coll_fields(n, COLL) == Some((true, Some("a_key".to_string())))
134+
})
135+
},
136+
)
137+
.await;
138+
for node in &cluster.nodes {
139+
assert_eq!(
140+
node.cached_collection_count(),
141+
1,
142+
"exactly one collection on node {}",
143+
node.node_id
144+
);
145+
assert_eq!(
146+
coll_fields(node, COLL),
147+
Some((true, Some("a_key".to_string()))),
148+
"A unchanged on node {}",
149+
node.node_id
150+
);
151+
}
152+
153+
cluster.shutdown().await;
154+
}

nodedb/src/control/catalog_entry/apply/collection.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,49 @@ pub fn put(stored: &StoredCollection, catalog: &SystemCatalog) {
2626
);
2727
}
2828

29+
/// Create-only variant of [`put`]: writes the collection (and its
30+
/// owner row) exactly as `put` does, but ONLY when no collection with
31+
/// the same `(database_id, tenant_id, name)` already exists. If one is
32+
/// present, this is a no-op — the existing schema is never clobbered.
33+
///
34+
/// The existence check is the ONLY behavioral difference from `put`;
35+
/// the create path mirrors `put`'s body precisely so replay/snapshot
36+
/// re-application stays idempotent.
37+
pub fn put_if_absent(stored: &StoredCollection, catalog: &SystemCatalog) {
38+
match catalog.get_collection(stored.database_id, stored.tenant_id, &stored.name) {
39+
Ok(Some(_)) => {
40+
debug!(
41+
collection = %stored.name,
42+
tenant = stored.tenant_id,
43+
"catalog_entry: put_collection_if_absent skipped existing collection"
44+
);
45+
}
46+
Ok(None) => {
47+
if let Err(e) = catalog.put_collection(stored.database_id, stored) {
48+
warn!(
49+
collection = %stored.name,
50+
tenant = stored.tenant_id,
51+
error = %e,
52+
"catalog_entry: put_collection_if_absent put_collection failed"
53+
);
54+
}
55+
super::owner::put_parent_owner(
56+
object_type::COLLECTION,
57+
stored.tenant_id,
58+
&stored.name,
59+
&stored.owner,
60+
catalog,
61+
);
62+
}
63+
Err(e) => warn!(
64+
collection = %stored.name,
65+
tenant = stored.tenant_id,
66+
error = %e,
67+
"catalog_entry: put_collection_if_absent get failed"
68+
),
69+
}
70+
}
71+
2972
/// Hard-delete the catalog metadata for a collection: primary
3073
/// `StoredCollection` row, owner row, and surrogate ↔ PK map.
3174
///

nodedb/src/control/catalog_entry/apply/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ pub fn apply_to(entry: &CatalogEntry, catalog: &SystemCatalog) {
7777
fn apply_to_inner(entry: &CatalogEntry, catalog: &SystemCatalog) {
7878
match entry {
7979
CatalogEntry::PutCollection(stored) => collection::put(stored, catalog),
80+
CatalogEntry::PutCollectionIfAbsent(stored) => collection::put_if_absent(stored, catalog),
8081
CatalogEntry::DeactivateCollection { tenant_id, name } => {
8182
collection::deactivate(*tenant_id, name, catalog)
8283
}

nodedb/src/control/catalog_entry/descriptor_stamp.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,33 @@ pub fn stamp(entry: CatalogEntry, clock: &HlcClock, catalog: &SystemCatalog) ->
7878
stored.modification_hlc = hlc;
7979
CatalogEntry::PutCollection(stored)
8080
}
81+
CatalogEntry::PutCollectionIfAbsent(mut stored) => {
82+
let prior = catalog
83+
.get_collection(stored.database_id, stored.tenant_id, &stored.name)
84+
.ok()
85+
.flatten();
86+
let prior_descriptor = prior.as_ref().map(|c| c.descriptor_version).unwrap_or(0);
87+
stored.descriptor_version = prior_descriptor.saturating_add(1);
88+
// Constraint version bumps ONLY when the derived constraint set
89+
// actually changes, so an unrelated ALTER never advances the
90+
// apply-time fence key and never transiently rejects in-flight
91+
// CRDT deltas. `Constraint: Eq` + name-sorted translator make the
92+
// set comparison exact and order-stable.
93+
let prior_constraint_version =
94+
prior.as_ref().map(|c| c.constraint_version).unwrap_or(0);
95+
let prior_set = prior
96+
.as_ref()
97+
.map(crate::control::security::catalog::collection_constraints)
98+
.unwrap_or_default();
99+
let new_set = crate::control::security::catalog::collection_constraints(&stored);
100+
stored.constraint_version = if new_set != prior_set {
101+
prior_constraint_version.saturating_add(1)
102+
} else {
103+
prior_constraint_version
104+
};
105+
stored.modification_hlc = hlc;
106+
CatalogEntry::PutCollectionIfAbsent(stored)
107+
}
81108
CatalogEntry::PutMaterializedView(mut stored) => {
82109
let prior = catalog
83110
.get_materialized_view(stored.tenant_id, &stored.name)

nodedb/src/control/catalog_entry/entry.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@ pub enum CatalogEntry {
3030
/// (strict schema changes, retention / legal_hold / LVC /
3131
/// append_only toggles, materialized_sum bindings).
3232
PutCollection(Box<StoredCollection>),
33+
/// Create-only collection upsert: applies iff the collection is
34+
/// absent, and never clobbers an existing schema. Used by CRDT
35+
/// sync to materialize announced collections without racing or
36+
/// overwriting a locally-authored definition of the same name.
37+
PutCollectionIfAbsent(Box<StoredCollection>),
3338
/// Mark a collection as `is_active = false`. Record is
3439
/// preserved for audit + undrop. The soft-delete step in the
3540
/// two-step DROP → retention-expiry → PURGE flow.
@@ -332,6 +337,7 @@ impl CatalogEntry {
332337
pub fn kind(&self) -> &'static str {
333338
match self {
334339
Self::PutCollection(_) => "put_collection",
340+
Self::PutCollectionIfAbsent(_) => "put_collection_if_absent",
335341
Self::DeactivateCollection { .. } => "deactivate_collection",
336342
Self::PurgeCollection { .. } => "purge_collection",
337343
Self::PutSequence(_) => "put_sequence",

nodedb/src/control/catalog_entry/post_apply/async_dispatch/dispatcher.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,17 @@ pub fn spawn_post_apply_async_side_effects(
4949
});
5050
});
5151
}
52+
CatalogEntry::PutCollectionIfAbsent(stored) => {
53+
// SYNCHRONOUS: Register must complete before the applied-index
54+
// watcher bumps so any subsequent scan on this node finds the
55+
// collection in doc_configs. block_in_place is valid because
56+
// the raft tick loop runs on a tokio worker thread.
57+
tokio::task::block_in_place(|| {
58+
tokio::runtime::Handle::current().block_on(async move {
59+
collection::put_async(*stored, shared).await;
60+
});
61+
});
62+
}
5263
CatalogEntry::PurgeCollection { tenant_id, name } => {
5364
tokio::spawn(async move {
5465
collection::purge_async(tenant_id, name, raft_index, shared).await;

nodedb/src/control/catalog_entry/post_apply/gateway_invalidation.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ pub(crate) fn invalidate_gateway_cache_for_entry(entry: &CatalogEntry, shared: &
5050
CatalogEntry::PutCollection(stored) => {
5151
inv.invalidate(&stored.name, stored.descriptor_version.max(1));
5252
}
53+
CatalogEntry::PutCollectionIfAbsent(stored) => {
54+
inv.invalidate(&stored.name, stored.descriptor_version.max(1));
55+
}
5356
CatalogEntry::DeactivateCollection { name, .. } => {
5457
// Treat deactivation as version 0 (collection gone — any cached
5558
// plan for it is stale).

nodedb/src/control/catalog_entry/post_apply/sync.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,11 @@ pub fn apply_post_apply_side_effects_sync(entry: &CatalogEntry, shared: &Arc<Sha
4444
// the async part, handled by `spawn_post_apply_async_side_effects`.
4545
collection::put_owner_sync(stored, Arc::clone(shared));
4646
}
47+
CatalogEntry::PutCollectionIfAbsent(stored) => {
48+
// Owner record install is sync; Data Plane register is
49+
// the async part, handled by `spawn_post_apply_async_side_effects`.
50+
collection::put_owner_sync(stored, Arc::clone(shared));
51+
}
4752
CatalogEntry::DeactivateCollection { tenant_id, name } => {
4853
collection::deactivate(*tenant_id, name.clone(), Arc::clone(shared));
4954
}

nodedb/src/control/cluster/metadata_applier_audit.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,11 @@ pub(super) fn describe_entry(e: &catalog_entry::CatalogEntry) -> (String, u64, S
138138
c.descriptor_version,
139139
format!("{:?}", c.modification_hlc),
140140
),
141+
E::PutCollectionIfAbsent(c) => (
142+
c.name.clone(),
143+
c.descriptor_version,
144+
format!("{:?}", c.modification_hlc),
145+
),
141146
E::DeactivateCollection { name, .. } => (name.clone(), 0, String::new()),
142147
E::PurgeCollection { name, .. } => (name.clone(), 0, String::new()),
143148
E::RecordWalTombstone { collection, .. } => (collection.clone(), 0, String::new()),

nodedb/src/control/lease/drain_propose.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,11 @@ pub fn descriptor_id_for_implicit_clear(entry: &CatalogEntry) -> Option<Descript
256256
DescriptorKind::Collection,
257257
stored.name.clone(),
258258
)),
259+
CatalogEntry::PutCollectionIfAbsent(stored) => Some(DescriptorId::new(
260+
stored.tenant_id,
261+
DescriptorKind::Collection,
262+
stored.name.clone(),
263+
)),
259264
CatalogEntry::PutMaterializedView(stored) => Some(DescriptorId::new(
260265
stored.tenant_id,
261266
DescriptorKind::MaterializedView,
@@ -319,6 +324,22 @@ pub fn descriptor_id_and_prior_version(
319324
prior,
320325
))
321326
}
327+
CatalogEntry::PutCollectionIfAbsent(stored) => {
328+
let prior = catalog
329+
.get_collection(DatabaseId::DEFAULT, stored.tenant_id, &stored.name)
330+
.ok()
331+
.flatten()
332+
.map(|c| c.descriptor_version)
333+
.unwrap_or(0);
334+
Some((
335+
DescriptorId::new(
336+
stored.tenant_id,
337+
DescriptorKind::Collection,
338+
stored.name.clone(),
339+
),
340+
prior,
341+
))
342+
}
322343
CatalogEntry::PutMaterializedView(stored) => {
323344
let prior = catalog
324345
.get_materialized_view(stored.tenant_id, &stored.name)

0 commit comments

Comments
 (0)