You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2021/04/28 00:09:24 UTC

[GitHub] [helix] jiajunwang commented on a change in pull request #1716: Fix unstable test TestZkCallbackHandlerLeak.

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



##########
File path: helix-core/src/test/java/org/apache/helix/integration/TestZkCallbackHandlerLeak.java
##########
@@ -162,33 +154,25 @@ public boolean verify() throws Exception {
     Assert.assertTrue(result);
 
     // check controller zk-watchers
-    result = TestHelper.verify(new TestHelper.Verifier() {
-
-      @Override
-      public boolean verify() throws Exception {
-        Map<String, Set<String>> watchers = ZkTestHelper.getListenersBySession(ZK_ADDR);
-        Set<String> watchPaths = watchers.get("0x" + controller.getSessionId());
-        // System.out.println("controller watch paths after session expiry: " + watchPaths);
-
-        // where n is number of nodes and r is number of resources
-        // one participant is disconnected, and its task current states are removed
-        return watchPaths.size() == (8 + r + (6 + r + taskResourceCount) * (n - 1) + 6 + r);
-      }
+    result = TestHelper.verify(() -> {
+      Map<String, Set<String>> watchers = ZkTestHelper.getListenersBySession(ZK_ADDR);
+      Set<String> watchPaths = watchers.get("0x" + controller.getSessionId());
+      // System.out.println("controller watch paths after session expiry: " + watchPaths);

Review comment:
       Removed

##########
File path: helix-core/src/test/java/org/apache/helix/integration/TestZkCallbackHandlerLeak.java
##########
@@ -494,12 +466,12 @@ public void testCurrentStatePathLeakingByAsycRemoval() throws Exception {
     }
 
     // verify new watcher is installed on the new node
-    boolean result = TestHelper.verify(() -> {
-      return ZkTestHelper.getListenersByZkPath(ZK_ADDR).keySet().contains(jobKey.getPath());
-    }, TestHelper.WAIT_DURATION);
-    Assert.assertTrue(result, "Should get initial clusterConfig callback invoked");
-    rpWatchPaths = ZkTestHelper.getZkWatch(rpManager.getZkClient());
-    Assert.assertTrue(rpWatchPaths.get("dataWatches").contains(jobKey.getPath()));
+    boolean result = TestHelper.verify(
+        () -> ZkTestHelper.getListenersByZkPath(ZK_ADDR).keySet().contains(jobKey.getPath())
+            && ZkTestHelper.getZkWatch(rpManager.getZkClient()).get("dataWatches")
+            .contains(jobKey.getPath()), TestHelper.WAIT_DURATION);
+    Assert.assertTrue(result,
+        "Should get initial clusterConfig callback invoked and add data watchers");
 
     LOG.info("remove job");

Review comment:
       Yeah, I think it is not necessary. But I will leave it for now since it is not related to this PR change.

##########
File path: helix-core/src/test/java/org/apache/helix/integration/TestZkCallbackHandlerLeak.java
##########
@@ -106,33 +106,25 @@ public void testCbHandlerLeakOnParticipantSessionExpiry() throws Exception {
     final MockParticipantManager participantManagerToExpire = participants[1];
 
     // check controller zk-watchers
-    boolean result = TestHelper.verify(new TestHelper.Verifier() {
-
-      @Override
-      public boolean verify() throws Exception {
-        Map<String, Set<String>> watchers = ZkTestHelper.getListenersBySession(ZK_ADDR);
-        // Set<String> watchPaths = watchers.get("0x" + controllerManager.getSessionId());
-        Set<String> watchPaths = watchers.get("0x" + controller.getSessionId());
-        // System.out.println("controller watch paths: " + watchPaths);
+    boolean result = TestHelper.verify(() -> {
+      Map<String, Set<String>> watchers = ZkTestHelper.getListenersBySession(ZK_ADDR);
+      // Set<String> watchPaths = watchers.get("0x" + controllerManager.getSessionId());

Review comment:
       Removed




-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org