From 877074b8ca19d9ddcb5a04f1ffb819c4d635871d Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Tue, 2 Jul 2024 10:12:19 -0700 Subject: [PATCH] remove max gen from `virtual_provisioning_collection_delete_instance` --- .../virtual_provisioning_collection.rs | 34 +--------------- .../virtual_provisioning_collection_update.rs | 40 +++---------------- .../app/sagas/instance_update/destroyed.rs | 1 - 3 files changed, 7 insertions(+), 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 index 247eefd3d5b..7c3e1c4b8ff 100644 --- a/nexus/db-queries/src/db/datastore/virtual_provisioning_collection.rs +++ b/nexus/db-queries/src/db/datastore/virtual_provisioning_collection.rs @@ -280,10 +280,7 @@ 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, @@ -291,12 +288,10 @@ impl DataStore { project_id: Uuid, cpus_diff: i64, ram_diff: ByteCount, - max_instance_gen: i64, ) -> Result, Error> { let provisions = VirtualProvisioningCollectionUpdate::new_delete_instance( id, - max_instance_gen, cpus_diff, ram_diff, project_id, @@ -518,8 +513,6 @@ 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, @@ -527,7 +520,6 @@ mod test { project_id, cpus, ram, - max_instance_gen, ) .await .unwrap(); @@ -614,10 +606,6 @@ 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, @@ -625,25 +613,6 @@ mod test { 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 index fd86912107f..3381ec3e8a9 100644 --- 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 = < @@ -246,15 +238,7 @@ WITH ),") .bind::(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::(id) .bind::(id) - .bind::(max_instance_gen) }, }; @@ -477,7 +460,6 @@ FROM pub fn new_delete_instance( id: InstanceUuid, - max_instance_gen: i64, cpus_diff: i64, ram_diff: ByteCount, project_id: uuid::Uuid, @@ -485,7 +467,6 @@ FROM 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 index 75bd27793ee..206c1e40263 100644 --- 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 {