Hi tison, Thank you for reviewing.
pr#1859 tries to support standard watches and persistent watches on same paths. It has no code conflicts with pr#1820, but test requirement on pr#1820. Assumes that: 1. Persistent watch (and/or child watch) on “/a” 2. Persistent recursive watch on “/a” Ideally, persistent watch and/or child watch should receive `NodeChildrenChanged` while persistent recursive watch should not. Without pr#1820 which filter out `NodeChildrenChanged` for persistent recursive watch in client side, test introduced in pr#1859 will fail. There are other followups, which are related to watcher removing, I have reported but blocked by pr#1859(aka. ZOOKEEPER-4466): * ZOOKEEPER-4471[1]: Remove WatcherType.Children break persistent watcher's child events * ZOOKEEPER-4472[2]: Support persistent watchers removing individually [1]: https://issues.apache.org/jira/browse/ZOOKEEPER-4471 [2]: https://issues.apache.org/jira/browse/ZOOKEEPER-4472 Best, Kezhu Wang On June 29, 2022 at 17:19:37, tison (wander4...@gmail.com) wrote: Thanks for your contribution Kezhu! I've reviewed PR-1820. It looks good to me. PR-1859 seems a followup of 1820, will review 1859 after 1820 get accepted. Best, tison. Kezhu Wang <kez...@gmail.com> 于2022年6月28日周二 23:17写道： > Hi guys, > > First, let me summarize changes of these two issues and associated prs > here. > > ZOOKEEPER-4475[1] reports that NodeChildrenChanged could be delivered to > persistent recursive watchers if a child watch is created on descendants of > node being watched using persistent recursive watch. pr#1820[2] solves this > by filtering out NodeChildrenChanged events for persistent recursive > watches on the client side. > > ZOOKEEPER-4466[3] reports that standard watch and persistent watch could > not coexist on same path. pr#1859[4] introduces WatchStats to count and > coexist different modes on same path. > > pr#1820 has been opened for a while but received no reviews. I think it is > pretty simple and solves a simple bug. It should take a long time to > review. > > For pr#1859, @eolivelli has given valuable comments. But both I and > @eolivelli think ZOOKEEPER-4466 deserves more attention. So, basically, we > need more reviewers to make sure pr#1859 goes in the right direction and > breaks no sensible codes. > > It would be appreciated if any reviewers could take a look at these prs. > > Best, > Kezhu Wang > > [1]: https://issues.apache.org/jira/browse/ZOOKEEPER-4475 > [2]: https://github.com/apache/zookeeper/pull/1820 > [3]: https://issues.apache.org/jira/browse/ZOOKEEPER-4466 > [4]: https://github.com/apache/zookeeper/pull/1859 >
