remove max gen from virtual_provisioning_collection_delete_instance

oxidecomputer · Jul 2, 2024 · 877074b · 877074b
1 parent f1e7c5d
commit 877074b
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 68 deletions.
diff --git a/nexus/db-queries/src/db/datastore/virtual_provisioning_collection.rs b/nexus/db-queries/src/db/datastore/virtual_provisioning_collection.rs
@@ -280,23 +280,18 @@ impl DataStore {
     }
 
     /// Transitively removes the CPU and memory charges for an instance from the
-    /// instance's project, silo, and fleet, provided that the instance's state
-    /// generation is less than `max_instance_gen`. This allows a caller who is
-    /// about to apply generation G to an instance to avoid deleting resources
-    /// if its update was superseded.
+    /// instance's project, silo, and fleet.
     pub async fn virtual_provisioning_collection_delete_instance(
         &self,
         opctx: &OpContext,
         id: InstanceUuid,
         project_id: Uuid,
         cpus_diff: i64,
         ram_diff: ByteCount,
-        max_instance_gen: i64,
     ) -> Result<Vec<VirtualProvisioningCollection>, Error> {
         let provisions =
             VirtualProvisioningCollectionUpdate::new_delete_instance(
                 id,
-                max_instance_gen,
                 cpus_diff,
                 ram_diff,
                 project_id,
@@ -518,16 +513,13 @@ mod test {
 
         // Delete the instance
 
-        // Make this value outrageously high, so that as a "max" it is ignored.
-        let max_instance_gen: i64 = 1000;
         datastore
             .virtual_provisioning_collection_delete_instance(
                 &opctx,
                 instance_id,
                 project_id,
                 cpus,
                 ram,
-                max_instance_gen,
             )
             .await
             .unwrap();
@@ -614,36 +606,13 @@ mod test {
 
         // Delete the instance
 
-        // If the "instance gen" is too low, the delete operation should be
-        // dropped. This mimics circumstances where an instance update arrives
-        // late to the query.
-        let max_instance_gen = 0;
         datastore
             .virtual_provisioning_collection_delete_instance(
                 &opctx,
                 instance_id,
                 project_id,
                 cpus,
                 ram,
-                max_instance_gen,
-            )
-            .await
-            .unwrap();
-        for id in ids {
-            verify_collection_usage(&datastore, &opctx, id, 12, 1 << 30, 0)
-                .await;
-        }
-
-        // Make this value outrageously high, so that as a "max" it is ignored.
-        let max_instance_gen = 1000;
-        datastore
-            .virtual_provisioning_collection_delete_instance(
-                &opctx,
-                instance_id,
-                project_id,
-                cpus,
-                ram,
-                max_instance_gen,
             )
             .await
             .unwrap();
@@ -664,7 +633,6 @@ mod test {
                 project_id,
                 cpus,
                 ram,
-                max_instance_gen,
             )
             .await
             .unwrap();

diff --git a/nexus/db-queries/src/db/queries/virtual_provisioning_collection_update.rs b/nexus/db-queries/src/db/queries/virtual_provisioning_collection_update.rs
@@ -81,17 +81,9 @@ pub fn from_diesel(e: DieselError) -> external::Error {
 #[derive(Clone)]
 enum UpdateKind {
     InsertStorage(VirtualProvisioningResource),
-    DeleteStorage {
-        id: uuid::Uuid,
-        disk_byte_diff: ByteCount,
-    },
+    DeleteStorage { id: uuid::Uuid, disk_byte_diff: ByteCount },
     InsertInstance(VirtualProvisioningResource),
-    DeleteInstance {
-        id: uuid::Uuid,
-        max_instance_gen: i64,
-        cpus_diff: i64,
-        ram_diff: ByteCount,
-    },
+    DeleteInstance { id: uuid::Uuid, cpus_diff: i64, ram_diff: ByteCount },
 }
 
 type SelectableSql<T> = <
@@ -246,15 +238,7 @@ WITH
     ),")
                 .bind::<sql_types::Uuid, _>(id)
             },
-            UpdateKind::DeleteInstance { id, max_instance_gen, .. } => {
-                // The filter condition here ensures that the provisioning record is
-                // only deleted if the corresponding instance has a generation
-                // number less than the supplied `max_instance_gen`. This allows a
-                // caller that is about to apply an instance update that will stop
-                // the instance and that bears generation G to avoid deleting
-                // resources if the instance generation was already advanced to or
-                // past G.
-                //
+            UpdateKind::DeleteInstance { id, .. } => {
                 // If the relevant instance ID is not in the database, then some
                 // other operation must have ensured the instance was previously
                 // stopped (because that's the only way it could have been deleted),
@@ -279,14 +263,13 @@ WITH
           FROM
             instance
           WHERE
-            instance.id = ").param().sql(" AND instance.state_generation < ").param().sql("
+            instance.id = ").param().sql("
           LIMIT 1
         )
           AS update
     ),")
                 .bind::<sql_types::Uuid, _>(id)
                 .bind::<sql_types::Uuid, _>(id)
-                .bind::<sql_types::BigInt, _>(max_instance_gen)
             },
         };
 
@@ -477,15 +460,13 @@ FROM
 
     pub fn new_delete_instance(
         id: InstanceUuid,
-        max_instance_gen: i64,
         cpus_diff: i64,
         ram_diff: ByteCount,
         project_id: uuid::Uuid,
     ) -> TypedSqlQuery<SelectableSql<VirtualProvisioningCollection>> {
         Self::apply_update(
             UpdateKind::DeleteInstance {
                 id: id.into_untyped_uuid(),
-                max_instance_gen,
                 cpus_diff,
                 ram_diff,
             },
@@ -567,14 +548,9 @@ mod test {
         let project_id = Uuid::nil();
         let cpus_diff = 4;
         let ram_diff = 2048.try_into().unwrap();
-        let max_instance_gen = 0;
 
         let query = VirtualProvisioningCollectionUpdate::new_delete_instance(
-            id,
-            max_instance_gen,
-            cpus_diff,
-            ram_diff,
-            project_id,
+            id, cpus_diff, ram_diff, project_id,
         );
 
         expectorate_query_contents(
@@ -684,11 +660,7 @@ mod test {
         let ram_diff = 2048.try_into().unwrap();
 
         let query = VirtualProvisioningCollectionUpdate::new_delete_instance(
-            id,
-            max_instance_gen,
-            cpus_diff,
-            ram_diff,
-            project_id,
+            id, cpus_diff, ram_diff, project_id,
         );
         let _ = query
             .explain_async(&conn)

diff --git a/nexus/src/app/sagas/instance_update/destroyed.rs b/nexus/src/app/sagas/instance_update/destroyed.rs
@@ -86,7 +86,6 @@ pub(super) async fn siu_destroyed_release_virtual_provisioning(
             instance.project_id,
             i64::from(instance.ncpus.0 .0),
             instance.memory,
-            i64::try_from(&max_gen).unwrap(),
         )
         .await;
     match result {