You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by ne...@apache.org on 2022/10/28 17:02:02 UTC

[helix] branch master updated: Unit test stability improvement (#2255)

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

nealsun 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 b59280114 Unit test stability improvement (#2255)
b59280114 is described below

commit b5928011441a2efd0674339ab5074f60c6583d95
Author: Qi (Quincy) Qu <qq...@linkedin.com>
AuthorDate: Fri Oct 28 10:01:56 2022 -0700

    Unit test stability improvement (#2255)
    
    Fix potential race condition issue in unit test and other improvement.
---
 .../org/apache/helix/webapp/AdminTestHelper.java   |  14 +-
 .../helix/manager/zk/ZkBucketDataAccessor.java     |   4 +
 .../java/org/apache/helix/common/ZkTestBase.java   | 210 ++++-----------------
 .../manager/TestParticipantManager.java            |   1 -
 .../multizk/TestMultiZkConectionConfig.java        |  21 +--
 .../multizk/TestMultiZkHelixJavaApis.java          |  15 +-
 .../paticipant/TestInstanceAutoJoin.java           |   9 +-
 .../integration/task/TestForceDeleteWorkflow.java  |   7 +-
 .../helix/manager/zk/TestZkBucketDataAccessor.java |   6 +-
 .../org/apache/helix/tools/TestHelixAdminCli.java  |   1 -
 .../view/integration/TestHelixViewAggregator.java  |   2 +-
 zookeeper-api/src/test/conf/testng.xml             |   4 +-
 .../apache/helix/zookeeper/impl/ZkTestBase.java    |   9 +-
 .../zookeeper/impl/client/TestSharedZkClient.java  |   2 +-
 14 files changed, 67 insertions(+), 238 deletions(-)

diff --git a/helix-admin-webapp/src/test/java/org/apache/helix/webapp/AdminTestHelper.java b/helix-admin-webapp/src/test/java/org/apache/helix/webapp/AdminTestHelper.java
index b514ed59c..3b50783b1 100644
--- a/helix-admin-webapp/src/test/java/org/apache/helix/webapp/AdminTestHelper.java
+++ b/helix-admin-webapp/src/test/java/org/apache/helix/webapp/AdminTestHelper.java
@@ -89,11 +89,10 @@ public class AdminTestHelper {
     result.write(sw);
 
     String responseStr = sw.toString();
-    Assert.assertTrue(responseStr.toLowerCase().indexOf("error") == -1);
-    Assert.assertTrue(responseStr.toLowerCase().indexOf("exception") == -1);
+    Assert.assertEquals(responseStr.toLowerCase().indexOf("error"), -1);
+    Assert.assertEquals(responseStr.toLowerCase().indexOf("exception"), -1);
     ObjectMapper mapper = new ObjectMapper();
-    ZNRecord record = mapper.readValue(new StringReader(responseStr), ZNRecord.class);
-    return record;
+    return mapper.readValue(new StringReader(responseStr), ZNRecord.class);
   }
 
   public static void delete(Client client, String url) throws IOException {
@@ -119,11 +118,10 @@ public class AdminTestHelper {
       result.write(sw);
     }
     String responseStr = sw.toString();
-    Assert.assertTrue(responseStr.toLowerCase().indexOf("error") == -1);
-    Assert.assertTrue(responseStr.toLowerCase().indexOf("exception") == -1);
+    Assert.assertEquals(responseStr.toLowerCase().indexOf("error"), -1);
+    Assert.assertEquals(responseStr.toLowerCase().indexOf("exception"), -1);
 
     ObjectMapper mapper = new ObjectMapper();
-    ZNRecord record = mapper.readValue(new StringReader(responseStr), ZNRecord.class);
-    return record;
+    return mapper.readValue(new StringReader(responseStr), ZNRecord.class);
   }
 }
diff --git a/helix-core/src/main/java/org/apache/helix/manager/zk/ZkBucketDataAccessor.java b/helix-core/src/main/java/org/apache/helix/manager/zk/ZkBucketDataAccessor.java
index c20b3f902..a99ca99aa 100644
--- a/helix-core/src/main/java/org/apache/helix/manager/zk/ZkBucketDataAccessor.java
+++ b/helix-core/src/main/java/org/apache/helix/manager/zk/ZkBucketDataAccessor.java
@@ -96,6 +96,10 @@ public class ZkBucketDataAccessor implements BucketDataAccessor, AutoCloseable {
     this(zkClient, DEFAULT_BUCKET_SIZE, DEFAULT_VERSION_TTL, true);
   }
 
+  public ZkBucketDataAccessor(RealmAwareZkClient zkClient, int bucketSize, long versionTTLms) {
+    this(zkClient, bucketSize, versionTTLms, true);
+  }
+
   private ZkBucketDataAccessor(RealmAwareZkClient zkClient, int bucketSize, long versionTTLms,
       boolean usesExternalZkClient) {
     _zkClient = zkClient;
diff --git a/helix-core/src/test/java/org/apache/helix/common/ZkTestBase.java b/helix-core/src/test/java/org/apache/helix/common/ZkTestBase.java
index b1aadf65e..1cea1926a 100644
--- a/helix-core/src/test/java/org/apache/helix/common/ZkTestBase.java
+++ b/helix-core/src/test/java/org/apache/helix/common/ZkTestBase.java
@@ -19,12 +19,12 @@ package org.apache.helix.common;
  * under the License.
  */
 
+import com.google.common.base.Preconditions;
 import java.io.IOException;
 import java.lang.management.ManagementFactory;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -44,7 +44,6 @@ import org.apache.helix.PropertyKey.Builder;
 import org.apache.helix.PropertyPathBuilder;
 import org.apache.helix.SystemPropertyKeys;
 import org.apache.helix.TestHelper;
-import org.apache.helix.ThreadLeakageChecker;
 import org.apache.helix.api.config.HelixConfigProperty;
 import org.apache.helix.controller.pipeline.AbstractAsyncBaseStage;
 import org.apache.helix.controller.pipeline.Pipeline;
@@ -61,7 +60,6 @@ import org.apache.helix.manager.zk.ZNRecordSerializer;
 import org.apache.helix.manager.zk.ZkBaseDataAccessor;
 import org.apache.helix.model.BuiltInStateModelDefinitions;
 import org.apache.helix.model.ClusterConfig;
-import org.apache.helix.model.ConfigScope;
 import org.apache.helix.model.CurrentState;
 import org.apache.helix.model.ExternalView;
 import org.apache.helix.model.IdealState;
@@ -71,7 +69,6 @@ import org.apache.helix.model.Message;
 import org.apache.helix.model.OnlineOfflineSMD;
 import org.apache.helix.model.ResourceConfig;
 import org.apache.helix.model.StateModelDefinition;
-import org.apache.helix.model.builder.ConfigScopeBuilder;
 import org.apache.helix.tools.ClusterSetup;
 import org.apache.helix.tools.ClusterStateVerifier;
 import org.apache.helix.tools.StateModelConfigGenerator;
@@ -79,26 +76,19 @@ import org.apache.helix.zookeeper.api.client.HelixZkClient;
 import org.apache.helix.zookeeper.datamodel.ZNRecord;
 import org.apache.helix.zookeeper.impl.client.ZkClient;
 import org.apache.helix.zookeeper.impl.factory.DedicatedZkClientFactory;
-import org.apache.helix.zookeeper.zkclient.IZkStateListener;
-import org.apache.helix.zookeeper.zkclient.ZkConnection;
 import org.apache.helix.zookeeper.zkclient.ZkServer;
-import org.apache.zookeeper.WatchedEvent;
-import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZooKeeper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.testng.Assert;
-import org.testng.AssertJUnit;
 import org.testng.ITestContext;
 import org.testng.annotations.AfterClass;
-import org.testng.annotations.AfterMethod;
 import org.testng.annotations.AfterSuite;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.BeforeSuite;
 
 public class ZkTestBase {
-  private static Logger LOG = LoggerFactory.getLogger(ZkTestBase.class);
+  private static final Logger LOG = LoggerFactory.getLogger(ZkTestBase.class);
   private static final String MULTI_ZK_PROPERTY_KEY = "multiZk";
   private static final String NUM_ZK_PROPERTY_KEY = "numZk";
 
@@ -108,7 +98,7 @@ public class ZkTestBase {
   protected static BaseDataAccessor<ZNRecord> _baseAccessor;
   protected static MBeanServerConnection _server = ManagementFactory.getPlatformMBeanServer();
 
-  private Map<String, Map<String, HelixZkClient>> _liveInstanceOwners = new HashMap<>();
+  private final Map<String, Map<String, HelixZkClient>> _liveInstanceOwners = new HashMap<>();
 
   private static final String ZK_PREFIX = "localhost:";
   private static final int ZK_START_PORT = 2183;
@@ -123,10 +113,10 @@ public class ZkTestBase {
    * Multiple ZK references
    */
   // The following maps hold ZK connect string as keys
-  protected Map<String, ZkServer> _zkServerMap = new HashMap<>();
-  protected Map<String, HelixZkClient> _helixZkClientMap = new HashMap<>();
-  protected Map<String, ClusterSetup> _clusterSetupMap = new HashMap<>();
-  protected Map<String, BaseDataAccessor> _baseDataAccessorMap = new HashMap<>();
+  protected static final Map<String, ZkServer> _zkServerMap = new HashMap<>();
+  protected static final Map<String, HelixZkClient> _helixZkClientMap = new HashMap<>();
+  protected static final Map<String, ClusterSetup> _clusterSetupMap = new HashMap<>();
+  protected static final Map<String, BaseDataAccessor> _baseDataAccessorMap = new HashMap<>();
 
   static public void reportPhysicalMemory() {
     com.sun.management.OperatingSystemMXBean os = (com.sun.management.OperatingSystemMXBean)
@@ -143,7 +133,6 @@ public class ZkTestBase {
 
   @BeforeSuite
   public void beforeSuite() throws Exception {
-    reportPhysicalMemory();
     // TODO: use logging.properties file to config java.util.logging.Logger levels
     java.util.logging.Logger topJavaLogger = java.util.logging.Logger.getLogger("");
     topJavaLogger.setLevel(Level.WARNING);
@@ -195,22 +184,27 @@ public class ZkTestBase {
    * @param i index to be added to the ZK port to avoid conflicts
    * @throws Exception
    */
-  private void startZooKeeper(int i)
-      throws Exception {
+  private static synchronized void startZooKeeper(int i) {
     String zkAddress = ZK_PREFIX + (ZK_START_PORT + i);
-    ZkServer zkServer = TestHelper.startZkServer(zkAddress);
-    AssertJUnit.assertNotNull(zkServer);
+    _zkServerMap.computeIfAbsent(zkAddress, ZkTestBase::createZookeeperServer);
+    _helixZkClientMap.computeIfAbsent(zkAddress, ZkTestBase::createZkClient);
+    _clusterSetupMap.computeIfAbsent(zkAddress, key -> new ClusterSetup(_helixZkClientMap.get(key)));
+    _baseDataAccessorMap.computeIfAbsent(zkAddress, key -> new ZkBaseDataAccessor(_helixZkClientMap.get(key)));
+  }
+
+  private static ZkServer createZookeeperServer(String zkAddress) {
+    try {
+      return Preconditions.checkNotNull(TestHelper.startZkServer(zkAddress));
+    } catch (Exception e) {
+      throw new IllegalArgumentException("Failed to start zookeeper server at " + zkAddress, e);
+    }
+  }
+
+  private static HelixZkClient createZkClient(String zkAddress) {
     HelixZkClient.ZkClientConfig clientConfig = new HelixZkClient.ZkClientConfig();
     clientConfig.setZkSerializer(new ZNRecordSerializer());
-    HelixZkClient zkClient = DedicatedZkClientFactory.getInstance()
+    return DedicatedZkClientFactory.getInstance()
         .buildZkClient(new HelixZkClient.ZkConnectionConfig(zkAddress), clientConfig);
-    ClusterSetup gSetupTool = new ClusterSetup(zkClient);
-    BaseDataAccessor baseDataAccessor = new ZkBaseDataAccessor<>(zkClient);
-
-    _zkServerMap.put(zkAddress, zkServer);
-    _helixZkClientMap.put(zkAddress, zkClient);
-    _clusterSetupMap.put(zkAddress, gSetupTool);
-    _baseDataAccessorMap.put(zkAddress, baseDataAccessor);
   }
 
   @AfterSuite
@@ -224,17 +218,17 @@ public class ZkTestBase {
       }
     }
 
-    // Close all ZK resources
-    _baseDataAccessorMap.values().forEach(BaseDataAccessor::close);
-    _clusterSetupMap.values().forEach(ClusterSetup::close);
-    _helixZkClientMap.values().forEach(HelixZkClient::close);
-    _zkServerMap.values().forEach(TestHelper::stopZkServer);
+    synchronized (ZkTestBase.class) {
+      // Close all ZK resources
+      _baseDataAccessorMap.values().forEach(BaseDataAccessor::close);
+      _clusterSetupMap.values().forEach(ClusterSetup::close);
+      _helixZkClientMap.values().forEach(HelixZkClient::close);
+      _zkServerMap.values().forEach(TestHelper::stopZkServer);
+    }
   }
 
   @BeforeClass
   public void beforeClass() throws Exception {
-    String testClassName = this.getShortClassName();
-    System.out.println("BeforeClass: " + testClassName + " called.");
     cleanupJMXObjects();
     // Giving each test some time to settle (such as gc pause, etc).
     // Note that this is the best effort we could make to stabilize tests, not a complete solution
@@ -244,19 +238,9 @@ public class ZkTestBase {
 
   @BeforeMethod
   public void beforeTest(Method testMethod, ITestContext testContext) {
-    long startTime = System.currentTimeMillis();
-    System.out.println("START " + getShortClassName() + " " + testMethod.getName() + " at " + new Date(startTime));
     testContext.setAttribute("StartTime", System.currentTimeMillis());
   }
 
-  @AfterMethod
-  public void endTest(Method testMethod, ITestContext testContext) {
-    Long startTime = (Long) testContext.getAttribute("StartTime");
-    long endTime = System.currentTimeMillis();
-    System.out.println("END " + getShortClassName() + " " + testMethod.getName() + " at " + new Date(endTime) + ", took: "
-        + (endTime - startTime) + "ms.");
-  }
-
   protected void cleanupJMXObjects() throws IOException {
     // Clean up all JMX objects
     for (ObjectName mbean : _server.queryNames(null, null)) {
@@ -284,47 +268,6 @@ public class ZkTestBase {
     return leader.getInstanceName();
   }
 
-  protected void stopCurrentLeader(HelixZkClient zkClient, String clusterName,
-      Map<String, Thread> threadMap, Map<String, HelixManager> managerMap) {
-    String leader = getCurrentLeader(zkClient, clusterName);
-    Assert.assertTrue(leader != null);
-    System.out.println("stop leader:" + leader + " in " + clusterName);
-    Assert.assertTrue(leader != null);
-
-    HelixManager manager = managerMap.remove(leader);
-    Assert.assertTrue(manager != null);
-    manager.disconnect();
-
-    Thread thread = threadMap.remove(leader);
-    Assert.assertTrue(thread != null);
-    thread.interrupt();
-
-    boolean isNewLeaderElected = false;
-    try {
-      // Thread.sleep(2000);
-      for (int i = 0; i < 5; i++) {
-        Thread.sleep(1000);
-        String newLeader = getCurrentLeader(zkClient, clusterName);
-        if (!newLeader.equals(leader)) {
-          isNewLeaderElected = true;
-          System.out.println("new leader elected: " + newLeader + " in " + clusterName);
-          break;
-        }
-      }
-    } catch (InterruptedException e) {
-      e.printStackTrace();
-    }
-    if (!isNewLeaderElected) {
-      System.out.println("fail to elect a new leader elected in " + clusterName);
-    }
-    AssertJUnit.assertTrue(isNewLeaderElected);
-  }
-
-  protected void enableHealthCheck(String clusterName) {
-    ConfigScope scope = new ConfigScopeBuilder().forCluster(clusterName).build();
-    new ConfigAccessor(_gZkClient).set(scope, "healthChange" + ".enabled", "" + true);
-  }
-
   protected void enablePersistBestPossibleAssignment(HelixZkClient zkClient, String clusterName,
       Boolean enabled) {
     ConfigAccessor configAccessor = new ConfigAccessor(zkClient);
@@ -560,14 +503,14 @@ public class ZkTestBase {
     String instanceConfigsPath = PropertyPathBuilder.instanceConfig(clusterName);
     String instanceConfigPath = instanceConfigsPath + "/" + instance;
     String instancePath = PropertyPathBuilder.instance(clusterName, instance);
-    AssertJUnit.assertEquals(wantExists, zkClient.exists(instanceConfigPath));
-    AssertJUnit.assertEquals(wantExists, zkClient.exists(instancePath));
+    Assert.assertEquals(wantExists, zkClient.exists(instanceConfigPath));
+    Assert.assertEquals(wantExists, zkClient.exists(instancePath));
   }
 
   public void verifyResource(HelixZkClient zkClient, String clusterName, String resource,
       boolean wantExists) {
     String resourcePath = PropertyPathBuilder.idealState(clusterName, resource);
-    AssertJUnit.assertEquals(wantExists, zkClient.exists(resourcePath));
+    Assert.assertEquals(wantExists, zkClient.exists(resourcePath));
   }
 
   public void verifyEnabled(HelixZkClient zkClient, String clusterName, String instance,
@@ -577,7 +520,7 @@ public class ZkTestBase {
     Builder keyBuilder = accessor.keyBuilder();
 
     InstanceConfig config = accessor.getProperty(keyBuilder.instanceConfig(instance));
-    AssertJUnit.assertEquals(wantEnabled, config.getInstanceEnabled());
+    Assert.assertEquals(wantEnabled, config.getInstanceEnabled());
   }
 
   public void verifyReplication(HelixZkClient zkClient, String clusterName, String resource,
@@ -589,79 +532,13 @@ public class ZkTestBase {
     IdealState idealState = accessor.getProperty(keyBuilder.idealStates(resource));
     for (String partitionName : idealState.getPartitionSet()) {
       if (idealState.getRebalanceMode() == IdealState.RebalanceMode.SEMI_AUTO) {
-        AssertJUnit.assertEquals(repl, idealState.getPreferenceList(partitionName).size());
+        Assert.assertEquals(repl, idealState.getPreferenceList(partitionName).size());
       } else if (idealState.getRebalanceMode() == IdealState.RebalanceMode.CUSTOMIZED) {
-        AssertJUnit.assertEquals(repl, idealState.getInstanceStateMap(partitionName).size());
+        Assert.assertEquals(repl, idealState.getInstanceStateMap(partitionName).size());
       }
     }
   }
 
-  protected void simulateSessionExpiry(ZkConnection zkConnection)
-      throws IOException, InterruptedException {
-    ZooKeeper oldZookeeper = zkConnection.getZookeeper();
-    LOG.info("Old sessionId = " + oldZookeeper.getSessionId());
-
-    Watcher watcher = new Watcher() {
-      @Override
-      public void process(WatchedEvent event) {
-        LOG.info("In New connection, process event:" + event);
-      }
-    };
-
-    ZooKeeper newZookeeper =
-        new ZooKeeper(zkConnection.getServers(), oldZookeeper.getSessionTimeout(), watcher,
-            oldZookeeper.getSessionId(), oldZookeeper.getSessionPasswd());
-    LOG.info("New sessionId = " + newZookeeper.getSessionId());
-    // Thread.sleep(3000);
-    newZookeeper.close();
-    Thread.sleep(10000);
-    oldZookeeper = zkConnection.getZookeeper();
-    LOG.info("After session expiry sessionId = " + oldZookeeper.getSessionId());
-  }
-
-  protected void simulateSessionExpiry(HelixZkClient client)
-      throws IOException, InterruptedException, IOException {
-    ZkClient zkClient = (ZkClient) client;
-
-    IZkStateListener listener = new IZkStateListener() {
-      @Override
-      public void handleStateChanged(Watcher.Event.KeeperState state) throws Exception {
-        LOG.info("In Old connection, state changed:" + state);
-      }
-
-      @Override
-      public void handleNewSession(final String sessionId) throws Exception {
-        LOG.info("In Old connection, new session: {}.", sessionId);
-      }
-
-      @Override
-      public void handleSessionEstablishmentError(Throwable var1) throws Exception {
-      }
-    };
-    zkClient.subscribeStateChanges(listener);
-    ZkConnection connection = ((ZkConnection) zkClient.getConnection());
-    ZooKeeper oldZookeeper = connection.getZookeeper();
-    LOG.info("Old sessionId = " + oldZookeeper.getSessionId());
-
-    Watcher watcher = new Watcher() {
-      @Override
-      public void process(WatchedEvent event) {
-        LOG.info("In New connection, process event:" + event);
-      }
-    };
-
-    ZooKeeper newZookeeper =
-        new ZooKeeper(connection.getServers(), oldZookeeper.getSessionTimeout(), watcher,
-            oldZookeeper.getSessionId(), oldZookeeper.getSessionPasswd());
-    LOG.info("New sessionId = " + newZookeeper.getSessionId());
-    // Thread.sleep(3000);
-    newZookeeper.close();
-    Thread.sleep(10000);
-    connection = (ZkConnection) zkClient.getConnection();
-    oldZookeeper = connection.getZookeeper();
-    LOG.info("After session expiry sessionId = " + oldZookeeper.getSessionId());
-  }
-
   protected void setupStateModel(String clusterName) {
     ZKHelixDataAccessor accessor =
         new ZKHelixDataAccessor(clusterName, new ZkBaseDataAccessor<>(_gZkClient));
@@ -738,17 +615,6 @@ public class ZkTestBase {
       clientMap.clear();
     }
     _liveInstanceOwners.clear();
-
-    boolean status = false;
-    try {
-      status = ThreadLeakageChecker.afterClassCheck(testClassName);
-    } catch (Exception e) {
-      LOG.error("ThreadLeakageChecker exception:", e);
-    }
-    // todo: We should fail test here once we achieved 0 leakage and remove the following System print
-    if (!status) {
-      System.out.println("---------- Test Class " + testClassName + " thread leakage detected! ---------------");
-    }
   }
 
   protected List<LiveInstance> setupLiveInstances(String clusterName, int[] liveInstances) {
@@ -948,7 +814,7 @@ public class ZkTestBase {
 
     @Override
     public ZkClient getZkClient() {
-      return (ZkClient) _gZkClient;
+      return (ZkClient) _zkClient;
     }
 
     @Override
diff --git a/helix-core/src/test/java/org/apache/helix/integration/manager/TestParticipantManager.java b/helix-core/src/test/java/org/apache/helix/integration/manager/TestParticipantManager.java
index 95d1f452c..ed23a05f9 100644
--- a/helix-core/src/test/java/org/apache/helix/integration/manager/TestParticipantManager.java
+++ b/helix-core/src/test/java/org/apache/helix/integration/manager/TestParticipantManager.java
@@ -81,7 +81,6 @@ public class TestParticipantManager extends ZkTestBase {
   @AfterMethod
   public void afterMethod(Method testMethod, ITestContext testContext) {
     deleteCluster(_clusterName);
-    super.endTest(testMethod, testContext);
   }
 
   @AfterClass
diff --git a/helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkConectionConfig.java b/helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkConectionConfig.java
index dca3d524a..f6e051814 100644
--- a/helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkConectionConfig.java
+++ b/helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkConectionConfig.java
@@ -39,7 +39,6 @@ import org.apache.helix.HelixManagerProperty;
 import org.apache.helix.InstanceType;
 import org.apache.helix.SystemPropertyKeys;
 import org.apache.helix.TestHelper;
-import org.apache.helix.ThreadLeakageChecker;
 import org.apache.helix.cloud.constants.CloudProvider;
 import org.apache.helix.integration.manager.ClusterControllerManager;
 import org.apache.helix.integration.manager.MockParticipantManager;
@@ -80,7 +79,7 @@ import org.testng.annotations.Test;
  * in system property
  */
 public class TestMultiZkConectionConfig {
-  private static Logger LOG = LoggerFactory.getLogger(TestMultiZkHelixJavaApis.class);
+  private static Logger LOG = LoggerFactory.getLogger(TestMultiZkConectionConfig.class);
   private static final int NUM_ZK = 3;
   private static final Map<String, ZkServer> ZK_SERVER_MAP = new HashMap<>();
   private static final Map<String, HelixZkClient> ZK_CLIENT_MAP = new HashMap<>();
@@ -89,10 +88,6 @@ public class TestMultiZkConectionConfig {
   private static final List<String> CLUSTER_LIST =
       ImmutableList.of("CLUSTER_1", "CLUSTER_2", "CLUSTER_3");
 
-  // For testing different MSDS endpoint configs.
-  private static final String CLUSTER_ONE = CLUSTER_LIST.get(0);
-  private static final String CLUSTER_FOUR = "CLUSTER_4";
-
   private MockMetadataStoreDirectoryServer _msds;
   private static final Map<String, Collection<String>> _rawRoutingData = new HashMap<>();
   private RealmAwareZkClient _zkClient;
@@ -102,7 +97,7 @@ public class TestMultiZkConectionConfig {
   private final Map<String, String> _configStore = new HashMap<>();
 
   private static final String ZK_PREFIX = "localhost:";
-  private static final int ZK_START_PORT = 8777;
+  private static final int ZK_START_PORT = 8977;
   private String _msdsEndpoint;
 
   @BeforeClass
@@ -225,18 +220,6 @@ public class TestMultiZkConectionConfig {
         System.clearProperty(MetadataStoreRoutingConstants.MSDS_SERVER_ENDPOINT_KEY);
       }
     }
-
-    boolean status = false;
-    try {
-      status = ThreadLeakageChecker.afterClassCheck(testClassName);
-    } catch (Exception e) {
-      LOG.error("ThreadLeakageChecker exception:", e);
-    }
-    // todo: We should fail test here once we achieved 0 leakage and remove the following System print
-    if (!status) {
-      System.out.println(
-          "---------- Test Class " + testClassName + " thread leakage detected! ---------------");
-    }
   }
 
   /**
diff --git a/helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkHelixJavaApis.java b/helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkHelixJavaApis.java
index 62317ddae..32419a4f3 100644
--- a/helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkHelixJavaApis.java
+++ b/helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkHelixJavaApis.java
@@ -45,7 +45,6 @@ import org.apache.helix.HelixManagerProperty;
 import org.apache.helix.InstanceType;
 import org.apache.helix.SystemPropertyKeys;
 import org.apache.helix.TestHelper;
-import org.apache.helix.ThreadLeakageChecker;
 import org.apache.helix.api.config.RebalanceConfig;
 import org.apache.helix.cloud.constants.CloudProvider;
 import org.apache.helix.controller.rebalancer.DelayedAutoRebalancer;
@@ -237,18 +236,6 @@ public class TestMultiZkHelixJavaApis {
         System.clearProperty(MetadataStoreRoutingConstants.MSDS_SERVER_ENDPOINT_KEY);
       }
     }
-
-    boolean status = false;
-    try {
-      status = ThreadLeakageChecker.afterClassCheck(testClassName);
-    } catch (Exception e) {
-      LOG.error("ThreadLeakageChecker exception:", e);
-    }
-    // todo: We should fail test here once we achieved 0 leakage and remove the following System print
-    if (!status) {
-      System.out.println(
-          "---------- Test Class " + testClassName + " thread leakage detected! ---------------");
-    }
   }
 
   /**
@@ -716,7 +703,7 @@ public class TestMultiZkHelixJavaApis {
   public void testGenericBaseDataAccessorBuilder() {
     // Custom session timeout value is used to count active connections in SharedZkClientFactory
     int customSessionTimeout = 10000;
-    String firstZkAddress = "localhost:8777"; // has "CLUSTER_1"
+    String firstZkAddress = ZK_PREFIX + ZK_START_PORT; // has "CLUSTER_1"
     String firstClusterPath = "/CLUSTER_1";
     String secondClusterPath = "/CLUSTER_2";
     ZkBaseDataAccessor.Builder<ZNRecord> zkBaseDataAccessorBuilder =
diff --git a/helix-core/src/test/java/org/apache/helix/integration/paticipant/TestInstanceAutoJoin.java b/helix-core/src/test/java/org/apache/helix/integration/paticipant/TestInstanceAutoJoin.java
index 7d062569f..0dd96e0ac 100644
--- a/helix-core/src/test/java/org/apache/helix/integration/paticipant/TestInstanceAutoJoin.java
+++ b/helix-core/src/test/java/org/apache/helix/integration/paticipant/TestInstanceAutoJoin.java
@@ -57,8 +57,7 @@ public class TestInstanceAutoJoin extends ZkStandAloneCMTestBase {
 
     Thread.sleep(500);
     // Assert.assertFalse(result._thread.isAlive());
-    Assert.assertTrue(null == manager.getHelixDataAccessor()
-        .getProperty(accessor.keyBuilder().liveInstance(instance2)));
+    Assert.assertNull(manager.getHelixDataAccessor().getProperty(accessor.keyBuilder().liveInstance(instance2)));
 
     ConfigScope scope = new ConfigScopeBuilder().forCluster(CLUSTER_NAME).build();
 
@@ -76,8 +75,7 @@ public class TestInstanceAutoJoin extends ZkStandAloneCMTestBase {
       } else
         break;
     }
-    Assert.assertTrue(null != manager.getHelixDataAccessor()
-        .getProperty(accessor.keyBuilder().liveInstance(instance2)));
+    Assert.assertNotNull(manager.getHelixDataAccessor().getProperty(accessor.keyBuilder().liveInstance(instance2)));
 
     newParticipant.syncStop();
   }
@@ -122,8 +120,7 @@ public class TestInstanceAutoJoin extends ZkStandAloneCMTestBase {
         return true;
       }, 2000));
     } catch (HelixException e) {
-      Assert.assertTrue(null == manager.getHelixDataAccessor()
-          .getProperty(accessor.keyBuilder().liveInstance(instance3)));
+      Assert.assertNull(manager.getHelixDataAccessor().getProperty(accessor.keyBuilder().liveInstance(instance3)));
     }
 
     autoParticipant.syncStop();
diff --git a/helix-core/src/test/java/org/apache/helix/integration/task/TestForceDeleteWorkflow.java b/helix-core/src/test/java/org/apache/helix/integration/task/TestForceDeleteWorkflow.java
index 2ac9f519b..6065894eb 100644
--- a/helix-core/src/test/java/org/apache/helix/integration/task/TestForceDeleteWorkflow.java
+++ b/helix-core/src/test/java/org/apache/helix/integration/task/TestForceDeleteWorkflow.java
@@ -58,7 +58,6 @@ public class TestForceDeleteWorkflow extends TaskTestBase {
   private static final String LONG_EXECUTION_TIME = "100000";
   // Long delay to simulate the tasks that are stuck in Task.cancel().
   private static final String STOP_DELAY = "1000000";
-  private HelixAdmin _admin;
 
   // These AtomicIntegers are used to verify that the tasks are indeed stuck in Task.cancel().
   // CANCEL shows that the task cancellation process has been started. (Incremented at the beginning
@@ -96,8 +95,6 @@ public class TestForceDeleteWorkflow extends TaskTestBase {
           new TaskStateModelFactory(_participants[i], taskFactoryReg));
       _participants[i].syncStart();
     }
-
-    _admin = _gSetupTool.getClusterManagementTool();
   }
 
   @Test
@@ -316,9 +313,9 @@ public class TestForceDeleteWorkflow extends TaskTestBase {
   /**
    * A mock task that extents MockTask class and delays cancellation of the tasks.
    */
-  private class DelayedStopTask extends MockTask {
+  private static class DelayedStopTask extends MockTask {
     private static final String JOB_DELAY_CANCEL = "DelayCancel";
-    private long _delayCancel;
+    private final long _delayCancel;
 
     DelayedStopTask(TaskCallbackContext context) {
       super(context);
diff --git a/helix-core/src/test/java/org/apache/helix/manager/zk/TestZkBucketDataAccessor.java b/helix-core/src/test/java/org/apache/helix/manager/zk/TestZkBucketDataAccessor.java
index cef5d6bf4..8d6b83fc0 100644
--- a/helix-core/src/test/java/org/apache/helix/manager/zk/TestZkBucketDataAccessor.java
+++ b/helix-core/src/test/java/org/apache/helix/manager/zk/TestZkBucketDataAccessor.java
@@ -56,15 +56,14 @@ public class TestZkBucketDataAccessor extends ZkTestBase {
   private static final List<String> LIST_FIELD = ImmutableList.of("1", "2");
   private static final Map<String, String> MAP_FIELD = ImmutableMap.of("1", "2");
 
+  private final ZNRecord record = new ZNRecord(NAME_KEY);
+
   private BucketDataAccessor _bucketDataAccessor;
   private BaseDataAccessor<byte[]> _zkBaseDataAccessor;
 
-  private ZNRecord record = new ZNRecord(NAME_KEY);
-
   @BeforeClass
   public void beforeClass() {
     // Initialize ZK accessors for testing
-    _bucketDataAccessor = new ZkBucketDataAccessor(ZK_ADDR, 50 * 1024, VERSION_TTL_MS);
     HelixZkClient zkClient = DedicatedZkClientFactory.getInstance()
         .buildZkClient(new HelixZkClient.ZkConnectionConfig(ZK_ADDR));
     zkClient.setZkSerializer(new ZkSerializer() {
@@ -82,6 +81,7 @@ public class TestZkBucketDataAccessor extends ZkTestBase {
       }
     });
     _zkBaseDataAccessor = new ZkBaseDataAccessor<>(zkClient);
+    _bucketDataAccessor = new ZkBucketDataAccessor(zkClient, 50 * 1024, VERSION_TTL_MS);
 
     // Fill in some data for the record
     record.setSimpleField(NAME_KEY, NAME_KEY);
diff --git a/helix-core/src/test/java/org/apache/helix/tools/TestHelixAdminCli.java b/helix-core/src/test/java/org/apache/helix/tools/TestHelixAdminCli.java
index 0c272996a..ed270df09 100644
--- a/helix-core/src/test/java/org/apache/helix/tools/TestHelixAdminCli.java
+++ b/helix-core/src/test/java/org/apache/helix/tools/TestHelixAdminCli.java
@@ -62,7 +62,6 @@ public class TestHelixAdminCli extends ZkTestBase {
     } catch (Exception ignored) {
       // OK
     }
-    super.endTest(testMethod, testContext);
   }
 
   @Test
diff --git a/helix-view-aggregator/src/test/java/org/apache/helix/view/integration/TestHelixViewAggregator.java b/helix-view-aggregator/src/test/java/org/apache/helix/view/integration/TestHelixViewAggregator.java
index b98b6840e..96890463a 100644
--- a/helix-view-aggregator/src/test/java/org/apache/helix/view/integration/TestHelixViewAggregator.java
+++ b/helix-view-aggregator/src/test/java/org/apache/helix/view/integration/TestHelixViewAggregator.java
@@ -165,7 +165,7 @@ public class TestHelixViewAggregator extends ViewAggregatorIntegrationTestBase {
     _monitor.reset();
 
     // Modify view cluster config
-    _viewClusterRefreshPeriodSec = 3;
+    _viewClusterRefreshPeriodSec = 8;
     List<PropertyType> newProperties =
         new ArrayList<>(ViewClusterSourceConfig.getValidPropertyTypes());
     newProperties.remove(PropertyType.LIVEINSTANCES);
diff --git a/zookeeper-api/src/test/conf/testng.xml b/zookeeper-api/src/test/conf/testng.xml
index 15b368a02..abdce135c 100644
--- a/zookeeper-api/src/test/conf/testng.xml
+++ b/zookeeper-api/src/test/conf/testng.xml
@@ -18,8 +18,8 @@
   ~ under the License.
   -->
 <!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd">
-<suite name="Suite" parallel="false" preserve-order="true">
-  <test name="Test">
+<suite name="Suite" parallel="false" thread-count = "1" preserve-order="true">
+  <test name="Test" >
     <packages>
       <package name="org.apache.helix.zookeeper.*"/>
     </packages>
diff --git a/zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/ZkTestBase.java b/zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/ZkTestBase.java
index eff561016..ef4e02ff3 100644
--- a/zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/ZkTestBase.java
+++ b/zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/ZkTestBase.java
@@ -22,9 +22,9 @@ package org.apache.helix.zookeeper.impl;
 import java.io.File;
 import java.io.IOException;
 import java.lang.management.ManagementFactory;
-import java.util.HashMap;
 import java.util.Map;
 
+import java.util.concurrent.ConcurrentHashMap;
 import javax.management.MBeanServerConnection;
 import javax.management.ObjectName;
 
@@ -59,7 +59,7 @@ public class ZkTestBase {
    * Multiple ZK references
    */
   // The following maps hold ZK connect string as keys
-  protected static final Map<String, ZkServer> _zkServerMap = new HashMap<>();
+  protected static final Map<String, ZkServer> _zkServerMap = new ConcurrentHashMap<>();
   protected static int _numZk = 1; // Initial value
 
   @BeforeSuite
@@ -114,8 +114,7 @@ public class ZkTestBase {
     // Start "numZkFromConfigInt" ZooKeepers
     for (int i = 0; i < _numZk; i++) {
       String zkAddress = ZK_PREFIX + (ZK_START_PORT + i);
-      ZkServer zkServer = startZkServer(zkAddress);
-      _zkServerMap.put(zkAddress, zkServer);
+      _zkServerMap.computeIfAbsent(zkAddress, ZkTestBase::startZkServer);
     }
   }
 
@@ -124,7 +123,7 @@ public class ZkTestBase {
    * @param zkAddress
    * @return
    */
-  protected ZkServer startZkServer(final String zkAddress) {
+  protected synchronized static ZkServer startZkServer(final String zkAddress) {
     String zkDir = zkAddress.replace(':', '_');
     final String logDir = "/tmp/" + zkDir + "/logs";
     final String dataDir = "/tmp/" + zkDir + "/dataDir";
diff --git a/zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/client/TestSharedZkClient.java b/zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/client/TestSharedZkClient.java
index 08ec790e3..c8b251964 100644
--- a/zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/client/TestSharedZkClient.java
+++ b/zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/client/TestSharedZkClient.java
@@ -40,7 +40,7 @@ public class TestSharedZkClient extends RealmAwareZkClientFactoryTestBase {
     _realmAwareZkClientFactory = SharedZkClientFactory.getInstance();
   }
 
-  @Test
+  @Test(dependsOnMethods = "testRealmAwareZkClientCreation")
   public void testCreateEphemeralFailure() {
     _realmAwareZkClient.setZkSerializer(new ZNRecordSerializer());