[GitHub] [helix] pkuwm commented on a change in pull request #1295: fix TestRawZkClient unstableness

[GitBox <gi...@apache.org>]

pkuwm commented on a change in pull request #1295:
URL: https://github.com/apache/helix/pull/1295#discussion_r474338627



##########
File path: zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
##########
@@ -2156,6 +2160,21 @@ public void connect(final long maxMsToWaitUntilConnected, Watcher watcher)
 
       LOG.debug("ZkClient created with _uid {}, _eventThread {}", _uid, _eventThread.getId());
 
+      // initiate monitor
+      try {
+        if (_monitorKey != null && !_monitorKey.isEmpty() && _monitorType != null && !_monitorType
+            .isEmpty()) {
+          _monitor =
+              new ZkClientMonitor(_monitorType, _monitorKey, _monitorInstanceName, _monitorRootPathOnly,

Review comment:
       We should consider closing the monitor to avoid leakage. If the later part of code zk connection timeout and throws exception, the monitor should also be closed.

##########
File path: zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/TestHelper.java
##########
@@ -32,7 +32,7 @@
 
 public class TestHelper {
   private static final Logger LOG = LoggerFactory.getLogger(TestHelper.class);
-  public static final long WAIT_DURATION = 20 * 1000L; // 20 seconds
+  public static final long WAIT_DURATION = 60 * 1000L; // 20 seconds

Review comment:
       Does the test really need 60s to poll the result? In my opinion it is too long. It would fail with 20s?




----------------------------------------------------------------
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