[ZOOKEEPER-4466] Support different watch modes on same path

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 3.6.3, 3.6.4
• Fix Version/s: 3.9.0
• Component/s: java client, server
• Labels: pull-request-available

Release Note

Previously, there is only one watch mode per path and session. Later watch will ruin existing watch. For example, getData after AddWatchMode::Persistent will result in one-time data watch and persistent children watch. This is not our intend and it fears clients from arbitrary watch. So we decide to support multiple different watch modes on one path in one session. This way, users of client can watch whatever paths in whatever modes. They will not ruin each other.

• Assignee: Kezhu Wang
• Reporter: Kezhu Wang
• Created: 11/Feb/22 04:09
• Updated: 09/Oct/23 11:21
• Resolved: 05/May/23 07:59

Description

I used to think watchers of different modes are orthogonal. I found there are not, when I wrote tests for unfinished rust client. And I wrote test cases in java and confirmed.

I copied test case here for evaluation. You also clone from my fork.

    // zookeeper-server/src/test/java/org/apache/zookeeper/test/PersistentRecursiveWatcherTest.java

    @Test
    public void testPathOverlapWithStandardWatcher() throws Exception {
        try (ZooKeeper zk = createClient(new CountdownWatcher(), hostPort)) {
            CountDownLatch nodeCreated = new CountDownLatch(1);
            zk.addWatch("/a", persistentWatcher, PERSISTENT_RECURSIVE);
            zk.exists("/a", event -> nodeCreated.countDown());

            zk.create("/a", new byte[0], ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
            zk.create("/a/b", new byte[0], ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
            zk.delete("/a/b", -1);
            zk.delete("/a", -1);

            assertEvent(events, Watcher.Event.EventType.NodeCreated, "/a");
            assertEvent(events, Watcher.Event.EventType.NodeCreated, "/a/b");
            assertEvent(events, Watcher.Event.EventType.NodeDeleted, "/a/b");
            assertEvent(events, Watcher.Event.EventType.NodeDeleted, "/a");

            assertTrue(nodeCreated.await(5, TimeUnit.SECONDS));
        }
    }

    @Test
    public void testPathOverlapWithPersistentWatcher() throws Exception {
        try (ZooKeeper zk = createClient(new CountdownWatcher(), hostPort)) {
            zk.addWatch("/a", persistentWatcher, PERSISTENT_RECURSIVE);
            zk.addWatch("/a/b", event -> {}, PERSISTENT);
            zk.create("/a", new byte[0], ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
            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);
            zk.delete("/a/b/c", -1);
            zk.delete("/a/b", -1);
            zk.delete("/a", -1);
            assertEvent(events, Watcher.Event.EventType.NodeCreated, "/a");
            assertEvent(events, Watcher.Event.EventType.NodeCreated, "/a/b");
            assertEvent(events, Watcher.Event.EventType.NodeCreated, "/a/b/c");
            assertEvent(events, Watcher.Event.EventType.NodeDeleted, "/a/b/c");
            assertEvent(events, Watcher.Event.EventType.NodeDeleted, "/a/b");
            assertEvent(events, Watcher.Event.EventType.NodeDeleted, "/a");
        }
    }

I skimmed the code and found two possible causes:

1. ZKWatchManager.materialize materializes all persistent watchers(include recursive ones) for NodeChildrenChanged event.
2. WatcherModeManager trackes only one watcher mode.

Issue Links

• is related to: ZOOKEEPER-4472 Support persistent watchers removing individually
• links to: GitHub Pull Request #1859