Handle region snapshot replacement volume deletes

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

@@ -2563,39 +2563,48 @@ async fn cmd_db_region_used_by(
 async fn cmd_db_region_find_deleted(
     datastore: &DataStore,
 ) -> Result<(), anyhow::Error> {
-    let datasets_regions_volumes =
+    let freed_crucible_resources =
         datastore.find_deleted_volume_regions().await?;
 
     #[derive(Tabled)]
-    struct Row {
+    struct RegionRow {
         dataset_id: DatasetUuid,
         region_id: Uuid,
-        volume_id: String,
     }
 
-    let rows: Vec<Row> = datasets_regions_volumes
-        .into_iter()
+    #[derive(Tabled)]
+    struct VolumeRow {
+        volume_id: Uuid,
+    }
+
+    let region_rows: Vec<RegionRow> = freed_crucible_resources
+        .datasets_and_regions
+        .iter()
         .map(|row| {
-            let (dataset, region, volume) = row;
+            let (dataset, region) = row;
 
-            Row {
-                dataset_id: dataset.id(),
-                region_id: region.id(),
-                volume_id: if let Some(volume) = volume {
-                    volume.id().to_string()
-                } else {
-                    String::from("")
-                },
-            }
+            RegionRow { dataset_id: dataset.id(), region_id: region.id() }
         })
         .collect();
 
-    let table = tabled::Table::new(rows)
+    let table = tabled::Table::new(region_rows)
         .with(tabled::settings::Style::psql())
         .to_string();
 
     println!("{}", table);
 
+    let volume_rows: Vec<VolumeRow> = freed_crucible_resources
+        .volumes
+        .iter()
+        .map(|volume_id| VolumeRow { volume_id: *volume_id })
+        .collect();
+
+    let volume_table = tabled::Table::new(volume_rows)
+        .with(tabled::settings::Style::psql())
+        .to_string();
+
+    println!("{}", volume_table);
+
     Ok(())
 }
 

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

@@ -1655,6 +1655,14 @@ fn print_task_details(bgtask: &BackgroundTask, details: &serde_json::Value) {
                     println!("    > {line}");
                 }
 
+                println!(
+                    "    total requests completed ok: {}",
+                    status.requests_completed_ok.len(),
+                );
+                for line in &status.requests_completed_ok {
+                    println!("    > {line}");
+                }
+
                 println!("    errors: {}", status.errors.len());
                 for line in &status.errors {
                     println!("    > {line}");
@@ -1720,6 +1728,14 @@ fn print_task_details(bgtask: &BackgroundTask, details: &serde_json::Value) {
                     println!("    > {line}");
                 }
 
+                println!(
+                    "    total steps set to volume_deleted ok: {}",
+                    status.step_set_volume_deleted_ok.len(),
+                );
+                for line in &status.step_set_volume_deleted_ok {
+                    println!("    > {line}");
+                }
+
                 println!("    errors: {}", status.errors.len());
                 for line in &status.errors {
                     println!("    > {line}");
@@ -1831,10 +1847,11 @@ fn print_task_details(bgtask: &BackgroundTask, details: &serde_json::Value) {
 
             Ok(status) => {
                 println!(
-                    "    total records transitioned to done: {}",
-                    status.records_set_to_done.len(),
+                    "    region snapshot replacement finish sagas started \
+                    ok: {}",
+                    status.finish_invoked_ok.len()
                 );
-                for line in &status.records_set_to_done {
+                for line in &status.finish_invoked_ok {
                     println!("    > {line}");
                 }
 

dev-tools/omdb/tests/successes.out

@@ -627,7 +627,7 @@ task: "region_snapshot_replacement_finish"
   currently executing: no
   last completed activation: <REDACTED ITERATIONS>, triggered by a periodic timer firing
     started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
-    total records transitioned to done: 0
+    region snapshot replacement finish sagas started ok: 0
     errors: 0
 
 task: "region_snapshot_replacement_garbage_collection"
@@ -645,6 +645,7 @@ task: "region_snapshot_replacement_start"
     started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
     total requests created ok: 0
     total start saga invoked ok: 0
+    total requests completed ok: 0
     errors: 0
 
 task: "region_snapshot_replacement_step"
@@ -655,6 +656,7 @@ task: "region_snapshot_replacement_step"
     total step records created ok: 0
     total step garbage collect saga invoked ok: 0
     total step saga invoked ok: 0
+    total steps set to volume_deleted ok: 0
     errors: 0
 
 task: "saga_recovery"
@@ -1070,7 +1072,7 @@ task: "region_snapshot_replacement_finish"
   currently executing: no
   last completed activation: <REDACTED ITERATIONS>, triggered by a periodic timer firing
     started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
-    total records transitioned to done: 0
+    region snapshot replacement finish sagas started ok: 0
     errors: 0
 
 task: "region_snapshot_replacement_garbage_collection"
@@ -1088,6 +1090,7 @@ task: "region_snapshot_replacement_start"
     started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
     total requests created ok: 0
     total start saga invoked ok: 0
+    total requests completed ok: 0
     errors: 0
 
 task: "region_snapshot_replacement_step"
@@ -1098,6 +1101,7 @@ task: "region_snapshot_replacement_step"
     total step records created ok: 0
     total step garbage collect saga invoked ok: 0
     total step saga invoked ok: 0
+    total steps set to volume_deleted ok: 0
     errors: 0
 
 task: "saga_recovery"

nexus/db-model/src/region_snapshot_replacement.rs

@@ -28,6 +28,7 @@ impl_enum_type!(
     ReplacementDone => b"replacement_done"
     DeletingOldVolume => b"deleting_old_volume"
     Running => b"running"
+    Completing => b"completing"
     Complete => b"complete"
 );
 
@@ -46,6 +47,7 @@ impl std::str::FromStr for RegionSnapshotReplacementState {
                 Ok(RegionSnapshotReplacementState::DeletingOldVolume)
             }
             "running" => Ok(RegionSnapshotReplacementState::Running),
+            "completing" => Ok(RegionSnapshotReplacementState::Completing),
             "complete" => Ok(RegionSnapshotReplacementState::Complete),
             _ => Err(format!("unrecognized value {} for enum", s)),
         }
@@ -80,8 +82,13 @@ impl std::str::FromStr for RegionSnapshotReplacementState {
 ///          v                        ---
 ///                                   ---
 ///       Running                     |
-///                                   | set in region snapshot replacement
-///          |                        | finish background task
+///                                   |
+///          |                        |
+///          v                        |
+///                                   | responsibility of region snapshot
+///     Completing                    | replacement finish saga
+///                                   |
+///          |                        |
 ///          v                        |
 ///                                   |
 ///      Complete                     ---
@@ -133,6 +140,12 @@ pub struct RegionSnapshotReplacement {
     pub replacement_state: RegionSnapshotReplacementState,
 
     pub operating_saga_id: Option<Uuid>,
+
+    /// In order for the newly created region not to be deleted inadvertently,
+    /// an additional reference count bump is required. This volume should live
+    /// as long as this request so that all necessary replacements can be
+    /// completed.
+    pub new_region_volume_id: Option<Uuid>,
 }
 
 impl RegionSnapshotReplacement {
@@ -157,6 +170,7 @@ impl RegionSnapshotReplacement {
             old_snapshot_id,
             old_snapshot_volume_id: None,
             new_region_id: None,
+            new_region_volume_id: None,
             replacement_state: RegionSnapshotReplacementState::Requested,
             operating_saga_id: None,
         }

nexus/db-model/src/schema.rs

@@ -1929,6 +1929,7 @@ table! {
         new_region_id -> Nullable<Uuid>,
         replacement_state -> crate::RegionSnapshotReplacementStateEnum,
         operating_saga_id -> Nullable<Uuid>,
+        new_region_volume_id -> Nullable<Uuid>,
     }
 }
 

nexus/db-model/src/schema_versions.rs

@@ -17,7 +17,7 @@ use std::collections::BTreeMap;
 ///
 /// 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: SemverVersion = SemverVersion::new(114, 0, 0);
+pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(115, 0, 0);
 
 /// List of all past database schema versions, in *reverse* order
 ///
@@ -29,6 +29,7 @@ static KNOWN_VERSIONS: Lazy<Vec<KnownVersion>> = Lazy::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(115, "add-completing-and-new-region-volume"),
         KnownVersion::new(114, "crucible-ref-count-records"),
         KnownVersion::new(113, "add-tx-eq"),
         KnownVersion::new(112, "blueprint-dataset"),

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

@@ -125,16 +125,7 @@ pub use sled::TransitionError;
 pub use switch_port::SwitchPortSettingsCombinedResult;
 pub use virtual_provisioning_collection::StorageType;
 pub use vmm::VmmStateUpdateResult;
-pub use volume::read_only_resources_associated_with_volume;
-pub use volume::CrucibleResources;
-pub use volume::CrucibleTargets;
-pub use volume::ExistingTarget;
-pub use volume::ReplacementTarget;
-pub use volume::VolumeCheckoutReason;
-pub use volume::VolumeReplaceResult;
-pub use volume::VolumeReplacementParams;
-pub use volume::VolumeToDelete;
-pub use volume::VolumeWithTarget;
+pub use volume::*;
 
 // Number of unique datasets required to back a region.
 // TODO: This should likely turn into a configuration option.

nexus/db-queries/src/db/datastore/region_replacement.rs

@@ -21,6 +21,7 @@ use crate::db::pagination::Paginator;
 use crate::db::update_and_check::UpdateAndCheck;
 use crate::db::update_and_check::UpdateStatus;
 use crate::db::TransactionError;
+use crate::transaction_retry::OptionalError;
 use async_bb8_diesel::AsyncConnection;
 use async_bb8_diesel::AsyncRunQueryDsl;
 use diesel::prelude::*;
@@ -52,24 +53,38 @@ impl DataStore {
         opctx: &OpContext,
         request: RegionReplacement,
     ) -> Result<(), Error> {
+        let err = OptionalError::new();
         self.pool_connection_authorized(opctx)
             .await?
-            .transaction_async(|conn| async move {
-                use db::schema::region_replacement::dsl;
-
-                Self::volume_repair_insert_query(request.volume_id, request.id)
-                    .execute_async(&conn)
+            .transaction_async(|conn| {
+                let err = err.clone();
+                async move {
+                    use db::schema::region_replacement::dsl;
+
+                    Self::volume_repair_insert_in_txn(
+                        &conn,
+                        err,
+                        request.volume_id,
+                        request.id,
+                    )
                     .await?;
 
-                diesel::insert_into(dsl::region_replacement)
-                    .values(request)
-                    .execute_async(&conn)
-                    .await?;
+                    diesel::insert_into(dsl::region_replacement)
+                        .values(request)
+                        .execute_async(&conn)
+                        .await?;
 
-                Ok(())
+                    Ok(())
+                }
             })
             .await
-            .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
+            .map_err(|e| {
+                if let Some(err) = err.take() {
+                    err
+                } else {
+                    public_error_from_diesel(e, ErrorHandler::Server)
+                }
+            })
     }
 
     pub async fn get_region_replacement_request_by_id(
@@ -897,6 +912,7 @@ mod test {
 
     use crate::db::pub_test_utils::TestDatabase;
     use omicron_test_utils::dev;
+    use sled_agent_client::types::VolumeConstructionRequest;
 
     #[tokio::test]
     async fn test_one_replacement_per_volume() {
@@ -908,6 +924,20 @@ mod test {
         let region_2_id = Uuid::new_v4();
         let volume_id = Uuid::new_v4();
 
+        datastore
+            .volume_create(nexus_db_model::Volume::new(
+                volume_id,
+                serde_json::to_string(&VolumeConstructionRequest::Volume {
+                    id: volume_id,
+                    block_size: 512,
+                    sub_volumes: vec![],
+                    read_only_parent: None,
+                })
+                .unwrap(),
+            ))
+            .await
+            .unwrap();
+
         let request_1 = RegionReplacement::new(region_1_id, volume_id);
         let request_2 = RegionReplacement::new(region_2_id, volume_id);
 
@@ -939,6 +969,20 @@ mod test {
         let region_id = Uuid::new_v4();
         let volume_id = Uuid::new_v4();
 
+        datastore
+            .volume_create(nexus_db_model::Volume::new(
+                volume_id,
+                serde_json::to_string(&VolumeConstructionRequest::Volume {
+                    id: volume_id,
+                    block_size: 512,
+                    sub_volumes: vec![],
+                    read_only_parent: None,
+                })
+                .unwrap(),
+            ))
+            .await
+            .unwrap();
+
         let request = {
             let mut request = RegionReplacement::new(region_id, volume_id);
             request.replacement_state = RegionReplacementState::Running;
@@ -1031,6 +1075,20 @@ mod test {
         let region_id = Uuid::new_v4();
         let volume_id = Uuid::new_v4();
 
+        datastore
+            .volume_create(nexus_db_model::Volume::new(
+                volume_id,
+                serde_json::to_string(&VolumeConstructionRequest::Volume {
+                    id: volume_id,
+                    block_size: 512,
+                    sub_volumes: vec![],
+                    read_only_parent: None,
+                })
+                .unwrap(),
+            ))
+            .await
+            .unwrap();
+
         let request = {
             let mut request = RegionReplacement::new(region_id, volume_id);
             request.replacement_state = RegionReplacementState::ReplacementDone;