Skip to content

Commit dbc7b8e

Browse files
authored
Fix leaked bp_oximeter_read_policy rows (#8577)
Closes #8437
1 parent 509eab5 commit dbc7b8e

File tree

4 files changed

+71
-2
lines changed

4 files changed

+71
-2
lines changed

nexus/db-model/src/schema_versions.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock};
1616
///
1717
/// This must be updated when you change the database schema. Refer to
1818
/// schema/crdb/README.adoc in the root of this repository for details.
19-
pub const SCHEMA_VERSION: Version = Version::new(163, 0, 0);
19+
pub const SCHEMA_VERSION: Version = Version::new(164, 0, 0);
2020

2121
/// List of all past database schema versions, in *reverse* order
2222
///
@@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock<Vec<KnownVersion>> = LazyLock::new(|| {
2828
// | leaving the first copy as an example for the next person.
2929
// v
3030
// KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
31+
KnownVersion::new(164, "fix-leaked-bp-oximeter-read-policy-rows"),
3132
KnownVersion::new(163, "bp-desired-host-phase-2"),
3233
KnownVersion::new(162, "bundle-by-creation"),
3334
KnownVersion::new(161, "inv_cockroachdb_status"),

nexus/tests/integration_tests/schema.rs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2650,6 +2650,67 @@ mod migration_156 {
26502650
}
26512651
}
26522652

2653+
const BP_OXIMETER_READ_POLICY_ID_0: &str =
2654+
"5cb42909-d94a-4903-be72-330eea0325d9";
2655+
const BP_OXIMETER_READ_POLICY_ID_1: &str =
2656+
"142b62c2-9348-4530-9eed-7077351fb94b";
2657+
const BP_OXIMETER_READ_POLICY_ID_2: &str =
2658+
"3b5b7861-03aa-420a-a057-0a14347dc4c0";
2659+
const BP_OXIMETER_READ_POLICY_ID_3: &str =
2660+
"de7ab4c0-30d4-4e9e-b620-3a959a9d59dd";
2661+
2662+
// Insert two blueprints and 4 oximeter read policies, two of which do not have
2663+
// a corresponding blueprint
2664+
fn before_164_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> {
2665+
Box::pin(async move {
2666+
ctx.client
2667+
.batch_execute(
2668+
&format!("
2669+
INSERT INTO omicron.public.blueprint
2670+
(id, parent_blueprint_id, time_created, creator, comment, internal_dns_version, external_dns_version, cockroachdb_fingerprint, cockroachdb_setting_preserve_downgrade, target_release_minimum_generation)
2671+
VALUES
2672+
(
2673+
'{BP_OXIMETER_READ_POLICY_ID_0}', NULL, now(), 'bob', 'hi', 1, 1, 'fingerprint', NULL, 1
2674+
),
2675+
(
2676+
'{BP_OXIMETER_READ_POLICY_ID_1}', NULL, now(), 'bab', 'hi', 1, 1, 'fingerprint', NULL, 1
2677+
);
2678+
2679+
INSERT INTO omicron.public.bp_oximeter_read_policy
2680+
(blueprint_id, version, oximeter_read_mode)
2681+
VALUES
2682+
('{BP_OXIMETER_READ_POLICY_ID_0}', 1, 'cluster'),
2683+
('{BP_OXIMETER_READ_POLICY_ID_1}', 2, 'cluster'),
2684+
('{BP_OXIMETER_READ_POLICY_ID_2}', 3, 'cluster'),
2685+
('{BP_OXIMETER_READ_POLICY_ID_3}', 4, 'cluster')
2686+
"),
2687+
)
2688+
.await
2689+
.expect("failed to insert pre-migration rows for 163");
2690+
})
2691+
}
2692+
2693+
// Validate that rows that do not have a corresponding blueprint are gone
2694+
fn after_164_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> {
2695+
Box::pin(async move {
2696+
let rows = ctx
2697+
.client
2698+
.query(
2699+
"SELECT blueprint_id FROM omicron.public.bp_oximeter_read_policy ORDER BY blueprint_id;",
2700+
&[],
2701+
)
2702+
.await
2703+
.expect("failed to query post-migration bp_oximeter_read_policy table");
2704+
assert_eq!(rows.len(), 2);
2705+
2706+
let id_1: Uuid = (&rows[0]).get::<&str, Uuid>("blueprint_id");
2707+
assert_eq!(id_1.to_string(), BP_OXIMETER_READ_POLICY_ID_1);
2708+
2709+
let id_2: Uuid = (&rows[1]).get::<&str, Uuid>("blueprint_id");
2710+
assert_eq!(id_2.to_string(), BP_OXIMETER_READ_POLICY_ID_0);
2711+
})
2712+
}
2713+
26532714
// Lazily initializes all migration checks. The combination of Rust function
26542715
// pointers and async makes defining a static table fairly painful, so we're
26552716
// using lazy initialization instead.
@@ -2736,6 +2797,10 @@ fn get_migration_checks() -> BTreeMap<Version, DataMigrationFns> {
27362797
.before(migration_156::before)
27372798
.after(migration_156::after),
27382799
);
2800+
map.insert(
2801+
Version::new(164, 0, 0),
2802+
DataMigrationFns::new().before(before_164_0_0).after(after_164_0_0),
2803+
);
27392804

27402805
map
27412806
}

schema/crdb/dbinit.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6209,7 +6209,7 @@ INSERT INTO omicron.public.db_metadata (
62096209
version,
62106210
target_version
62116211
) VALUES
6212-
(TRUE, NOW(), NOW(), '163.0.0', NULL)
6212+
(TRUE, NOW(), NOW(), '164.0.0', NULL)
62136213
ON CONFLICT DO NOTHING;
62146214

62156215
COMMIT;
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
set local disallow_full_table_scans = off;
2+
3+
DELETE FROM bp_oximeter_read_policy WHERE blueprint_id NOT IN (SELECT id FROM blueprint);

0 commit comments

Comments
 (0)