ZK 3.5 Removing watches

[Cameron McKenzie <ca...@apache.org>]

Guys,
I've started looking into this enhancement (CURATOR-161), and was just
looking for some opinions on the fluid style for removing watches.

I was thinking of:

*1.) For removing a single watcher*

curator.removeWatcher(watcher).ofType(watcherType).forPath("/test");

*2.) For removing a single watcher in the background (would support the
normal Backgroundable interface)*

curator.removeWatcher(watcher).ofType(watcherType).inBackground().forPath("/test");

*3.) For removing all watchers of a given type:*

curator.removeWatchers().ofType(watcherType).forPath("/test");

*4.) For removing all watchers of a given type in background:*

curator.removeWatchers().ofType(watcherType).inBackground().forPath("/test");

*5.) Also handling the case where we wish to allow the client to remove its
local watches if it is not connected to the server. This would be supported
with both versions and inBackground()*

curator.removeWatcher(watcher).ofType(watcherType).allowLocalRemoval(true).forPath("/test");
curator.removeWatchers().ofType(watcherType).allowLocalRemoval(true).forPath("/test");

Or is it preferable to combine the top level removeWatcher() /
removeWatchers() into a single call and have the underlying interface
differentiate?

Something like:

curator.removeWatchers().watcher(watcherRef).ofType(watcherType).forPath("/test");
curator.removeWatchers().all().ofType(watcherType).forPath("/test");

I think that the first way reads better, but I guess it clutters up the
CuratorFramework a bit more.

Any thoughts are appreciated.
cheers

Re: ZK 3.5 Removing watches

[Mike Drob <ma...@cloudera.com>]

Not a fan of all(), but otherwise I think #5 is fine?

Completely unrelated to your question, but will it be possible to remove
watches recursively?

On Tue, Nov 25, 2014 at 10:52 PM, Cameron McKenzie <ca...@apache.org>
wrote:

> Guys,
> I've started looking into this enhancement (CURATOR-161), and was just
> looking for some opinions on the fluid style for removing watches.
>
> I was thinking of:
>
> *1.) For removing a single watcher*
>
> curator.removeWatcher(watcher).ofType(watcherType).forPath("/test");
>
> *2.) For removing a single watcher in the background (would support the
> normal Backgroundable interface)*
>
>
> curator.removeWatcher(watcher).ofType(watcherType).inBackground().forPath("/test");
>
> *3.) For removing all watchers of a given type:*
>
> curator.removeWatchers().ofType(watcherType).forPath("/test");
>
> *4.) For removing all watchers of a given type in background:*
>
>
> curator.removeWatchers().ofType(watcherType).inBackground().forPath("/test");
>
> *5.) Also handling the case where we wish to allow the client to remove its
> local watches if it is not connected to the server. This would be supported
> with both versions and inBackground()*
>
>
> curator.removeWatcher(watcher).ofType(watcherType).allowLocalRemoval(true).forPath("/test");
>
> curator.removeWatchers().ofType(watcherType).allowLocalRemoval(true).forPath("/test");
>
> Or is it preferable to combine the top level removeWatcher() /
> removeWatchers() into a single call and have the underlying interface
> differentiate?
>
> Something like:
>
>
> curator.removeWatchers().watcher(watcherRef).ofType(watcherType).forPath("/test");
> curator.removeWatchers().all().ofType(watcherType).forPath("/test");
>
> I think that the first way reads better, but I guess it clutters up the
> CuratorFramework a bit more.
>
> Any thoughts are appreciated.
> cheers
>

Re: ZK 3.5 Removing watches

[Cameron McKenzie <mc...@gmail.com>]

Mike,
I don't believe that ZK natively supports recursively removing watches, but
I can see that this would be something handy to do. We'd need change the
NamespaceWatcherMap to store data in a different data structure to make it
efficient to work out child watchers of a given path.

