jiajunwang commented on a change in pull request #1035:
URL: https://github.com/apache/helix/pull/1035#discussion_r435696914



##########
File path: 
helix-core/src/test/java/org/apache/helix/integration/TestZkCallbackHandlerLeak.java
##########
@@ -317,6 +324,150 @@ public boolean verify() throws Exception {
 
     System.out.println("END " + clusterName + " at " + new 
Date(System.currentTimeMillis()));
   }
+  @Test
+  public void testDanglingCallbackHanlderFix() throws Exception {
+    String className = TestHelper.getTestClassName();
+    String methodName = TestHelper.getTestMethodName();
+    String clusterName = className + "_" + methodName;
+    final int n = 3;
+    final String zkAddr = ZK_ADDR;
+    System.out.println("START " + clusterName + " at " + new 
Date(System.currentTimeMillis()));
+
+    TestHelper.setupCluster(clusterName, zkAddr, 12918, "localhost", "TestDB", 
1, // resource
+        32, // partitions
+        n, // nodes
+        2, // replicas
+        "MasterSlave", true);
+
+    final ClusterControllerManager controller =
+        new ClusterControllerManager(zkAddr, clusterName, "controller_0");
+    controller.syncStart();
+
+    MockParticipantManager[] participants = new MockParticipantManager[n];
+    for (int i = 0; i < n; i++) {
+      String instanceName = "localhost_" + (12918 + i);
+      participants[i] = new MockParticipantManager(zkAddr, clusterName, 
instanceName);
+      participants[i].syncStart();
+    }
+
+    Boolean result =
+        ClusterStateVerifier
+            .verifyByZkCallback(new 
ClusterStateVerifier.BestPossAndExtViewZkVerifier(zkAddr,
+                clusterName));
+    Assert.assertTrue(result);
+
+    //HelixManager rpManager  = HelixManagerFactory
+    //    .getZKHelixManager(clusterName, "", InstanceType.SPECTATOR, ZK_ADDR);
+    //rpManager.connect();
+    ClusterSpectatorManager rpManager  = new ClusterSpectatorManager(ZK_ADDR, 
clusterName, "router");
+    rpManager.syncStart();
+    RoutingTableProvider rp = new RoutingTableProvider(rpManager, 
PropertyType.CURRENTSTATES);
+
+    Thread.sleep(5000);

Review comment:
       @kaisun2000 unverifiable case does exist. But I'm not sure if here is 
the same case. I'm not reading the test code yet. Could you please clarify what 
it is sleeping for? Then I think I can give you some ideas.
   
   For example the later sleep, it seems you are waiting for new session 
established. In this case, instead of sleeping, you can use the verifier to 
polling until the session ID changed. Then you don't need to sleep for a 
hardcoded period.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to