apache / helix

Mirror of Apache Helix
Apache License 2.0
461 stars 224 forks source link

Fix flaky TestClusterStateVerifier due to local variable reference which can be GC-ed #1266

Closed kaisun2000 closed 3 years ago

kaisun2000 commented 4 years ago

TestClusterStateVerifier sometimes fails with log:

2020-08-08T10:12:24.8447598Z [ERROR] testResourceSubset(org.apache.helix.tools.TestClusterStateVerifier) Time elapsed: 1.012 s <<< FAILURE! 2020-08-08T10:12:24.8448106Z org.apache.helix.HelixException: Failed to create pause signal 2020-08-08T10:12:24.8449142Z at org.apache.helix.tools.TestClusterStateVerifier.testResourceSubset(TestClusterStateVerifier.java:115) 2020-08-08T10:12:24.8449351Z 2020-08-08T10:12:24.8450141Z [ERROR] afterMethod(org.apache.helix.tools.TestClusterStateVerifier) Time elapsed: 1.036 s <<< FAILURE! 2020-08-08T10:12:24.8450566Z java.lang.IllegalStateException: ZkClient already closed! 2020-08-08T10:12:24.8451566Z at org.apache.helix.tools.TestClusterStateVerifier.afterMethod(TestClusterStateVerifier.java:98)

Reason: _admin refers to zkClient in local variable. The local variable setupTool later gets GCed and finalizer would call "close()" which shuts down the zkClient. Thus, later when _admin is used, it would throw exception, which fail the test at a not assert location.

 @BeforeMethod
  public void beforeMethod() throws InterruptedException {
    final int NUM_PARTITIONS = 1;
    final int NUM_REPLICAS = 1;

    // Cluster and resource setup
    String className = TestHelper.getTestClassName();
    String methodName = TestHelper.getTestMethodName();
    _clusterName = className + "_" + methodName;
    ClusterSetup setupTool = new ClusterSetup(ZK_ADDR);
    _admin = setupTool.getClusterManagementTool();
    setupTool.addCluster(_clusterName, true);
    setupTool.addResourceToCluster(_clusterName, RESOURCES[0], NUM_PARTITIONS, "OnlineOffline",
        RebalanceMode.SEMI_AUTO.toString());
    setupTool.addResourceToCluster(_clusterName, RESOURCES[1], NUM_PARTITIONS, "OnlineOffline",
        RebalanceMode.SEMI_AUTO.toString());
kaisun2000 commented 4 years ago

fixed by #1227

jiajunwang commented 3 years ago

Close test unstable tickets since we have an automatic tracking mechanism https://github.com/apache/helix/pull/1757 now for tracking the most recent test issues.