You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by Kezhu Wang <ke...@gmail.com> on 2022/07/01 03:38:16 UTC
Re: Call for reviews for ZOOKEEPER-4475 and ZOOKEEPER-4466
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 (wander4096@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 <ke...@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
>
Re: Call for reviews for ZOOKEEPER-4475 and ZOOKEEPER-4466
Posted by Kezhu Wang <ke...@gmail.com>.
Hi,
Now, both ZOOKEEPER-4475[1] and ZOOKEEPER-44662[2] are merged. Thank
you Enrico and tison for reviewing.
I have moved on to ZOOKEEPER-4471[3] and ZOOKEEPER[4].
ZOOKEEPER-4471[3] reported that `AddWatchMode.PERSISTENT` could be
broke(a.k.a partially removed) by `removeWatches` `WatcherType.Data`
or `WatcherType.Children`. I think we should avoid doing such
magic/compute things on the server side.
ZOOKEEPER-4472[4] proposed to add new `WatcherType`s to remove
`AddWatchMode.PERSISTENT` and `AddWatchMode.PERSISTENT_RECUR
SIVE`
individually. Currently, they can only be removed by
`WatcherType.Any`. After ZOOKEEPER-4466, which enables multiple watch
modes on one path, we need separated `WatcherType`s to remove them
without interfering with others.
Look forward to your reviews in pending pr #1998[5] for ZOOKEEPER-4471.
[1]: https://issues.apache.org/jira/browse/ZOOKEEPER-4475
[2]: https://issues.apache.org/jira/browse/ZOOKEEPER-4466
[3]: https://issues.apache.org/jira/browse/ZOOKEEPER-4471
[4]: https://issues.apache.org/jira/browse/ZOOKEEPER-4472
[5]: https://github.com/apache/zookeeper/pull/1998
I am sending this reply for the second time, I can't see my reply from
the mailing list after 14 hours. Sorry for being boring if you got two
copies.
Best,
Kezhu Wang
On Tue, May 9, 2023 at 12:01 AM Kezhu Wang <ke...@gmail.com> wrote:
>
> Hi,
>
> Now, both ZOOKEEPER-4475[1] and ZOOKEEPER-44662[2] are merged. Thank
> you Enrico and tison for reviewing.
>
> I have moved on to ZOOKEEPER-4471[3] and ZOOKEEPER[4].
>
> ZOOKEEPER-4471[3] reported that `AddWatchMode.PERSISTENT` could be
> broke(a.k.a partially removed) by `removeWatches` `WatcherType.Data`
> or `WatcherType.Children`. I think we should avoid doing such
> magic/compute things on the server side.
>
> ZOOKEEPER-4472[4] proposed to add new `WatcherType`s to remove
> `AddWatchMode.PERSISTENT` and `AddWatchMode.PERSISTENT_RECURSIVE`
> individually. Currently, they can only be removed by
> `WatcherType.Any`. After ZOOKEEPER-4466, which enables multiple watch
> modes on one path, we need separated `WatcherType`s to remove them
> without interfering with others.
>
> Look forward to your reviews in pending pr #1998[5] for ZOOKEEPER-4471.
>
> [1]: https://issues.apache.org/jira/browse/ZOOKEEPER-4475
> [2]: https://issues.apache.org/jira/browse/ZOOKEEPER-4466
> [3]: https://issues.apache.org/jira/browse/ZOOKEEPER-4471
> [4]: https://issues.apache.org/jira/browse/ZOOKEEPER-4472
> [5]: https://github.com/apache/zookeeper/pull/1998
>
> Best,
> Kezhu Wang
>
> On Mon, Feb 27, 2023 at 9:19 PM Kezhu Wang <ke...@gmail.com> wrote:
> >
> > Hi eolivelli, tison,
> >
> > Thank you for reviewing and merging ZOOKEEPER-4475[1](pr#1820[2]).
> >
> > It is time to move on ZOOKEEPER-4466[3](pr#1859[4]) which make standard watch and persistent watch orthogonal on same path.
> >
> > Contrast to pure client side fix ZOOKEEPER-4475[1], ZOOKEEPER-4466[3] demands only server side changes. I agree to what @eolivelli says "We need more eyes on this patch". Ping here for possible more attentions.
> >
> > [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
> >
> > Best,
> > Kezhu Wang
> >
> > On Sat, Dec 24, 2022 at 8:33 PM Enrico Olivelli <eo...@gmail.com> wrote:
> >>
> >> Kezhu,
> >> Sorry for late reply.
> >> We should definitely move forward with this work
> >>
> >>
> >> Enrico
> >>
> >> Il Lun 17 Ott 2022, 16:27 Kezhu Wang <ke...@gmail.com> ha scritto:
> >>
> >> > Ping.
> >> >
> >> > Best,
> >> > Kezhu Wang
> >> >
> >> >
> >> > On July 1, 2022 at 11:38:16, Kezhu Wang (kezhuw@gmail.com) wrote:
> >> >
> >> > 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 (wander4096@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 <ke...@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
> >> > >
> >> >
Re: Call for reviews for ZOOKEEPER-4475 and ZOOKEEPER-4466
Posted by Kezhu Wang <ke...@gmail.com>.
Hi,
Now, both ZOOKEEPER-4475[1] and ZOOKEEPER-44662[2] are merged. Thank
you Enrico and tison for reviewing.
I have moved on to ZOOKEEPER-4471[3] and ZOOKEEPER[4].
ZOOKEEPER-4471[3] reported that `AddWatchMode.PERSISTENT` could be
broke(a.k.a partially removed) by `removeWatches` `WatcherType.Data`
or `WatcherType.Children`. I think we should avoid doing such
magic/compute things on the server side.
ZOOKEEPER-4472[4] proposed to add new `WatcherType`s to remove
`AddWatchMode.PERSISTENT` and `AddWatchMode.PERSISTENT_RECURSIVE`
individually. Currently, they can only be removed by
`WatcherType.Any`. After ZOOKEEPER-4466, which enables multiple watch
modes on one path, we need separated `WatcherType`s to remove them
without interfering with others.
Look forward to your reviews in pending pr #1998[5] for ZOOKEEPER-4471.
[1]: https://issues.apache.org/jira/browse/ZOOKEEPER-4475
[2]: https://issues.apache.org/jira/browse/ZOOKEEPER-4466
[3]: https://issues.apache.org/jira/browse/ZOOKEEPER-4471
[4]: https://issues.apache.org/jira/browse/ZOOKEEPER-4472
[5]: https://github.com/apache/zookeeper/pull/1998
Best,
Kezhu Wang
On Mon, Feb 27, 2023 at 9:19 PM Kezhu Wang <ke...@gmail.com> wrote:
>
> Hi eolivelli, tison,
>
> Thank you for reviewing and merging ZOOKEEPER-4475[1](pr#1820[2]).
>
> It is time to move on ZOOKEEPER-4466[3](pr#1859[4]) which make standard watch and persistent watch orthogonal on same path.
>
> Contrast to pure client side fix ZOOKEEPER-4475[1], ZOOKEEPER-4466[3] demands only server side changes. I agree to what @eolivelli says "We need more eyes on this patch". Ping here for possible more attentions.
>
> [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
>
> Best,
> Kezhu Wang
>
> On Sat, Dec 24, 2022 at 8:33 PM Enrico Olivelli <eo...@gmail.com> wrote:
>>
>> Kezhu,
>> Sorry for late reply.
>> We should definitely move forward with this work
>>
>>
>> Enrico
>>
>> Il Lun 17 Ott 2022, 16:27 Kezhu Wang <ke...@gmail.com> ha scritto:
>>
>> > Ping.
>> >
>> > Best,
>> > Kezhu Wang
>> >
>> >
>> > On July 1, 2022 at 11:38:16, Kezhu Wang (kezhuw@gmail.com) wrote:
>> >
>> > 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 (wander4096@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 <ke...@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
>> > >
>> >
Re: Call for reviews for ZOOKEEPER-4475 and ZOOKEEPER-4466
Posted by Kezhu Wang <ke...@gmail.com>.
Hi eolivelli, tison,
Thank you for reviewing and merging ZOOKEEPER-4475[1](pr#1820[2]).
It is time to move on ZOOKEEPER-4466[3](pr#1859[4]) which make standard
watch and persistent watch orthogonal on same path.
Contrast to pure client side fix ZOOKEEPER-4475[1], ZOOKEEPER-4466[3]
demands only server side changes. I agree to what @eolivelli says "We need
more eyes on this patch". Ping here for possible more attentions.
[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
Best,
Kezhu Wang
On Sat, Dec 24, 2022 at 8:33 PM Enrico Olivelli <eo...@gmail.com> wrote:
> Kezhu,
> Sorry for late reply.
> We should definitely move forward with this work
>
>
> Enrico
>
> Il Lun 17 Ott 2022, 16:27 Kezhu Wang <ke...@gmail.com> ha scritto:
>
> > Ping.
> >
> > Best,
> > Kezhu Wang
> >
> >
> > On July 1, 2022 at 11:38:16, Kezhu Wang (kezhuw@gmail.com) wrote:
> >
> > 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 (wander4096@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 <ke...@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
> > >
> >
>
Re: Call for reviews for ZOOKEEPER-4475 and ZOOKEEPER-4466
Posted by Enrico Olivelli <eo...@gmail.com>.
Kezhu,
Sorry for late reply.
We should definitely move forward with this work
Enrico
Il Lun 17 Ott 2022, 16:27 Kezhu Wang <ke...@gmail.com> ha scritto:
> Ping.
>
> Best,
> Kezhu Wang
>
>
> On July 1, 2022 at 11:38:16, Kezhu Wang (kezhuw@gmail.com) wrote:
>
> 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 (wander4096@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 <ke...@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
> >
>
Re: Call for reviews for ZOOKEEPER-4475 and ZOOKEEPER-4466
Posted by Kezhu Wang <ke...@gmail.com>.
Ping.
Best,
Kezhu Wang
On July 1, 2022 at 11:38:16, Kezhu Wang (kezhuw@gmail.com) wrote:
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 (wander4096@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 <ke...@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
>