[helix] branch master updated: Fix Regression on Flaky Tests in TestResourceAccessor (#959)

[hu...@apache.org]

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

hulee 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 38b4fa3  Fix Regression on Flaky Tests in TestResourceAccessor (#959)
38b4fa3 is described below

commit 38b4fa3dc5c9973ba3a13e07163b48adc4ba7c4a
Author: Neal Sun <ne...@gmail.com>
AuthorDate: Tue Apr 21 12:49:56 2020 -0700

    Fix Regression on Flaky Tests in TestResourceAccessor (#959)
    
    The previous fix to TestResourceAccessor (stopping all mock controllers) might have affected other tests. A better approach, instead of stopping all controllers, is to pause the clusters instead.
---
 .../helix/rest/server/TestResourceAccessor.java    | 39 +++++++---------------
 1 file changed, 12 insertions(+), 27 deletions(-)

diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/TestResourceAccessor.java b/helix-rest/src/test/java/org/apache/helix/rest/server/TestResourceAccessor.java
index 9f5b980..ee949aa 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/server/TestResourceAccessor.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/server/TestResourceAccessor.java
@@ -40,7 +40,6 @@ import org.apache.helix.HelixManagerFactory;
 import org.apache.helix.InstanceType;
 import org.apache.helix.PropertyPathBuilder;
 import org.apache.helix.TestHelper;
-import org.apache.helix.integration.manager.ClusterControllerManager;
 import org.apache.helix.zookeeper.datamodel.ZNRecord;
 import org.apache.helix.controller.rebalancer.waged.WagedRebalancer;
 import org.apache.helix.model.ClusterConfig;
@@ -166,13 +165,13 @@ public class TestResourceAccessor extends AbstractTestClass {
   @Test(dependsOnMethods = "testExternalView")
   public void testPartitionHealth() throws Exception {
     System.out.println("Start test :" + TestHelper.getTestMethodName());
-    // Stop all controllers because this test case creates external views; the external views will
-    // be deleted by the controllers if controllers are not stopped and cause test failures
-    stopClustersControllers();
 
     String clusterName = "TestCluster_1";
     String resourceName = clusterName + "_db_0";
 
+    // Disable the cluster to prevent external view from being removed
+    _gSetupTool.getClusterManagementTool().enableCluster(clusterName, false);
+
     // Use mock numbers for testing
     Map<String, String> idealStateParams = new HashMap<>();
     idealStateParams.put("MinActiveReplicas", "2");
@@ -211,16 +210,14 @@ public class TestResourceAccessor extends AbstractTestClass {
     Assert.assertEquals(healthStatus.get("p1"), "PARTIAL_HEALTHY");
     Assert.assertEquals(healthStatus.get("p2"), "UNHEALTHY");
     System.out.println("End test :" + TestHelper.getTestMethodName());
-    // Restart the stopped controllers
-    startClustersControllers();
+
+    // Re-enable the cluster
+    _gSetupTool.getClusterManagementTool().enableCluster(clusterName, true);
   }
 
   @Test(dependsOnMethods = "testPartitionHealth")
   public void testResourceHealth() throws Exception {
     System.out.println("Start test :" + TestHelper.getTestMethodName());
-    // Stop all controllers because this test case creates external views; the external views will
-    // be deleted by the controllers if controllers are not stopped and cause test failures
-    stopClustersControllers();
 
     String clusterName = "TestCluster_1";
     Map<String, String> idealStateParams = new HashMap<>();
@@ -230,6 +227,9 @@ public class TestResourceAccessor extends AbstractTestClass {
     idealStateParams.put("Replicas", "3");
     idealStateParams.put("NumPartitions", "3");
 
+    // Disable the cluster to prevent external view from being removed
+    _gSetupTool.getClusterManagementTool().enableCluster(clusterName, false);
+
     // Create a healthy resource
     String resourceNameHealthy = clusterName + "_db_0";
     Map<String, List<String>> partitionReplicaStates = new LinkedHashMap<>();
@@ -299,8 +299,9 @@ public class TestResourceAccessor extends AbstractTestClass {
     Assert.assertEquals(healthStatus.get(resourceNamePartiallyHealthy), "PARTIAL_HEALTHY");
     Assert.assertEquals(healthStatus.get(resourceNameUnhealthy), "UNHEALTHY");
     System.out.println("End test :" + TestHelper.getTestMethodName());
-    // Restart the stopped controllers
-    startClustersControllers();
+
+    // Re-enable the cluster
+    _gSetupTool.getClusterManagementTool().enableCluster(clusterName, true);
   }
 
   /**
@@ -656,20 +657,4 @@ public class TestResourceAccessor extends AbstractTestClass {
         externalView);
     System.out.println("End test :" + TestHelper.getTestMethodName());
   }
-
-  private void stopClustersControllers() {
-    for (ClusterControllerManager cm : _clusterControllerManagers) {
-      if (cm != null && cm.isConnected()) {
-        cm.syncStop();
-      }
-    }
-  }
-
-  private void startClustersControllers() {
-    for (ClusterControllerManager cm : _clusterControllerManagers) {
-      if (cm != null && cm.isConnected()) {
-        cm.syncStart();
-      }
-    }
-  }
 }