post merge fixy-wixy

Author: hawkw

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

@@ -412,109 +412,6 @@ impl DataStore {
         Ok(updated)
     }
 
-    /// Attempts to acquire the instance-updater lock for the saga with ID
-    /// `saga_id` at the current generation.
-    ///
-    /// # Arguments
-    ///
-    /// - `instance_id`: the UUID of the Instance to lock
-    /// - `current_gen`: the current generation of the instance's `updater_id`
-    /// - `saga_id`: the UUID of the saga that's attempting to lock this
-    ///   instance.
-    ///
-    /// # Returns
-    ///
-    /// - [`Ok`]`(`[`Some`]`(`[`InstanceUpdaterLock`]`))` if the lock was acquired.
-    /// - [`Ok`]`(`[`None`]`)` if the lock could not be acquired (i.e. the
-    ///   generation has advanced since `current_gen` was captured)
-    /// - [`Err`] if a database error occurred.
-    pub async fn instance_updater_try_lock(
-        &self,
-        opctx: &OpContext,
-        authz_instance: &authz::Instance,
-        current_gen: Generation,
-        saga_lock_id: &Uuid,
-    ) -> Result<Option<Generation>, Error> {
-        use db::schema::instance::dsl;
-
-        // The generation to advance to.
-        let new_gen = Generation(current_gen.0.next());
-
-        let instance_id = authz_instance.id();
-
-        let locked = diesel::update(dsl::instance)
-            .filter(dsl::time_deleted.is_null())
-            .filter(dsl::id.eq(instance_id))
-            // If the generation is the same as the captured generation, we can
-            // lock this instance.
-            .filter(dsl::updater_gen.eq(current_gen))
-            .set((
-                dsl::updater_gen.eq(new_gen),
-                dsl::updater_id.eq(Some(*saga_lock_id)),
-            ))
-            .check_if_exists::<Instance>(instance_id)
-            .execute_and_check(&*self.pool_connection_authorized(opctx).await?)
-            .await
-            .map(|r| match r.status {
-                UpdateStatus::Updated => Some(new_gen),
-                UpdateStatus::NotUpdatedButExists => None,
-            })
-            .map_err(|e| {
-                public_error_from_diesel(
-                    e,
-                    ErrorHandler::NotFoundByLookup(
-                        ResourceType::Instance,
-                        LookupType::ById(instance_id),
-                    ),
-                )
-            })?;
-
-        Ok(locked)
-    }
-
-    /// Release the instance-updater lock.
-    pub async fn instance_updater_unlock(
-        &self,
-        opctx: &OpContext,
-        authz_instance: &authz::Instance,
-        saga_lock_id: &Uuid,
-    ) -> Result<bool, Error> {
-        use db::schema::instance::dsl;
-
-        let instance_id = authz_instance.id();
-
-        let unlocked = diesel::update(dsl::instance)
-            .filter(dsl::time_deleted.is_null())
-            .filter(dsl::id.eq(authz_instance.id()))
-            // Only unlock the instance if:
-            // - the provided updater ID matches that of the saga that has
-            //   currently locked this instance.
-            .filter(dsl::updater_id.eq(Some(*saga_lock_id)))
-            .set((
-                dsl::updater_gen.eq(dsl::updater_gen + 1),
-                dsl::updater_id.eq(None::<Uuid>),
-            ))
-            .check_if_exists::<Instance>(instance_id)
-            .execute_and_check(&*self.pool_connection_authorized(opctx).await?)
-            .await
-            .map(|r| match r.status {
-                UpdateStatus::Updated => true,
-                // TODO(eliza): should this be an error?
-                UpdateStatus::NotUpdatedButExists => false,
-            })
-            .map_err(|e| {
-                public_error_from_diesel(
-                    e,
-                    ErrorHandler::NotFoundByLookup(
-                        ResourceType::Instance,
-                        LookupType::ById(instance_id),
-                    ),
-                )
-            })?;
-
-        Ok(unlocked)
-    }
-
     /// Updates an instance record and a VMM record with a single database
     /// command.
     ///

nexus/src/app/sagas/instance_update/mod.rs

@@ -129,81 +129,18 @@ async fn siu_lock_instance(
         crate::context::op_context_for_saga_action(&sagactx, serialized_authn);
     let datastore = osagactx.datastore();
     let log = osagactx.log();
-    let instance_id = authz_instance.id();
     slog::info!(
-        log,
+        osagactx.log(),
         "instance update: attempting to lock instance";
-        "instance_id" => %instance_id,
+        "instance_id" => %instance.id(),
         "saga_id" => %lock_id,
     );
-
-    loop {
-        let instance = datastore
-            .instance_refetch(&opctx, &authz_instance)
-            .await
-            .map_err(ActionError::action_failed)?;
-        // Look at the current lock state of the instance and determine whether
-        // we can lock it.
-        match instance.runtime_state.updater_id {
-            Some(ref id) if id == &lock_id => {
-                slog::info!(
-                    log,
-                    "instance update: instance already locked by this saga";
-                    "instance_id" => %instance_id,
-                    "saga_id" => %lock_id,
-                );
-                return Ok(instance.runtime_state.updater_gen);
-            }
-            Some(ref id) => {
-                slog::info!(
-                    log,
-                    "instance update: instance locked by another saga";
-                    "instance_id" => %instance_id,
-                    "saga_id" => %lock_id,
-                    "locked_by" => %lock_id,
-                );
-                return Err(ActionError::action_failed(serde_json::json!({
-                    "error": "instance locked by another saga",
-                    "saga_id": lock_id,
-                    "locked_by": id,
-                })));
-            }
-            None => {}
-        };
-        let gen = instance.runtime_state.updater_gen;
-        slog::debug!(
-            log,
-            "instance update: trying to acquire updater lock...";
-            "instance_id" => %instance_id,
-            "saga_id" => %lock_id,
-            "updater_gen" => ?gen,
-        );
-        let lock = datastore
-            .instance_updater_try_lock(&opctx, &authz_instance, gen, &lock_id)
-            .await
-            .map_err(ActionError::action_failed)?;
-        match lock {
-            Some(lock_gen) => {
-                slog::info!(
-                    log,
-                    "instance update: acquired updater lock";
-                    "instance_id" => %instance_id,
-                    "saga_id" => %lock_id,
-                    "updater_gen" => ?gen,
-                );
-                return Ok(lock_gen);
-            }
-            None => {
-                slog::debug!(
-                    log,
-                    "instance update: generation has advanced, retrying...";
-                    "instance_id" => %instance_id,
-                    "saga_id" => %lock_id,
-                    "updater_gen" => ?gen,
-                );
-            }
-        }
-    }
+    osagactx
+        .datastore()
+        .instance_updater_lock(&opctx, authz_instance, &lock_id)
+        .await
+        .map_err(ActionError::action_failed)
+        .map(|_| ())
 }
 
 async fn siu_fetch_state(