ZOOKEEPER-4475: Fix NodeChildrenChanged delivered to recursive watcher (#1820)

The semantics of persistent recursive watch promise no child events on
descendant nodes. When there are standard child watches on descendants
of node being watches in persistent recursive mode, server will deliver
child events to client inevitably. So we have to filter out child events
for persistent recursive watches on client side.

2 files changed, 32 insertions, 4 deletions

diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ZKWatchManager.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ZKWatchManager.java
index 95a07f0e7..9da424944 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/ZKWatchManager.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ZKWatchManager.java
@@ -404,13 +404,13 @@ class ZKWatchManager implements ClientWatchManager {
             synchronized (existWatches) {
                 addTo(existWatches.remove(clientPath), result);
             }
-            addPersistentWatches(clientPath, result);
+            addPersistentWatches(clientPath, type, result);
             break;
         case NodeChildrenChanged:
             synchronized (childWatches) {
                 addTo(childWatches.remove(clientPath), result);
             }
-            addPersistentWatches(clientPath, result);
+            addPersistentWatches(clientPath, type, result);
             break;
         case NodeDeleted:
             synchronized (dataWatches) {
@@ -427,7 +427,7 @@ class ZKWatchManager implements ClientWatchManager {
             synchronized (childWatches) {
                 addTo(childWatches.remove(clientPath), result);
             }
-            addPersistentWatches(clientPath, result);
+            addPersistentWatches(clientPath, type, result);
             break;
         default:
             String errorMsg = String.format(
@@ -442,10 +442,17 @@ class ZKWatchManager implements ClientWatchManager {
         return result;
     }
 
-    private void addPersistentWatches(String clientPath, Set<Watcher> result) {
+    private void addPersistentWatches(String clientPath, Watcher.Event.EventType type, Set<Watcher> result) {
         synchronized (persistentWatches) {
             addTo(persistentWatches.get(clientPath), result);
         }
+        // The semantics of persistent recursive watch promise no child events on descendant nodes. When there
+        // are standard child watches on descendants of node being watched in persistent recursive mode, server
+        // will deliver child events to client inevitably. So we have to filter out child events for persistent
+        // recursive watches on client side.
+        if (type == Watcher.Event.EventType.NodeChildrenChanged) {
+            return;
+        }
         synchronized (persistentRecursiveWatches) {
             for (String path : PathParentIterator.forAll(clientPath).asIterable()) {
                 addTo(persistentRecursiveWatches.get(path), result);
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/PersistentRecursiveWatcherTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/PersistentRecursiveWatcherTest.java
index 80d8c400c..e74ee2fd6 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/PersistentRecursiveWatcherTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/PersistentRecursiveWatcherTest.java
@@ -115,6 +115,27 @@ public class PersistentRecursiveWatcherTest extends ClientBase {
     }
 
     @Test
+    public void testNoChildEvents() throws Exception {
+        try (ZooKeeper zk = createClient()) {
+            zk.create("/a", new byte[0], ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
+
+            zk.addWatch("/", persistentWatcher, PERSISTENT_RECURSIVE);
+
+            BlockingQueue<WatchedEvent> childEvents = new LinkedBlockingQueue<>();
+            zk.getChildren("/a", childEvents::add);
+
+            zk.create("/a/b", new byte[0], ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
+            zk.create("/a/b/c", new byte[0], ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
+
+            assertEvent(childEvents, Watcher.Event.EventType.NodeChildrenChanged, "/a");
+
+            assertEvent(events, Watcher.Event.EventType.NodeCreated, "/a/b");
+            assertEvent(events, Watcher.Event.EventType.NodeCreated, "/a/b/c");
+            assertTrue(events.isEmpty());
+        }
+    }
+
+    @Test
     public void testDisconnect() throws Exception {
         try (ZooKeeper zk = createClient(new CountdownWatcher(), hostPort)) {
             zk.addWatch("/a/b", persistentWatcher, PERSISTENT_RECURSIVE);