Skip to content

Commit 2231164

Browse files
[fm] Rename SitrepGuardedResource to FmRendezvousResource and extract to a shared module (#10721)
Both the sitrep generation-guarded insert (`SitrepGuardedInsert`) and the upcoming creation-marker GC sweep need essentially the same abstraction to capture "Diesel schema types I need for my generic query." This currently comprises: * The `fm_sitrep` table's generation column for a given resource (e.g. `fm_sitrep::alert_generation` for `Alert`), * The marker table's id column for a given resource (e.g. `rendezvous_alert_created::alert_id` for `Alert`), * A couple annoying Diesel trait bounds. When we implement GC for the creation markers, this will also include the table for the resource creation request (e.g. `fm_alert_request` for `Alert`) and one more annoying Diesel trait bound. So this PR renames the `SitrepGuardedResource` trait to the slightly more palatable `FmRendezvousResource` and extracts it to a new module so both the guarded-insert CTE and the GC sweep can share it. It also extracts the dummy resource used for testing. This is a pure refactor with no behavioral change. Context: #10248.
1 parent c54aeae commit 2231164

3 files changed

Lines changed: 239 additions & 143 deletions

File tree

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
// This Source Code Form is subject to the terms of the Mozilla Public
2+
// License, v. 2.0. If a copy of the MPL was not distributed with this
3+
// file, You can obtain one at https://mozilla.org/MPL/2.0/.
4+
5+
//! Resources created by FM rendezvous, described in terms of their Diesel
6+
//! schema types.
7+
//!
8+
//! When executing a sitrep, FM rendezvous can create resources (currently
9+
//! alerts and support bundles) in the database. To prevent wrongly creating
10+
//! resources that have already been deleted by the user/operator, inserts are
11+
//! guarded by a "creation marker" and a CTE that understands the semantics of
12+
//! that marker (see [`crate::db::sitrep_guard`]).
13+
//!
14+
//! The [`FmRendezvousResource`] trait gathers the Diesel schema types that the
15+
//! guard CTE needs for a given resource. Implementations are provided for the
16+
//! two supported resource types, `Alert` and `SupportBundle`.
17+
18+
use diesel::Column;
19+
use diesel::associations::HasTable;
20+
use diesel::pg::Pg;
21+
use diesel::query_builder::QueryFragment;
22+
use diesel::query_source::QuerySource;
23+
use nexus_db_schema::schema;
24+
25+
/// The creation marker table corresponding to some [`FmRendezvousResource`]
26+
/// `R`. This is the table that defines the column referenced by the
27+
/// [`FmRendezvousResource::IdColumn`] associated type.
28+
pub type MarkerTable<R> =
29+
<<R as FmRendezvousResource>::IdColumn as Column>::Table;
30+
31+
/// A resource created by FM rendezvous, comprising the Diesel schema types
32+
/// required to generically construct useful queries / CTEs that operate on that
33+
/// resource.
34+
///
35+
/// Each resource type is backed by three pieces of schema:
36+
///
37+
/// - The resource's own table (e.g. `alert`, `support_bundle`), holding the
38+
/// resource rows themselves. This trait doesn't name that table; queries
39+
/// that create the resource take a caller-built `INSERT` into it as input.
40+
///
41+
/// - A per-resource-type generation column on the `fm_sitrep` table
42+
/// ([`Self::GenerationColumn`], e.g. `fm_sitrep.alert_generation`), bumped
43+
/// each time a sitrep's request set for this resource type changes.
44+
/// Comparing the executing sitrep's value against the latest sitrep's
45+
/// detects a rendezvous task executing a stale sitrep.
46+
///
47+
/// - A creation marker table (e.g. `rendezvous_alert_created`), recording
48+
/// each resource that FM rendezvous has ever created: the resource's id
49+
/// ([`Self::IdColumn`], the marker table's primary key) and the generation
50+
/// at which it was created. The marker is written atomically with the
51+
/// resource row and outlives it: if the resource is deleted while some
52+
/// sitrep still requests it, the marker's presence prevents the resource
53+
/// from being re-created.
54+
///
55+
/// The common required trait bounds for building these queries are captured
56+
/// here, in the trait's `where` clause, so we don't have to repeat them at
57+
/// every use site.
58+
pub trait FmRendezvousResource
59+
where
60+
// `MarkerTable<R>` is a `Table` (see above), but disappointingly, `Table`
61+
// isn't `HasTable`! We need this bound to be able to call
62+
// `MarkerTable::table()`...
63+
MarkerTable<Self>: HasTable<Table = MarkerTable<Self>>,
64+
// ... Furthermore, we need to use the marker table in a `FROM` clause, so
65+
// it must be `QuerySource`. To be spliced into the query by `walk_ast`, the
66+
// `FROM` clause must also be `QueryFragment<Pg>`. And finally, the built
67+
// query is held across `.await`, so it must be `Send`.
68+
<MarkerTable<Self> as QuerySource>::FromClause: QueryFragment<Pg> + Send,
69+
{
70+
/// The column on `fm_sitrep` carrying this resource's generation counter
71+
/// (e.g. `fm_sitrep::alert_generation` for `Alert`). Read by
72+
/// [`SitrepGuardedInsert`] to guard against stale execution.
73+
///
74+
/// [`SitrepGuardedInsert`]: crate::db::sitrep_guard::SitrepGuardedInsert
75+
type GenerationColumn: Column<Table = schema::fm_sitrep::table>;
76+
77+
/// The id column in the creation marker table
78+
/// (e.g. `rendezvous_alert_created::alert_id` for `Alert`).
79+
type IdColumn: Column;
80+
}
81+
82+
impl FmRendezvousResource for nexus_db_model::Alert {
83+
type GenerationColumn = schema::fm_sitrep::dsl::alert_generation;
84+
type IdColumn = schema::rendezvous_alert_created::dsl::alert_id;
85+
}
86+
87+
impl FmRendezvousResource for nexus_db_model::SupportBundle {
88+
type GenerationColumn = schema::fm_sitrep::dsl::support_bundle_generation;
89+
type IdColumn =
90+
schema::rendezvous_support_bundle_created::dsl::support_bundle_id;
91+
}
92+
93+
/// Test fixtures for the insert (`sitrep_guard`) test suite: a synthetic
94+
/// [`FmRendezvousResource`] and the throwaway schema its query references.
95+
///
96+
/// The dummy schema lets the suite exercise the generic, typed query path
97+
/// against a stand-in resource, decoupled from the specifics of any real
98+
/// resource.
99+
#[cfg(test)]
100+
pub(crate) mod test_utils {
101+
use super::FmRendezvousResource;
102+
use async_bb8_diesel::AsyncRunQueryDsl;
103+
use async_bb8_diesel::AsyncSimpleConnection;
104+
use diesel::prelude::*;
105+
use nexus_db_lookup::DbConnection;
106+
use uuid::Uuid;
107+
108+
// Schema for a dummy resource and its creation marker.
109+
table! {
110+
test_schema.dummy_resource (id) {
111+
id -> Uuid,
112+
name -> Text,
113+
}
114+
}
115+
table! {
116+
test_schema.dummy_marker (dummy_id) {
117+
dummy_id -> Uuid,
118+
created_at_generation -> Int8,
119+
}
120+
}
121+
122+
// The dummy resource's generation column lives on the *real* `fm_sitrep`
123+
// table, not a dummy table. But of course the real `fm_sitrep`'s `table!`
124+
// doesn't declare it, so we have to manually define a Diesel `Column` for
125+
// it, equivalent to what `table!` would have generated.
126+
#[allow(non_camel_case_types)]
127+
#[derive(Debug, Clone, Copy)]
128+
pub(crate) struct dummy_generation;
129+
130+
impl diesel::Expression for dummy_generation {
131+
type SqlType = diesel::sql_types::BigInt;
132+
}
133+
134+
impl diesel::Column for dummy_generation {
135+
type Table = nexus_db_schema::schema::fm_sitrep::table;
136+
const NAME: &'static str = "dummy_generation";
137+
}
138+
139+
// Model for the dummy resource.
140+
#[derive(Queryable, Selectable, Debug)]
141+
#[diesel(table_name = dummy_resource)]
142+
pub(crate) struct DummyResource {
143+
pub id: Uuid,
144+
pub name: String,
145+
}
146+
147+
impl FmRendezvousResource for DummyResource {
148+
type GenerationColumn = dummy_generation;
149+
type IdColumn = dummy_marker::dsl::dummy_id;
150+
}
151+
152+
/// Creates the tables the guarded-insert SQL references but which the real
153+
/// schema doesn't have: the dummy resource and marker tables, plus a
154+
/// `dummy_generation` column on the real `fm_sitrep` table.
155+
///
156+
/// Mutating the real `fm_sitrep` is safe because each test runs against its
157+
/// own fresh `TestDatabase`; the added column never escapes into production
158+
/// or another test.
159+
pub(crate) async fn setup_dummy_schema(
160+
conn: &async_bb8_diesel::Connection<DbConnection>,
161+
) {
162+
conn.batch_execute_async(
163+
"
164+
CREATE SCHEMA IF NOT EXISTS test_schema; \
165+
CREATE TABLE IF NOT EXISTS test_schema.dummy_resource ( \
166+
id UUID PRIMARY KEY, \
167+
name TEXT NOT NULL \
168+
); \
169+
CREATE TABLE IF NOT EXISTS test_schema.dummy_marker ( \
170+
dummy_id UUID PRIMARY KEY, \
171+
created_at_generation INT8 NOT NULL \
172+
); \
173+
ALTER TABLE omicron.public.fm_sitrep \
174+
ADD COLUMN IF NOT EXISTS dummy_generation INT8;",
175+
)
176+
.await
177+
.unwrap();
178+
}
179+
180+
/// Insert a row into the dummy marker table at the given
181+
/// `created_at_generation`.
182+
pub(crate) async fn insert_dummy_marker(
183+
conn: &async_bb8_diesel::Connection<DbConnection>,
184+
id: Uuid,
185+
created_at_generation: i64,
186+
) {
187+
diesel::insert_into(dummy_marker::table)
188+
.values((
189+
dummy_marker::dsl::dummy_id.eq(id),
190+
dummy_marker::dsl::created_at_generation
191+
.eq(created_at_generation),
192+
))
193+
.execute_async(conn)
194+
.await
195+
.unwrap();
196+
}
197+
198+
/// The `created_at_generation` recorded in the marker row for `id`, if
199+
/// any (`None` means no marker row exists).
200+
pub(crate) async fn marker_generation(
201+
conn: &async_bb8_diesel::Connection<DbConnection>,
202+
id: Uuid,
203+
) -> Option<i64> {
204+
dummy_marker::table
205+
.filter(dummy_marker::dsl::dummy_id.eq(id))
206+
.select(dummy_marker::dsl::created_at_generation)
207+
.first_async::<i64>(conn)
208+
.await
209+
.optional()
210+
.unwrap()
211+
}
212+
}

nexus/db-queries/src/db/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ mod cte_utils;
1616
// This is marked public for use by the integration tests
1717
pub mod datastore;
1818
mod explain;
19+
pub(crate) mod fm_rendezvous_resources;
1920
mod on_conflict_ext;
2021
// Public for doctests.
2122
pub mod pagination;

0 commit comments

Comments
 (0)