[nexus] garbage collect orphaned FM sitreps (#9335)

When a Nexus attempts to commit a new fault management situation report
to the sitrep history but fails to do so because another sitrep with the
same parent has already been inserted, that sitrep is said to be
_orphaned_. Records pertaining to it are left behind in the database,
but it will not be accessed by the rest of the system. Thus, we must
occasionally garbage-collect such sitreps. This branch adds a background
task for doing so.

Depends on #9320

Co-authored-by: Sean Klein <sean@oxide.computer>

Author: hawkw

dev-tools/omdb/src/bin/omdb/db.rs

@@ -1309,7 +1309,7 @@ impl DbArgs {
                         sitrep::cmd_db_sitrep(&opctx, &datastore, &fetch_opts, args).await
                     }
                     DbCommands::Sitreps(args) => {
-                        sitrep::cmd_db_sitrep_history(&datastore, &fetch_opts, args).await
+                        sitrep::cmd_db_sitrep_history(&opctx, &datastore, &fetch_opts, args).await
                     }
                     DbCommands::Sleds(args) => {
                         cmd_db_sleds(&opctx, &datastore, &fetch_opts, args).await

dev-tools/omdb/src/bin/omdb/db/sitrep.rs

@@ -7,7 +7,7 @@
 use crate::db::DbFetchOptions;
 use crate::db::check_limit;
 use crate::helpers::const_max_len;
-use crate::helpers::datetime_rfc3339_concise;
+use crate::helpers::datetime_opt_rfc3339_concise;
 use anyhow::Context;
 use async_bb8_diesel::AsyncRunQueryDsl;
 use chrono::{DateTime, Utc};
@@ -17,7 +17,6 @@ use diesel::prelude::*;
 use nexus_db_queries::context::OpContext;
 use nexus_db_queries::db::DataStore;
 use nexus_db_queries::db::model;
-use nexus_db_queries::db::pagination::paginated;
 use nexus_types::fm;
 use omicron_common::api::external::DataPageParams;
 use omicron_common::api::external::PaginationOrder;
@@ -26,7 +25,6 @@ use omicron_uuid_kinds::SitrepUuid;
 use tabled::Tabled;
 use uuid::Uuid;
 
-use nexus_db_schema::schema::fm_sitrep::dsl as sitrep_dsl;
 use nexus_db_schema::schema::fm_sitrep_history::dsl as history_dsl;
 use nexus_db_schema::schema::inv_collection::dsl as inv_collection_dsl;
 
@@ -100,7 +98,7 @@ pub(super) async fn cmd_db_sitrep(
 ) -> anyhow::Result<()> {
     match args.command {
         Commands::History(ref args) => {
-            cmd_db_sitrep_history(datastore, fetch_opts, args).await
+            cmd_db_sitrep_history(opctx, datastore, fetch_opts, args).await
         }
         Commands::Info { sitrep, ref args } => {
             cmd_db_sitrep_show(opctx, datastore, fetch_opts, args, sitrep).await
@@ -119,6 +117,7 @@ pub(super) async fn cmd_db_sitrep(
 }
 
 pub(super) async fn cmd_db_sitrep_history(
+    opctx: &OpContext,
     datastore: &DataStore,
     fetch_opts: &DbFetchOptions,
     args: &SitrepHistoryArgs,
@@ -138,53 +137,48 @@ pub(super) async fn cmd_db_sitrep_history(
     struct SitrepRow {
         v: u32,
         id: Uuid,
-        #[tabled(display_with = "datetime_rfc3339_concise")]
-        created_at: DateTime<Utc>,
+        #[tabled(display_with = "datetime_opt_rfc3339_concise")]
+        created_at: Option<DateTime<Utc>>,
         comment: String,
     }
 
-    let conn = datastore.pool_connection_for_tests().await?;
     let marker = args.from.map(model::SqlU32::new);
     let pagparams = DataPageParams {
         marker: marker.as_ref(),
         direction: PaginationOrder::Descending,
         limit: fetch_opts.fetch_limit,
     };
-    let sitreps: Vec<(model::SitrepVersion, model::SitrepMetadata)> =
-        paginated(
-            history_dsl::fm_sitrep_history,
-            history_dsl::version,
-            &pagparams,
-        )
-        .inner_join(
-            sitrep_dsl::fm_sitrep.on(history_dsl::sitrep_id.eq(sitrep_dsl::id)),
-        )
-        .select((
-            model::SitrepVersion::as_select(),
-            model::SitrepMetadata::as_select(),
-        ))
-        .load_async(&*conn)
+    let versions = datastore
+        .fm_sitrep_version_list(&opctx, &pagparams)
         .await
         .with_context(ctx)?;
 
-    check_limit(&sitreps, fetch_opts.fetch_limit, ctx);
-
-    let rows = sitreps.into_iter().map(|(version, metadata)| {
-        let model::SitrepMetadata {
-            id,
-            time_created,
-            comment,
-            creator_id: _,
-            parent_sitrep_id: _,
-            inv_collection_id: _,
-        } = metadata;
-        SitrepRow {
-            v: version.version.into(),
-            id: id.into_untyped_uuid(),
+    check_limit(&versions, fetch_opts.fetch_limit, ctx);
+
+    let mut rows = Vec::with_capacity(versions.len());
+    for v in versions {
+        let (comment, time_created) =
+            match datastore.fm_sitrep_metadata_read(&opctx, v.id).await {
+                Ok(s) => (s.comment, Some(s.time_created)),
+                Err(e) => {
+                    // If the sitrep has an entry in the history table, we
+                    // expect that it will not yet have been archived and
+                    // deleted, so this is an error rather a case of it just
+                    // no longer existing.
+                    eprintln!(
+                        "failed to fetch metadata for sitrep {} (v{}): {e}",
+                        v.id, v.version
+                    );
+                    ("<ERROR>".to_string(), None)
+                }
+            };
+        rows.push(SitrepRow {
+            v: v.version,
+            id: v.id.into_untyped_uuid(),
             created_at: time_created,
             comment,
-        }
-    });
+        });
+    }
 
     let table = tabled::Table::new(rows)
         .with(tabled::settings::Style::empty())

dev-tools/omdb/src/bin/omdb/nexus.rs

@@ -67,6 +67,7 @@ use nexus_types::internal_api::background::RegionSnapshotReplacementFinishStatus
 use nexus_types::internal_api::background::RegionSnapshotReplacementGarbageCollectStatus;
 use nexus_types::internal_api::background::RegionSnapshotReplacementStartStatus;
 use nexus_types::internal_api::background::RegionSnapshotReplacementStepStatus;
+use nexus_types::internal_api::background::SitrepGcStatus;
 use nexus_types::internal_api::background::SitrepLoadStatus;
 use nexus_types::internal_api::background::SupportBundleCleanupReport;
 use nexus_types::internal_api::background::SupportBundleCollectionReport;
@@ -1240,6 +1241,9 @@ fn print_task_details(bgtask: &BackgroundTask, details: &serde_json::Value) {
         "fm_sitrep_loader" => {
             print_task_fm_sitrep_loader(details);
         }
+        "fm_sitrep_gc" => {
+            print_task_fm_sitrep_gc(details);
+        }
         _ => {
             println!(
                 "warning: unknown background task: {:?} \
@@ -3142,6 +3146,41 @@ fn print_task_fm_sitrep_loader(details: &serde_json::Value) {
     };
 }
 
+fn print_task_fm_sitrep_gc(details: &serde_json::Value) {
+    let SitrepGcStatus {
+        orphaned_sitreps_found,
+        orphaned_sitreps_deleted,
+        errors,
+    } = match serde_json::from_value::<SitrepGcStatus>(details.clone()) {
+        Err(error) => {
+            eprintln!(
+                "warning: failed to interpret task details: {:?}: {:?}",
+                error, details
+            );
+            return;
+        }
+        Ok(status) => status,
+    };
+
+    pub const ORPHANS_FOUND: &str = "orphaned sitreps found:";
+    pub const ORPHANS_DELETED: &str = "orphaned sitreps deleted:";
+    pub const ERRORS: &str = "errors:";
+    pub const WIDTH: usize =
+        const_max_len(&[ERRORS, ORPHANS_FOUND, ORPHANS_DELETED]) + 1;
+    pub const NUM_WIDTH: usize = 4;
+    if !errors.is_empty() {
+        println!("{ERRICON}   {ERRORS:<WIDTH$}{:>NUM_WIDTH$}", errors.len());
+        for error in errors {
+            println!("      > {error}")
+        }
+    }
+
+    println!("    {ORPHANS_FOUND:<WIDTH$}{orphaned_sitreps_found:>NUM_WIDTH$}");
+    println!(
+        "    {ORPHANS_DELETED:<WIDTH$}{orphaned_sitreps_deleted:>NUM_WIDTH$}"
+    );
+}
+
 const ERRICON: &str = "/!\\";
 
 fn warn_if_nonzero(n: usize) -> &'static str {

dev-tools/omdb/tests/env.out

@@ -99,6 +99,10 @@ task: "external_endpoints"
     on each one
 
 
+task: "fm_sitrep_gc"
+    garbage collects fault management situation reports
+
+
 task: "fm_sitrep_loader"
     loads the current fault management situation report from the database
 
@@ -319,6 +323,10 @@ task: "external_endpoints"
     on each one
 
 
+task: "fm_sitrep_gc"
+    garbage collects fault management situation reports
+
+
 task: "fm_sitrep_loader"
     loads the current fault management situation report from the database
 
@@ -526,6 +534,10 @@ task: "external_endpoints"
     on each one
 
 
+task: "fm_sitrep_gc"
+    garbage collects fault management situation reports
+
+
 task: "fm_sitrep_loader"
     loads the current fault management situation report from the database
 

dev-tools/omdb/tests/successes.out

@@ -334,6 +334,10 @@ task: "external_endpoints"
     on each one
 
 
+task: "fm_sitrep_gc"
+    garbage collects fault management situation reports
+
+
 task: "fm_sitrep_loader"
     loads the current fault management situation report from the database
 
@@ -619,6 +623,13 @@ task: "external_endpoints"
 
     TLS certificates: 0
 
+task: "fm_sitrep_gc"
+  configured period: every <REDACTED_DURATION>s
+  last completed activation: <REDACTED ITERATIONS>, triggered by <TRIGGERED_BY_REDACTED>
+    started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
+    orphaned sitreps found:      0
+    orphaned sitreps deleted:    0
+
 task: "fm_sitrep_loader"
   configured period: every <REDACTED_DURATION>s
   last completed activation: <REDACTED ITERATIONS>, triggered by <TRIGGERED_BY_REDACTED>
@@ -1166,6 +1177,13 @@ task: "external_endpoints"
 
     TLS certificates: 0
 
+task: "fm_sitrep_gc"
+  configured period: every <REDACTED_DURATION>s
+  last completed activation: <REDACTED ITERATIONS>, triggered by <TRIGGERED_BY_REDACTED>
+    started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
+    orphaned sitreps found:      0
+    orphaned sitreps deleted:    0
+
 task: "fm_sitrep_loader"
   configured period: every <REDACTED_DURATION>s
   last completed activation: <REDACTED ITERATIONS>, triggered by <TRIGGERED_BY_REDACTED>

nexus-config/src/nexus_config.rs

@@ -879,11 +879,21 @@ pub struct FmTasksConfig {
     /// reads the latest fault management sitrep from the database.
     #[serde_as(as = "DurationSeconds<u64>")]
     pub sitrep_load_period_secs: Duration,
+    /// period (in seconds) for periodic activations of the background task that
+    /// garbage collects unneeded fault management sitreps in the database.
+    #[serde_as(as = "DurationSeconds<u64>")]
+    pub sitrep_gc_period_secs: Duration,
 }
 
 impl Default for FmTasksConfig {
     fn default() -> Self {
-        Self { sitrep_load_period_secs: Duration::from_secs(15) }
+        Self {
+            sitrep_load_period_secs: Duration::from_secs(15),
+            // This need not be activated very frequently, as it's triggered any
+            // time the current sitrep changes, and activating it more
+            // frequently won't make things more responsive.
+            sitrep_gc_period_secs: Duration::from_secs(600),
+        }
     }
 }
 
@@ -1190,6 +1200,7 @@ mod test {
             webhook_deliverator.second_retry_backoff_secs = 46
             sp_ereport_ingester.period_secs = 47
             fm.sitrep_load_period_secs = 48
+            fm.sitrep_gc_period_secs = 49
             [default_region_allocation_strategy]
             type = "random"
             seed = 0
@@ -1436,6 +1447,7 @@ mod test {
                         },
                         fm: FmTasksConfig {
                             sitrep_load_period_secs: Duration::from_secs(48),
+                            sitrep_gc_period_secs: Duration::from_secs(49),
                         }
                     },
                     default_region_allocation_strategy:
@@ -1536,6 +1548,7 @@ mod test {
             webhook_deliverator.period_secs = 43
             sp_ereport_ingester.period_secs = 44
             fm.sitrep_load_period_secs = 45
+            fm.sitrep_gc_period_secs = 46
 
             [default_region_allocation_strategy]
             type = "random"

nexus/background-task-interface/src/init.rs

@@ -52,6 +52,7 @@ pub struct BackgroundTasks {
     pub task_sp_ereport_ingester: Activator,
     pub task_reconfigurator_config_loader: Activator,
     pub task_fm_sitrep_loader: Activator,
+    pub task_fm_sitrep_gc: Activator,
 
     // Handles to activate background tasks that do not get used by Nexus
     // at-large.  These background tasks are implementation details as far as

nexus/db-model/src/schema_versions.rs

@@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock};
 ///
 /// This must be updated when you change the database schema.  Refer to
 /// schema/crdb/README.adoc in the root of this repository for details.
-pub const SCHEMA_VERSION: Version = Version::new(205, 0, 0);
+pub const SCHEMA_VERSION: Version = Version::new(206, 0, 0);
 
 /// List of all past database schema versions, in *reverse* order
 ///
@@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock<Vec<KnownVersion>> = LazyLock::new(|| {
         // |  leaving the first copy as an example for the next person.
         // v
         // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
+        KnownVersion::new(206, "fm-sitreps-by-parent-id-index"),
         KnownVersion::new(205, "fm-sitrep"),
         KnownVersion::new(204, "local-storage-dataset"),
         KnownVersion::new(203, "scim-actor-audit-log"),