[helix] branch master updated: Remove the logic that disables the partitions which fails to be state-transited from the ERROR state. (#1721)

[jiajunwang]

This is an automated email from the ASF dual-hosted git repository.

jiajunwang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/helix.git


The following commit(s) were added to refs/heads/master by this push:
     new bee860a  Remove the logic that disables the partitions which fails to 
be state-transited from the ERROR state. (#1721)
bee860a is described below

commit bee860a8b8d2a9594e15b8e37de37fbb2e22b612
Author: Jiajun Wang <jjw...@linkedin.com>
AuthorDate: Tue May 4 15:17:46 2021 -0700

    Remove the logic that disables the partitions which fails to be 
state-transited from the ERROR state. (#1721)
    
    This logic of disabling an ERROR partition with additional failure 
transition causes confusion. When the partitions are disabled unexpected, the 
system admin has to double-check and manually enable the partitions.
    In reality, if the partition fails to finish a state transition from the 
ERROR state, it will remain in the ERROR state. And disabling it in addition is 
not desired. Otherwise, if it is eventually reset successfully, it should be 
good to continue serving. Sanity check shall be done in the ERROR->OFFLINE 
state transition process.
    
    [Behavior Changed] This PR changes the existing behavior.
---
 .../handling/HelixStateTransitionHandler.java      | 23 ----------------------
 .../org/apache/helix/integration/TestDrop.java     |  5 -----
 2 files changed, 28 deletions(-)

diff --git 
a/helix-core/src/main/java/org/apache/helix/messaging/handling/HelixStateTransitionHandler.java
 
b/helix-core/src/main/java/org/apache/helix/messaging/handling/HelixStateTransitionHandler.java
index 174d2b1..590064d 100644
--- 
a/helix-core/src/main/java/org/apache/helix/messaging/handling/HelixStateTransitionHandler.java
+++ 
b/helix-core/src/main/java/org/apache/helix/messaging/handling/HelixStateTransitionHandler.java
@@ -263,11 +263,6 @@ public class HelixStateTransitionHandler extends 
MessageHandler {
         _stateModel.rollbackOnError(_message, _notificationContext, error);
         _currentStateDelta.setState(partitionKey, 
HelixDefinedState.ERROR.toString());
         _stateModel.updateState(HelixDefinedState.ERROR.toString());
-
-        // if we have errors transit from ERROR state, disable the partition
-        if 
(_message.getFromState().equalsIgnoreCase(HelixDefinedState.ERROR.toString())) {
-          disablePartition();
-        }
       }
     }
 
@@ -300,19 +295,6 @@ public class HelixStateTransitionHandler extends 
MessageHandler {
     }
   }
 
-  void disablePartition() {
-    String instanceName = _manager.getInstanceName();
-    String resourceName = _message.getResourceName();
-    String partitionName = _message.getPartitionName();
-    String clusterName = _manager.getClusterName();
-    HelixAdmin admin = _manager.getClusterManagmentTool();
-    admin.enablePartition(false, clusterName, instanceName, resourceName,
-        Arrays.asList(partitionName));
-    logger.info("error in transit from ERROR to " + _message.getToState() + " 
for partition: "
-        + partitionName + ". disable it on " + instanceName);
-
-  }
-
   @Override
   public HelixTaskResult handleMessage() {
     NotificationContext context = _notificationContext;
@@ -444,11 +426,6 @@ public class HelixStateTransitionHandler extends 
MessageHandler {
         currentStateDelta.setState(partition, 
HelixDefinedState.ERROR.toString());
         _stateModel.updateState(HelixDefinedState.ERROR.toString());
 
-        // if transit from ERROR state, disable the partition
-        if 
(_message.getFromState().equalsIgnoreCase(HelixDefinedState.ERROR.toString())) {
-          disablePartition();
-        }
-
         PropertyKey currentStateKey =
             _isTaskMessage && !_isTaskCurrentStatePathDisabled ? keyBuilder
                 .taskCurrentState(instanceName, _message.getTgtSessionId(), 
resourceName)
diff --git 
a/helix-core/src/test/java/org/apache/helix/integration/TestDrop.java 
b/helix-core/src/test/java/org/apache/helix/integration/TestDrop.java
index b882d8f..a759f2a 100644
--- a/helix-core/src/test/java/org/apache/helix/integration/TestDrop.java
+++ b/helix-core/src/test/java/org/apache/helix/integration/TestDrop.java
@@ -273,11 +273,6 @@ public class TestDrop extends ZkTestBase {
     ZKHelixDataAccessor accessor =
         new ZKHelixDataAccessor(clusterName, new 
ZkBaseDataAccessor<>(_gZkClient));
     PropertyKey.Builder keyBuilder = accessor.keyBuilder();
-    InstanceConfig config = 
accessor.getProperty(keyBuilder.instanceConfig("localhost_12918"));
-    List<String> disabledPartitions = config.getDisabledPartitions();
-    // System.out.println("disabledPartitions: " + disabledPartitions);
-    Assert.assertEquals(disabledPartitions.size(), 1, "TestDB0_4 should be 
disabled");
-    Assert.assertEquals(disabledPartitions.get(0), "TestDB0_4");
 
     // ExternalView should have TestDB0_4->localhost_12918_>ERROR
     Thread.sleep(250L);