Support preserving history of HELIX_ENABLED legacy fields when combo …

…of instance operation and old helix version are used (#2987) When the new instance operation APIs are used, we need to consider that an older version of helix could have been used to modify the legacy fields of HELIX_ENABLED, HELIX_DISABLED_TYPE, and HELIX_DISABLED_REASON. When this happens, we will write those legacy fields to the USER source instance operation to preserve the history when another source sets the instance operation. When the other source re-enables the instance, we should restore the old legacy fields with what was previously set.
apache · Jan 18, 2025 · 0f95ef3 · 0f95ef3
1 parent 4e1b5ea
commit 0f95ef3
Show file tree

Hide file tree

Showing 7 changed files with 284 additions and 112 deletions.
diff --git a/helix-common/src/main/java/org/apache/helix/constants/InstanceConstants.java b/helix-common/src/main/java/org/apache/helix/constants/InstanceConstants.java
@@ -54,7 +54,9 @@ public int getPriority() {
      *
      * @param disabledType InstanceDisabledType
      * @return InstanceOperationTrigger
+     * @deprecated The concept of InstanceDisabledType mapping directly to an InstanceOperationSource is no longer used.
      */
+    @Deprecated
     public static InstanceOperationSource instanceDisabledTypeToInstanceOperationSource(
         InstanceDisabledType disabledType) {
       switch (disabledType) {

diff --git a/...-core/src/main/java/org/apache/helix/cloud/event/helix/DefaultCloudEventCallbackImpl.java b/...-core/src/main/java/org/apache/helix/cloud/event/helix/DefaultCloudEventCallbackImpl.java
@@ -51,14 +51,9 @@ public void disableInstance(HelixManager manager, Object eventInfo) {
     LOG.info("DefaultCloudEventCallbackImpl disable Instance {}", manager.getInstanceName());
     if (InstanceValidationUtil
         .isEnabled(manager.getHelixDataAccessor(), manager.getInstanceName())) {
-      InstanceUtil.setInstanceOperation(manager.getConfigAccessor(),
-          manager.getHelixDataAccessor().getBaseDataAccessor(), manager.getClusterName(),
-          manager.getInstanceName(),
-              new InstanceConfig.InstanceOperation.Builder().setOperation(
-                      InstanceConstants.InstanceOperation.DISABLE)
-                  .setSource(InstanceConstants.InstanceOperationSource.AUTOMATION)
-                  .setReason(message)
-                  .build());
+      manager.getClusterManagmentTool()
+          .enableInstance(manager.getClusterName(), manager.getInstanceName(), false,
+              InstanceConstants.InstanceDisabledType.CLOUD_EVENT, message);
     }
     HelixEventHandlingUtil.updateCloudEventOperationInClusterConfig(manager.getClusterName(),
         manager.getInstanceName(), manager.getHelixDataAccessor().getBaseDataAccessor(), false,
@@ -79,13 +74,10 @@ public void enableInstance(HelixManager manager, Object eventInfo) {
     HelixEventHandlingUtil
         .updateCloudEventOperationInClusterConfig(manager.getClusterName(), instanceName,
             manager.getHelixDataAccessor().getBaseDataAccessor(), true, message);
-    InstanceUtil.setInstanceOperation(manager.getConfigAccessor(),
-        manager.getHelixDataAccessor().getBaseDataAccessor(), manager.getClusterName(),
-        manager.getInstanceName(),
-            new InstanceConfig.InstanceOperation.Builder().setOperation(
-                    InstanceConstants.InstanceOperation.ENABLE)
-                .setSource(InstanceConstants.InstanceOperationSource.AUTOMATION).setReason(message)
-                .build());
+    if (HelixEventHandlingUtil.isInstanceDisabledForCloudEvent(instanceName, accessor)) {
+      manager.getClusterManagmentTool().enableInstance(manager.getClusterName(), instanceName, true,
+          InstanceConstants.InstanceDisabledType.CLOUD_EVENT, message);
+    }
   }
 
   /**

diff --git a/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java b/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java
@@ -747,7 +747,7 @@ public boolean forceKillInstance(String clusterName, String instanceName, String
 
     InstanceConfig.InstanceOperation instanceOperationObj = new InstanceConfig.InstanceOperation.Builder()
         .setOperation(InstanceConstants.InstanceOperation.UNKNOWN).setReason(reason)
-        .setSource(operationSource != null ? operationSource : InstanceConstants.InstanceOperationSource.USER).build();
+        .setSource(operationSource).build();
     InstanceConfig instanceConfig = getInstanceConfig(clusterName, instanceName);
     instanceConfig.setInstanceOperation(instanceOperationObj);
 
@@ -2363,12 +2363,17 @@ public ZNRecord update(ZNRecord currentData) {
         }
 
         InstanceConfig config = new InstanceConfig(currentData);
-        config.setInstanceOperation(new InstanceConfig.InstanceOperation.Builder().setOperation(
-            enabled ? InstanceConstants.InstanceOperation.ENABLE
-                : InstanceConstants.InstanceOperation.DISABLE).setReason(reason).setSource(
-            disabledType != null
-                ? InstanceConstants.InstanceOperationSource.instanceDisabledTypeToInstanceOperationSource(
-                disabledType) : null).build());
+        config.setInstanceEnabled(enabled);
+        if (!enabled) {
+          // new disabled type and reason will overwrite existing ones.
+          config.resetInstanceDisabledTypeAndReason();
+          if (reason != null) {
+            config.setInstanceDisabledReason(reason);
+          }
+          if (disabledType != null) {
+            config.setInstanceDisabledType(disabledType);
+          }
+        }
         return config.getRecord();
       }
     }, AccessOption.PERSISTENT);