Hi all and committers,

[ZOOKEEPER-4471][1] reported that `AddWatchMode.PERSISTENT` could be
partially removed by `WatcherType.Data` or `WatcherType.Children`.

[ZOOKEEPER-4472][2] proposed to add `WatcherType.Persistent` and
`WatcherType.PersistentRecursive` to remove `AddWatchMode.PERSISTENT`
and `AddWatchMode.PERSISTENT_RECURSIVE` respectively.

[ZOOKEEPER-4466][3] rescues us from conflict among different watch
types on the same path. So, clients can watch whatever paths in
whatever modes. They will not ruin each other.

On the other hand, [ZOOKEEPER-4472][2] complements the removing part
of [ZOOKEEPER-4466][3]. If a client wants to remove a persistent
watch, it will issue a `removeWatches` `WatcherType.Persistent`. This
will not affect any other watcher types. Currently, clients have to
resort to `WatcherType.Any` to remove them. This could potentially
affect other ongoing watcher types.

Ideally, ZOOKEEPER-4472 is independent of ZOOKEEPER-4471, but it would
be hard to ship only ZOOKEEPER-4472 without a fix to ZOOKEEPER-4471.
That will require particular attention to avoid trigger paths for
ZOOKEEPER-4471. That is why I delayed it until now.

I saw discussions about [cut release for 3.9][4]. I really hope we can
merge [pr#1998][5] for ZOOKEEPER-4471 and [pr#2006][6] for
ZOOKEEPER-4472 in 3.9.0. It would be impossible for us to merge
ZOOKEEPER-4472 to patch versions of 3.9 series as it touches both
server logic and api side while ZOOKEEPER-4466 is shipped into 3.9. It
looks weird if we support different watcher types one same path in
watching but not all of them in removing. That is why I hope we can
ship them along with ZOOKEEPER-4466 in 3.9.0.

I plan to reply with a short message about this possibility in the
release discussion thread to ref back this mail. Hope it won't bore
you in there or here.

Anyway, please take your time to review pr#1998 and pr#2006 no matter
whether they will be included in 3.9.0.

Last, if you are curious how ZOOKEEPER-4472 could affect a real
program. You can take a look at [try_remove_watcher][7] and
[dispatch_path_event][8] in [zookeeper-rust-client][9]. The
`try_remove_watcher` ignores dropping of persistent watches if there
are other watches remaining. But this will leak persistent watches on
the server side. `dispatch_path_event` handles this, it will issue
`removeWatches` with `WatcherType.Any` if it receives events to a path
with no watchers. For no persistent watches, the client will issue
corresponding `WatcherType`s if there are more watchers with them. But
it can't do the same for persistent watches due to leak of
`WatcherType.Persistent` and `WatcherType.PersistentRecursive`. Hope
this convinces you of  ZOOKEEPER-4472.

[1]: https://issues.apache.org/jira/browse/ZOOKEEPER-4471
[2]: https://issues.apache.org/jira/browse/ZOOKEEPER-4472
[3]: https://issues.apache.org/jira/browse/ZOOKEEPER-4466
[4]: https://lists.apache.org/thread/dgnt1xnlf5n9tzt7m4otbodg2qdx3fz4
[5]: https://github.com/apache/zookeeper/pull/1998
[6]: https://github.com/apache/zookeeper/pull/2006
[7]: 
https://github.com/kezhuw/zookeeper-client-rust/blob/7466e867fb1b229d6b6ffb230e2e682c49ff494e/src/session/watch.rs#L368-L388
[8]: 
https://github.com/kezhuw/zookeeper-client-rust/blob/7466e867fb1b229d6b6ffb230e2e682c49ff494e/src/session/watch.rs#L341-L366
[9]: https://github.com/kezhuw/zookeeper-client-rust

Best,
Kezhu Wang

Reply via email to