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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
