[PLAT-12342] Make lockUnivForUpdate and lockForFreezeAndUpdate the same

Summary: lockUniverseForUpdate is replaced with lockForFreezeAndUpdate. Some unnecessary helper methods are removed. Test Plan: Trivial change but UTs + iTest must not fail. Reviewers: cwang, sanketh, amahajan Reviewed By: cwang Subscribers: yugaware Differential Revision: https://phorge.dev.yugabyte.com/D32296
yugabyte · Mar 6, 2024 · ae49a75 · ae49a75
1 parent 2dab8fb
commit ae49a75
Show file tree

Hide file tree

Showing 50 changed files with 268 additions and 2,631 deletions.
diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/TaskExecutor.java b/managed/src/main/java/com/yugabyte/yw/commissioner/TaskExecutor.java
@@ -1152,6 +1152,11 @@ public synchronized void doHeartbeat() {
      */
     public void addSubTaskGroup(SubTaskGroup subTaskGroup) {
       log.info("Adding SubTaskGroup #{}: {}", subTaskGroups.size(), subTaskGroup.name);
+      if (subTaskGroup.getSubTaskCount() == 0) {
+        // Allowing to add this just messes up the positions.
+        log.info("Ignoring subtask SubTaskGroup {} as it is empty", subTaskGroup.name);
+        return;
+      }
       subTaskGroup.setRunnableTaskContext(this, subTaskPosition);
       subTaskGroups.add(subTaskGroup);
       subTaskPosition++;

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/AddOnClusterCreate.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/AddOnClusterCreate.java
@@ -32,10 +32,10 @@ protected AddOnClusterCreate(BaseTaskDependencies baseTaskDependencies) {
   @Override
   public void run() {
     log.info("Started {} task for uuid={}", getName(), taskParams().getUniverseUUID());
-
+    Universe universe = null;
     try {
       Cluster cluster = taskParams().getAddOnClusters().get(0);
-      Universe universe =
+      universe =
           lockAndFreezeUniverseForUpdate(
               taskParams().expectedUniverseVersion,
               u -> {
@@ -97,16 +97,25 @@ public void run() {
       log.error("Error executing task {} with error='{}'.", getName(), t.getMessage(), t);
       throw t;
     } finally {
-      // Mark the update of the universe as done. This will allow future edits/updates to the
-      // universe to happen.
-      Universe universe = unlockUniverseForUpdate();
-      if (universe.getConfig().getOrDefault(Universe.USE_CUSTOM_IMAGE, "false").equals("true")) {
-        universe.updateConfig(
-            ImmutableMap.of(
-                Universe.USE_CUSTOM_IMAGE,
-                Boolean.toString(
-                    universe.getUniverseDetails().nodeDetailsSet.stream()
-                        .allMatch(n -> n.ybPrebuiltAmi))));
+      if (universe != null) {
+        // Universe is locked by this task.
+        try {
+          // Fetch the latest universe.
+          universe = Universe.getOrBadRequest(universe.getUniverseUUID());
+          if (universe
+              .getConfig()
+              .getOrDefault(Universe.USE_CUSTOM_IMAGE, "false")
+              .equals("true")) {
+            universe.updateConfig(
+                ImmutableMap.of(
+                    Universe.USE_CUSTOM_IMAGE,
+                    Boolean.toString(
+                        universe.getUniverseDetails().nodeDetailsSet.stream()
+                            .allMatch(n -> n.ybPrebuiltAmi))));
+          }
+        } finally {
+          unlockUniverseForUpdate();
+        }
       }
     }
     log.info("Finished {} task.", getName());

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/AddOnClusterDelete.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/AddOnClusterDelete.java
@@ -66,7 +66,9 @@ public void run() {
       if (params().isForceDelete) {
         universe = forceLockUniverseForUpdate(-1 /* expectedUniverseVersion */);
       } else {
-        universe = lockUniverseForUpdate(params().expectedUniverseVersion);
+        universe =
+            lockAndFreezeUniverseForUpdate(
+                params().expectedUniverseVersion, null /* Txn callback */);
       }
 
       Cluster clusterToDelete = universe.getCluster(params().clusterUUID);

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/BackupUniverse.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/BackupUniverse.java
@@ -107,7 +107,7 @@ public void run() {
       checkUniverseVersion();
       // Update the universe DB with the update to be performed and set the 'updateInProgress' flag
       // to prevent other updates from happening.
-      universe = lockUniverseForUpdate(-1);
+      universe = lockAndFreezeUniverseForUpdate(-1, null /* Txn callback */);
 
       try {
 

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/CreateKubernetesUniverse.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/CreateKubernetesUniverse.java
@@ -105,7 +105,9 @@ public void run() {
         }
       }
 
-      Universe universe = lockUniverseForUpdate(taskParams().expectedUniverseVersion);
+      Universe universe =
+          lockAndFreezeUniverseForUpdate(
+              taskParams().expectedUniverseVersion, null /* Txn callback */);
       kubernetesStatus.startYBUniverseEventStatus(
           universe,
           taskParams().getKubernetesResourceDetails(),

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/CreateTableSpacesInUniverse.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/CreateTableSpacesInUniverse.java
@@ -40,7 +40,9 @@ public void run() {
       checkUniverseVersion();
 
       // Update the DB to prevent other changes from happening.
-      universe = lockUniverseForUpdate(taskParams().expectedUniverseVersion);
+      universe =
+          lockAndFreezeUniverseForUpdate(
+              taskParams().expectedUniverseVersion, null /* Txn callback */);
 
       createTableSpacesTask().setSubTaskGroupType(SubTaskGroupType.CreatingTablespaces);
 

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/CreateXClusterConfig.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/CreateXClusterConfig.java
@@ -69,10 +69,12 @@ public void run() {
     Universe targetUniverse = Universe.getOrBadRequest(xClusterConfig.getTargetUniverseUUID());
     try {
       // Lock the source universe.
-      lockUniverseForUpdate(sourceUniverse.getUniverseUUID(), sourceUniverse.getVersion());
+      lockAndFreezeUniverseForUpdate(
+          sourceUniverse.getUniverseUUID(), sourceUniverse.getVersion(), null /* Txn callback */);
       try {
         // Lock the target universe.
-        lockUniverseForUpdate(targetUniverse.getUniverseUUID(), targetUniverse.getVersion());
+        lockAndFreezeUniverseForUpdate(
+            targetUniverse.getUniverseUUID(), targetUniverse.getVersion(), null /* Txn callback */);
 
         createCheckXUniverseAutoFlag(sourceUniverse, targetUniverse)
             .setSubTaskGroupType(UserTaskDetails.SubTaskGroupType.PreflightChecks);

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/DeleteDrConfig.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/DeleteDrConfig.java
@@ -61,12 +61,16 @@ public void run() {
     try {
       if (sourceUniverse != null) {
         // Lock the source universe.
-        lockUniverseForUpdate(sourceUniverse.getUniverseUUID(), sourceUniverse.getVersion());
+        lockAndFreezeUniverseForUpdate(
+            sourceUniverse.getUniverseUUID(), sourceUniverse.getVersion(), null /* Txn callback */);
       }
       try {
         if (targetUniverse != null) {
           // Lock the target universe.
-          lockUniverseForUpdate(targetUniverse.getUniverseUUID(), targetUniverse.getVersion());
+          lockAndFreezeUniverseForUpdate(
+              targetUniverse.getUniverseUUID(),
+              targetUniverse.getVersion(),
+              null /* Txn callback */);
         }
 
         for (XClusterConfig xcc : drConfig.getXClusterConfigs()) {

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/DeleteXClusterConfig.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/DeleteXClusterConfig.java
@@ -45,12 +45,16 @@ public void run() {
     try {
       if (sourceUniverse != null) {
         // Lock the source universe.
-        lockUniverseForUpdate(sourceUniverse.getUniverseUUID(), sourceUniverse.getVersion());
+        lockAndFreezeUniverseForUpdate(
+            sourceUniverse.getUniverseUUID(), sourceUniverse.getVersion(), null /* Txn callback */);
       }
       try {
         if (targetUniverse != null) {
           // Lock the target universe.
-          lockUniverseForUpdate(targetUniverse.getUniverseUUID(), targetUniverse.getVersion());
+          lockAndFreezeUniverseForUpdate(
+              targetUniverse.getUniverseUUID(),
+              targetUniverse.getVersion(),
+              null /* Txn callback */);
         }
 
         createDeleteXClusterConfigSubtasks(xClusterConfig, sourceUniverse, targetUniverse);

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/DestroyKubernetesUniverse.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/DestroyKubernetesUniverse.java
@@ -85,7 +85,7 @@ public void run() {
       if (params().isForceDelete) {
         universe = forceLockUniverseForUpdate(-1);
       } else {
-        universe = lockUniverseForUpdate(-1 /* expectedUniverseVersion */);
+        universe = lockAndFreezeUniverseForUpdate(-1, null /* Txn callback */);
       }
       kubernetesStatus.startYBUniverseEventStatus(
           universe,

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/DestroyUniverse.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/DestroyUniverse.java
@@ -99,7 +99,7 @@ public void run() {
       if (params().isForceDelete) {
         universe = forceLockUniverseForUpdate(-1);
       } else {
-        universe = lockUniverseForUpdate(-1);
+        universe = lockAndFreezeUniverseForUpdate(-1, null /* Txn callback */);
       }
 
       // Delete xCluster configs involving this universe and put the locked universes to

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/EditDrConfig.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/EditDrConfig.java
@@ -55,14 +55,18 @@ public void run() {
         Universe.getOrBadRequest(newXClusterConfig.getTargetUniverseUUID());
     try {
       // Lock the source universe.
-      lockUniverseForUpdate(sourceUniverse.getUniverseUUID(), sourceUniverse.getVersion());
+      lockAndFreezeUniverseForUpdate(
+          sourceUniverse.getUniverseUUID(), sourceUniverse.getVersion(), null /* Txn callback */);
       try {
         // Lock the target universe.
-        lockUniverseForUpdate(targetUniverse.getUniverseUUID(), targetUniverse.getVersion());
+        lockAndFreezeUniverseForUpdate(
+            targetUniverse.getUniverseUUID(), targetUniverse.getVersion(), null /* Txn callback */);
         try {
           // Lock the new target universe.
-          lockUniverseForUpdate(
-              newTargetUniverse.getUniverseUUID(), newTargetUniverse.getVersion());
+          lockAndFreezeUniverseForUpdate(
+              newTargetUniverse.getUniverseUUID(),
+              newTargetUniverse.getVersion(),
+              null /* Txn callback */);
 
           addSubtasksToUseNewXClusterConfig(
               currentXClusterConfig,

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/EditKubernetesUniverse.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/EditKubernetesUniverse.java
@@ -78,7 +78,9 @@ public void run() {
       verifyParams(UniverseOpType.EDIT);
       // TODO: Would it make sense to have a precheck k8s task that does
       // some precheck operations to verify kubeconfig, svcaccount, connectivity to universe here ?
-      Universe universe = lockUniverseForUpdate(taskParams().expectedUniverseVersion);
+      Universe universe =
+          lockAndFreezeUniverseForUpdate(
+              taskParams().expectedUniverseVersion, null /* Txn callback */);
       addBasicPrecheckTasks();
 
       kubernetesStatus.startYBUniverseEventStatus(

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/EditUniverse.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/EditUniverse.java
@@ -126,24 +126,31 @@ public void run() {
       // Run all the tasks.
       getRunnableTask().runSubTasks();
     } catch (Throwable t) {
-      log.error("Error executing task {} with error='{}'.", getName(), t.getMessage(), t);
       errorString = t.getMessage();
+      log.error("Error executing task {} with error='{}'.", getName(), t.getMessage(), t);
       throw t;
     } finally {
       releaseReservedNodes();
-      // Mark the update of the universe as done. This will allow future edits/updates to the
-      // universe to happen.
-      universe = unlockUniverseForUpdate(errorString);
-
-      if (universe != null
-          && universe.getConfig().getOrDefault(Universe.USE_CUSTOM_IMAGE, "false").equals("true")) {
-        universe.updateConfig(
-            ImmutableMap.of(
-                Universe.USE_CUSTOM_IMAGE,
-                Boolean.toString(
-                    universe.getUniverseDetails().nodeDetailsSet.stream()
-                        .allMatch(n -> n.ybPrebuiltAmi))));
-        universe.save();
+      if (universe != null) {
+        // Universe is locked by this task.
+        try {
+          // Fetch the latest universe.
+          universe = Universe.getOrBadRequest(universe.getUniverseUUID());
+          if (universe
+              .getConfig()
+              .getOrDefault(Universe.USE_CUSTOM_IMAGE, "false")
+              .equals("true")) {
+            universe.updateConfig(
+                ImmutableMap.of(
+                    Universe.USE_CUSTOM_IMAGE,
+                    Boolean.toString(
+                        universe.getUniverseDetails().nodeDetailsSet.stream()
+                            .allMatch(n -> n.ybPrebuiltAmi))));
+            universe.save();
+          }
+        } finally {
+          unlockUniverseForUpdate(errorString);
+        }
       }
     }
     log.info("Finished {} task.", getName());

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/EditXClusterConfig.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/EditXClusterConfig.java
@@ -46,10 +46,12 @@ public void run() {
     XClusterConfigEditFormData editFormData = taskParams().getEditFormData();
 
     // Lock the source universe.
-    lockUniverseForUpdate(sourceUniverse.getUniverseUUID(), sourceUniverse.getVersion());
+    lockAndFreezeUniverseForUpdate(
+        sourceUniverse.getUniverseUUID(), sourceUniverse.getVersion(), null /* Txn callback */);
     try {
       // Lock the target universe.
-      lockUniverseForUpdate(targetUniverse.getUniverseUUID(), targetUniverse.getVersion());
+      lockAndFreezeUniverseForUpdate(
+          targetUniverse.getUniverseUUID(), targetUniverse.getVersion(), null /* Txn callback */);
       try {
 
         // Check Auto flags on source and target universes while resuming xCluster.

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/FailoverDrConfig.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/FailoverDrConfig.java
@@ -62,10 +62,12 @@ public void run() {
         Universe.getOrBadRequest(currentXClusterConfig.getTargetUniverseUUID());
     try {
       // Lock the source universe.
-      lockUniverseForUpdate(sourceUniverse.getUniverseUUID(), sourceUniverse.getVersion());
+      lockAndFreezeUniverseForUpdate(
+          sourceUniverse.getUniverseUUID(), sourceUniverse.getVersion(), null /* Txn callback */);
       try {
         // Lock the target universe.
-        lockUniverseForUpdate(targetUniverse.getUniverseUUID(), targetUniverse.getVersion());
+        lockAndFreezeUniverseForUpdate(
+            targetUniverse.getUniverseUUID(), targetUniverse.getVersion(), null /* Txn callback */);
 
         createSetDrStatesTask(
                 currentXClusterConfig,

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/LdapUnivSync.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/LdapUnivSync.java
@@ -65,7 +65,7 @@ public void run() {
     taskParams().ldapGroups = ldapGroups;
 
     try {
-      lockUniverseForUpdate(taskParams().getUniverseUUID(), -1);
+      lockAndFreezeUniverseForUpdate(-1, null /* Txn callback */);
 
       // queryLdap
       createQueryLdapServerTask(taskParams()).setSubTaskGroupType(SubTaskGroupType.QueryLdapServer);

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/MultiTableBackup.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/MultiTableBackup.java
@@ -95,7 +95,7 @@ public void run() {
 
       // Update the universe DB with the update to be performed and set the 'updateInProgress' flag
       // to prevent other updates from happening.
-      universe = lockUniverseForUpdate(-1);
+      universe = lockAndFreezeUniverseForUpdate(-1, null /* Txn callback */);
 
       try {
         String masterAddresses = universe.getMasterAddresses();

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/PauseUniverse.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/PauseUniverse.java
@@ -49,7 +49,7 @@ public void run() {
     try {
       // Update the universe DB with the update to be performed and set the
       // 'updateInProgress' flag to prevent other updates from happening.
-      Universe universe = lockUniverseForUpdate(-1 /* expectedUniverseVersion */);
+      Universe universe = lockAndFreezeUniverseForUpdate(-1, null /* Txn callback */);
       if (universe.getUniverseDetails().universePaused) {
         String msg =
             "Unable to pause universe \"" + universe.getName() + "\" as it is already paused.";

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/ReadOnlyClusterCreate.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/ReadOnlyClusterCreate.java
@@ -44,11 +44,11 @@ protected void createPrecheckTasks(Universe universe) {
   @Override
   public void run() {
     log.info("Started {} task for uuid={}", getName(), taskParams().getUniverseUUID());
-
+    Universe universe = null;
     try {
       // Set the 'updateInProgress' flag to prevent other updates from happening.
       Cluster cluster = taskParams().getReadOnlyClusters().get(0);
-      Universe universe =
+      universe =
           lockAndFreezeUniverseForUpdate(
               taskParams().expectedUniverseVersion,
               u -> {
@@ -157,17 +157,26 @@ public void run() {
       log.error("Error executing task {} with error='{}'.", getName(), t.getMessage(), t);
       throw t;
     } finally {
-      // Mark the update of the universe as done. This will allow future edits/updates to the
-      // universe to happen.
-      Universe universe = unlockUniverseForUpdate();
-      if (universe.getConfig().getOrDefault(Universe.USE_CUSTOM_IMAGE, "false").equals("true")) {
-        universe.updateConfig(
-            ImmutableMap.of(
-                Universe.USE_CUSTOM_IMAGE,
-                Boolean.toString(
-                    universe.getUniverseDetails().nodeDetailsSet.stream()
-                        .allMatch(n -> n.ybPrebuiltAmi))));
-        universe.save();
+      if (universe != null) {
+        // Universe is locked by this task.
+        try {
+          // Fetch the latest universe.
+          universe = Universe.getOrBadRequest(universe.getUniverseUUID());
+          if (universe
+              .getConfig()
+              .getOrDefault(Universe.USE_CUSTOM_IMAGE, "false")
+              .equals("true")) {
+            universe.updateConfig(
+                ImmutableMap.of(
+                    Universe.USE_CUSTOM_IMAGE,
+                    Boolean.toString(
+                        universe.getUniverseDetails().nodeDetailsSet.stream()
+                            .allMatch(n -> n.ybPrebuiltAmi))));
+            universe.save();
+          }
+        } finally {
+          unlockUniverseForUpdate();
+        }
       }
     }
     log.info("Finished {} task.", getName());

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/ReadOnlyClusterDelete.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/ReadOnlyClusterDelete.java
@@ -65,7 +65,9 @@ public void run() {
       if (params().isForceDelete) {
         universe = forceLockUniverseForUpdate(-1 /* expectedUniverseVersion */);
       } else {
-        universe = lockUniverseForUpdate(params().expectedUniverseVersion);
+        universe =
+            lockAndFreezeUniverseForUpdate(
+                params().expectedUniverseVersion, null /* Txn callback */);
       }
 
       List<Cluster> roClusters = universe.getUniverseDetails().getReadOnlyClusters();

diff --git a/...ged/src/main/java/com/yugabyte/yw/commissioner/tasks/ReadOnlyKubernetesClusterCreate.java b/...ged/src/main/java/com/yugabyte/yw/commissioner/tasks/ReadOnlyKubernetesClusterCreate.java
@@ -39,7 +39,9 @@ public void run() {
     log.info("Started {} task for uuid={}", getName(), taskParams().getUniverseUUID());
     try {
       verifyParams(UniverseOpType.CREATE);
-      Universe universe = lockUniverseForUpdate(taskParams().expectedUniverseVersion);
+      Universe universe =
+          lockAndFreezeUniverseForUpdate(
+              taskParams().expectedUniverseVersion, null /* Txn callback */);
       preTaskActions(universe);
       addBasicPrecheckTasks();
 

diff --git a/...ged/src/main/java/com/yugabyte/yw/commissioner/tasks/ReadOnlyKubernetesClusterDelete.java b/...ged/src/main/java/com/yugabyte/yw/commissioner/tasks/ReadOnlyKubernetesClusterDelete.java
@@ -69,7 +69,7 @@ public void run() {
       if (params().isForceDelete) {
         universe = forceLockUniverseForUpdate(-1);
       } else {
-        universe = lockUniverseForUpdate(-1 /* expectedUniverseVersion */);
+        universe = lockAndFreezeUniverseForUpdate(-1, null /* Txn callback */);
       }
 
       List<Cluster> roClusters = universe.getUniverseDetails().getReadOnlyClusters();