more consistent naming for logs

Author: hawkw

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

@@ -159,8 +159,8 @@ pub struct InstanceAndVmms {
 /// when the lock is released.
 #[derive(Debug, serde::Serialize, serde::Deserialize)]
 pub struct UpdaterLock {
-    pub saga_lock_id: Uuid,
-    pub locked_gen: Generation,
+    updater_id: Uuid,
+    locked_gen: Generation,
 }
 
 /// Errors returned by [`DataStore::instance_updater_lock`].
@@ -685,7 +685,7 @@ impl DataStore {
     /// # Arguments
     ///
     /// - `authz_instance`: the instance to attempt to lock to lock
-    /// - `saga_lock_id`: the UUID of the saga that's attempting to lock this
+    /// - `updater_id`: the UUID of the saga that's attempting to lock this
     ///   instance.
     ///
     /// # Returns
@@ -700,7 +700,7 @@ impl DataStore {
         &self,
         opctx: &OpContext,
         authz_instance: &authz::Instance,
-        saga_lock_id: Uuid,
+        updater_id: Uuid,
     ) -> Result<UpdaterLock, UpdaterLockError> {
         use db::schema::instance::dsl;
 
@@ -726,16 +726,16 @@ impl DataStore {
             match instance.updater_id {
                 // If the `updater_id` field is not null and the ID equals this
                 // saga's ID, we already have the lock. We're done here!
-                Some(lock_id) if lock_id == saga_lock_id => {
+                Some(lock_id) if lock_id == updater_id => {
                     slog::debug!(
                         &opctx.log,
                         "instance updater lock acquired!";
                         "instance_id" => %instance_id,
-                        "saga_id" => %saga_lock_id,
-                        "already_locked" => !did_lock,
+                        "updater_id" => %updater_id,
                         "locked_gen" => ?locked_gen,
+                        "already_locked" => !did_lock,
                     );
-                    return Ok(UpdaterLock { saga_lock_id, locked_gen });
+                    return Ok(UpdaterLock { updater_id, locked_gen });
                 }
                 // The `updater_id` field is set, but it's not our ID. The instance
                 // is locked by a different saga, so give up.
@@ -745,7 +745,7 @@ impl DataStore {
                         "instance is locked by another saga";
                         "instance_id" => %instance_id,
                         "locked_by" => %lock_id,
-                        "saga_id" => %saga_lock_id,
+                        "updater_id" => %updater_id,
                     );
                     return Err(UpdaterLockError::AlreadyLocked);
                 }
@@ -761,7 +761,7 @@ impl DataStore {
                 &opctx.log,
                 "attempting to acquire instance updater lock";
                 "instance_id" => %instance_id,
-                "saga_id" => %saga_lock_id,
+                "updater_id" => %updater_id,
                 "current_gen" => ?current_gen,
             );
 
@@ -780,7 +780,7 @@ impl DataStore {
                 .filter(dsl::updater_gen.eq(current_gen))
                 .set((
                     dsl::updater_gen.eq(locked_gen),
-                    dsl::updater_id.eq(Some(saga_lock_id)),
+                    dsl::updater_id.eq(Some(updater_id)),
                 ))
                 .check_if_exists::<Instance>(instance_id)
                 .execute_and_check(
@@ -813,7 +813,7 @@ impl DataStore {
         &self,
         opctx: &OpContext,
         authz_instance: &authz::Instance,
-        UpdaterLock { saga_lock_id: parent_id, locked_gen }: UpdaterLock,
+        UpdaterLock { updater_id: parent_id, locked_gen }: UpdaterLock,
         child_lock_id: Uuid,
     ) -> Result<UpdaterLock, UpdaterLockError> {
         use db::schema::instance::dsl;
@@ -851,11 +851,13 @@ impl DataStore {
                     &opctx.log,
                     "inherited lock from {parent_id} to {child_lock_id}";
                     "instance_id" => %instance_id,
+                    "updater_id" => %child_lock_id,
                     "locked_gen" => ?new_gen,
+                    "parent_id" => %parent_id,
                     "parent_gen" => ?locked_gen,
                 );
                 Ok(UpdaterLock {
-                    saga_lock_id: child_lock_id,
+                    updater_id: child_lock_id,
                     locked_gen: new_gen,
                 })
             }
@@ -868,7 +870,7 @@ impl DataStore {
             } if found.updater_id == Some(child_lock_id) => {
                 debug_assert_eq!(found.updater_gen, new_gen,);
                 Ok(UpdaterLock {
-                    saga_lock_id: child_lock_id,
+                    updater_id: child_lock_id,
                     locked_gen: new_gen,
                 })
             }
@@ -883,7 +885,7 @@ impl DataStore {
     /// [`DataStore::instance_updater_lock`].
     ///
     /// This method will unlock the instance if (and only if) the lock is
-    /// currently held by the provided `saga_lock_id`. If the lock is held by a
+    /// currently held by the provided `updater_id`. If the lock is held by a
     /// different saga UUID, the instance will remain locked. If the instance
     /// has already been unlocked, this method will return `false`.
     ///
@@ -896,7 +898,7 @@ impl DataStore {
         &self,
         opctx: &OpContext,
         authz_instance: &authz::Instance,
-        UpdaterLock { saga_lock_id, locked_gen }: UpdaterLock,
+        UpdaterLock { updater_id, locked_gen }: UpdaterLock,
     ) -> Result<bool, Error> {
         use db::schema::instance::dsl;
 
@@ -908,7 +910,7 @@ impl DataStore {
             // 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)))
+            .filter(dsl::updater_id.eq(Some(updater_id)))
             // - the provided updater generation matches the current updater
             //   generation.
             .filter(dsl::updater_gen.eq(locked_gen))
@@ -947,17 +949,17 @@ impl DataStore {
             // is almost certainly a programmer error.
             UpdateAndQueryResult { ref found, .. } => {
                 match found.updater_id {
-                    Some(lock_holder) => {
+                    Some(actual_id) => {
                         slog::error!(
                             &opctx.log,
                             "attempted to release a lock held by another saga";
                             "instance_id" => %instance_id,
-                            "saga_id" => %saga_lock_id,
-                            "lock_holder" => %lock_holder,
+                            "updater_id" => %updater_id,
+                            "actual_id" => %actual_id,
                             "found_gen" => ?found.updater_gen,
                             "locked_gen" => ?locked_gen,
                         );
-                        debug_assert_ne!(lock_holder, saga_lock_id);
+                        debug_assert_ne!(actual_id, updater_id);
                         Err(Error::internal_error(
                             "attempted to release a lock held by another saga! this is a bug!",
                         ))
@@ -1067,7 +1069,7 @@ mod tests {
                     stringify!($id)
                 ));
                 assert_eq!(
-                    lock.saga_lock_id,
+                    lock.updater_id,
                     $id,
                     "instance's `updater_id` must be set to {}",
                     stringify!($id),
@@ -1137,7 +1139,7 @@ mod tests {
                 .await
         )
         .expect("instance should be locked");
-        assert_eq!(lock1.saga_lock_id, saga1);
+        assert_eq!(lock1.updater_id, saga1);
 
         // doing it again should be fine.
         let lock2 = dbg!(
@@ -1148,7 +1150,7 @@ mod tests {
         .expect(
             "instance_updater_lock should succeed again with the same saga ID",
         );
-        assert_eq!(lock2.saga_lock_id, saga1);
+        assert_eq!(lock2.updater_id, saga1);
         // the generation should not have changed as a result of the second
         // update.
         assert_eq!(lock1.locked_gen, lock2.locked_gen);
@@ -1209,7 +1211,7 @@ mod tests {
                     // an incorrect one is constructed, or a raw database query
                     // attempts an invalid unlock operation.
                     UpdaterLock {
-                        saga_lock_id: saga2,
+                        updater_id: saga2,
                         locked_gen: lock1.locked_gen,
                     },
                 )
@@ -1246,7 +1248,7 @@ mod tests {
                     // Again, these fields are private specifically to prevent
                     // you from doing this exact thing. But, we should  still
                     // test that we handle it gracefully.
-                    UpdaterLock { saga_lock_id: saga1, locked_gen: next_gen },
+                    UpdaterLock { updater_id: saga1, locked_gen: next_gen },
                 )
                 .await
         )