Scott,
That's a good point. I guess that we'd need to introduce some sort of
WatcherListener framework, whereby the recipes can register for events
indicating that watches have been removed (and maybe added, but I doubt
that's of much interest).

cheers

On Thu, Nov 27, 2014 at 6:38 AM, Scott Blum <dr...@gmail.com> wrote:

> Just one high level question, the forms where you don't need to pass in
> the watcher instance, that would tend to allow people to break recipes by
> killing watchers out from under the recipe?  If so... should the recipes
> have a way to be notified this happened and potentially re-establish
> watches?
>
> On Tue, Nov 25, 2014 at 11:52 PM, Cameron McKenzie <cammckenzie@apache.org
> > wrote:
>
>> Guys,
>> I've started looking into this enhancement (CURATOR-161), and was just
>> looking for some opinions on the fluid style for removing watches.
>>
>> I was thinking of:
>>
>> *1.) For removing a single watcher*
>>
>> curator.removeWatcher(watcher).ofType(watcherType).forPath("/test");
>>
>> *2.) For removing a single watcher in the background (would support the
>> normal Backgroundable interface)*
>>
>>
>> curator.removeWatcher(watcher).ofType(watcherType).inBackground().forPath("/test");
>>
>> *3.) For removing all watchers of a given type:*
>>
>> curator.removeWatchers().ofType(watcherType).forPath("/test");
>>
>> *4.) For removing all watchers of a given type in background:*
>>
>>
>> curator.removeWatchers().ofType(watcherType).inBackground().forPath("/test");
>>
>> *5.) Also handling the case where we wish to allow the client to remove
>> its
>> local watches if it is not connected to the server. This would be
>> supported
>> with both versions and inBackground()*
>>
>>
>> curator.removeWatcher(watcher).ofType(watcherType).allowLocalRemoval(true).forPath("/test");
>>
>> curator.removeWatchers().ofType(watcherType).allowLocalRemoval(true).forPath("/test");
>>
>> Or is it preferable to combine the top level removeWatcher() /
>> removeWatchers() into a single call and have the underlying interface
>> differentiate?
>>
>> Something like:
>>
>>
>> curator.removeWatchers().watcher(watcherRef).ofType(watcherType).forPath("/test");
>> curator.removeWatchers().all().ofType(watcherType).forPath("/test");
>>
>> I think that the first way reads better, but I guess it clutters up the
>> CuratorFramework a bit more.
>>
>> Any thoughts are appreciated.
>> cheers
>>
>
>

Re: ZK 3.5 Removing watches

[Scott Blum <dr...@gmail.com>]

Just one high level question, the forms where you don't need to pass in the
watcher instance, that would tend to allow people to break recipes by
killing watchers out from under the recipe?  If so... should the recipes
have a way to be notified this happened and potentially re-establish
watches?

On Tue, Nov 25, 2014 at 11:52 PM, Cameron McKenzie <ca...@apache.org>
wrote:

> Guys,
> I've started looking into this enhancement (CURATOR-161), and was just
> looking for some opinions on the fluid style for removing watches.
>
> I was thinking of:
>
> *1.) For removing a single watcher*
>
> curator.removeWatcher(watcher).ofType(watcherType).forPath("/test");
>
> *2.) For removing a single watcher in the background (would support the
> normal Backgroundable interface)*
>
>
> curator.removeWatcher(watcher).ofType(watcherType).inBackground().forPath("/test");
>
> *3.) For removing all watchers of a given type:*
>
> curator.removeWatchers().ofType(watcherType).forPath("/test");
>
> *4.) For removing all watchers of a given type in background:*
>
>
> curator.removeWatchers().ofType(watcherType).inBackground().forPath("/test");
>
> *5.) Also handling the case where we wish to allow the client to remove its
> local watches if it is not connected to the server. This would be supported
> with both versions and inBackground()*
>
>
> curator.removeWatcher(watcher).ofType(watcherType).allowLocalRemoval(true).forPath("/test");
>
> curator.removeWatchers().ofType(watcherType).allowLocalRemoval(true).forPath("/test");
>
> Or is it preferable to combine the top level removeWatcher() /
> removeWatchers() into a single call and have the underlying interface
> differentiate?
>
> Something like:
>
>
> curator.removeWatchers().watcher(watcherRef).ofType(watcherType).forPath("/test");
> curator.removeWatchers().all().ofType(watcherType).forPath("/test");
>
> I think that the first way reads better, but I guess it clutters up the
> CuratorFramework a bit more.
>
> Any thoughts are appreciated.
> cheers
>