desaikomal commented on code in PR #2465:
URL: https://github.com/apache/helix/pull/2465#discussion_r1181662867
##########
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java:
##########
@@ -1360,6 +1360,13 @@ public List<String> getChildren(String path) {
}
protected List<String> getChildren(final String path, final boolean watch) {
+ // Throws exception when try to subscribe watch when using
_usePersistWatcher. When ZkClient
+ // is subscribed as persist watcher, resubscribing the same object as onw
time watcher will
+ // over write the persist watcher causing missing following event.
+ if (_usePersistWatcher && watch) {
Review Comment:
is this change in API contract? If not, we should be good.
##########
zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/TestZooKeeperConnection.java:
##########
@@ -0,0 +1,69 @@
+package org.apache.helix.zookeeper.impl;
+
+import java.io.IOException;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.helix.zookeeper.zkclient.IZkConnection;
+import org.apache.helix.zookeeper.zkclient.ZkClient;
+import org.apache.zookeeper.AddWatchMode;
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.WatchedEvent;
+import org.apache.zookeeper.Watcher;
+import org.apache.zookeeper.ZooDefs;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+
+public class TestZooKeeperConnection extends ZkTestBase {
+ final int count = 100;
+ final int[] get_count = {0};
+ CountDownLatch countDownLatch = new CountDownLatch(count*2);
+
+
+ /*
+ This function tests persist watchers' behavior in {@link
org.apache.helix.zookeeper.zkclient.ZkConnection}
+ 1. Register a persist watcher on a path and create 100 children Znode, edit
the ZNode for 100 times.
+ Expecting 200 events.
+ 2. register a one time listener on the path. Make the same change and count
the total number of event.
Review Comment:
shouldn't the one-time listener throw exception?
##########
zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/TestZooKeeperConnection.java:
##########
@@ -0,0 +1,69 @@
+package org.apache.helix.zookeeper.impl;
+
+import java.io.IOException;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.helix.zookeeper.zkclient.IZkConnection;
+import org.apache.helix.zookeeper.zkclient.ZkClient;
+import org.apache.zookeeper.AddWatchMode;
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.WatchedEvent;
+import org.apache.zookeeper.Watcher;
+import org.apache.zookeeper.ZooDefs;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+
+public class TestZooKeeperConnection extends ZkTestBase {
+ final int count = 100;
+ final int[] get_count = {0};
+ CountDownLatch countDownLatch = new CountDownLatch(count*2);
+
+
+ /*
+ This function tests persist watchers' behavior in {@link
org.apache.helix.zookeeper.zkclient.ZkConnection}
+ 1. Register a persist watcher on a path and create 100 children Znode, edit
the ZNode for 100 times.
+ Expecting 200 events.
+ 2. register a one time listener on the path. Make the same change and count
the total number of event.
+ */
+ @Test
+ void testPersistWatcher() throws Exception {
+ Watcher watcher1 = new PersistWatcher();
+ ZkClient zkClient = new
org.apache.helix.zookeeper.impl.client.ZkClient(ZK_ADDR);
+ IZkConnection _zk = zkClient.getConnection();
+ String path="/testPersistWatcher";
+ _zk.create(path, null, ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
+ // register a persist listener on a path, change the ZNode 100 times,
create 100 child ZNode,
+ // and expecting 200 events
+ _zk.addWatch(path, watcher1, AddWatchMode.PERSISTENT);
+ for (int i=0; i<count; ++i) {
Review Comment:
nit: just curious - style guide doesn't complain about spacing etc?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]