You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@zookeeper.apache.org by Stu Hood <st...@mailtrust.com> on 2008/11/12 19:31:59 UTC

Exists Watch Triggered by Delete

I'm running the 3.0.0 release, and I'm receiving a warning thrown by this block of code:
> case NodeDeleted:
>     synchronized (dataWatches) {
>         addTo(dataWatches.remove(path), result);
>     }
>     // XXX This shouldn't be needed, but just in case
>     synchronized (existWatches) {
>         addTo(existWatches.remove(path), result);
>         LOG.warn("We are triggering an exists watch for delete! Shouldn't happen!");
>     }
>     synchronized (childWatches) {
>         addTo(childWatches.remove(path), result);
>     }
>     break;

--

Why shouldn't an exists watch be triggered by a node being deleted? That is a really common use case in my code, so I want to rule it out as the cause of a bug I'm hunting for.

Thanks,

Stu Hood
Architecture Software Developer
Mailtrust, a Division of Rackspace


Re: Exists Watch Triggered by Delete

Posted by Patrick Hunt <ph...@apache.org>.
No problem, we really appreciate your feedback -- keep it coming.

Patrick

Stu Hood wrote:
> Thanks a bunch Patrick! I really appreciate your work on ZooKeeper... you have made a tremendous impact on the project.
> 
> I will try the ZOOKEEPER-221 patch.
> 
> Thanks,
> Stu
> 
> 
> -----Original Message-----
> From: "Patrick Hunt" <ph...@apache.org>
> Sent: Wednesday, November 12, 2008 6:37pm
> To: zookeeper-user@hadoop.apache.org
> Subject: Re: Exists Watch Triggered by Delete
> 
> Stu Hood wrote:
>>> The comment you referenced in your original email is true - that
>>> code should never execute as the existsWatches list only contains
>>> watches for NONODE watch registrations (which obv couldn't be
>>> deleted since it doesn't exist).
>> So I am experiencing a bug then?
> 
> Wow, this is a dumb mistake. The log message you pointed to is being 
> output _every time_ this code is called. There needs to be a conditional 
> on the log message to check the result of the remove call.
> 
> I have entered a JIRA and have already submitted a patch:
> https://issues.apache.org/jira/browse/ZOOKEEPER-221
> 
> So unfortunately we can't tell if this is the source of the issue you 
> are seeing. Can you run with the patch attached to ZOOKEEPER-221?
> 
>> It may be better not to let the programmer know about those two
>> lists, and to see if the abstraction can be improved instead.
>> Sometimes I feel that I have to know too much about the internal
>> working of ZooKeeper to use its API.
> 
> That's a good point. We're working to improve the docs & code, I've made 
> a note of it. Perhaps we should move that to "internals" doc and rework 
> this section of the guide...
> https://issues.apache.org/jira/browse/ZOOKEEPER-220
> 
> Patrick
> 
>>
>> -----Original Message----- From: "Patrick Hunt" <ph...@apache.org> 
>> Sent: Wednesday, November 12, 2008 2:11pm To:
>> zookeeper-user@hadoop.apache.org Subject: Re: Exists Watch Triggered
>> by Delete
>>
>> Hi Stu,
>>
>> The zk server maintains 2 lists of watches, data and child watches: 
>> http://hadoop.apache.org/zookeeper/docs/r3.0.0/zookeeperProgrammers.html#ch_zkWatches
>>  (after reviewing this doc I've entered a jira to clarify that the
>> server is maintaining 2 lists being referenced). From the server
>> perspective if you register a watch on a node by calling getData &
>> exists, only a single watch, a data watch, is stored by the server.
>>
>> The client is maintaining lists of watches as well. This is
>> essentially to enable the auto watch reset and multi-watcher features
>> added in v3.
>>
>> Take a look at class ExistsWatchRegistration, it will register client
>>  side dataWatches for exists calls -- unless the result code is not 0
>> (ie NONODE), in which case it will register using existsWatches
>> (again, client side).
>>
>> The comment you referenced in your original email is true - that code
>>  should never execute as the existsWatches list only contains watches
>> for NONODE watch registrations (which obv couldn't be deleted since
>> it doesn't exist).
>>
>> Hope this helps,
>>
>> Patrick
>>
>> Stu Hood wrote:
>>> I'm running the 3.0.0 release, and I'm receiving a warning thrown
>>> by this block of code:
>>>> case NodeDeleted: synchronized (dataWatches) { 
>>>> addTo(dataWatches.remove(path), result); } // XXX This shouldn't
>>>> be needed, but just in case synchronized (existWatches) { 
>>>> addTo(existWatches.remove(path), result); LOG.warn("We are
>>>> triggering an exists watch for delete! Shouldn't happen!"); } 
>>>> synchronized (childWatches) { addTo(childWatches.remove(path),
>>>> result); } break;
>>> --
>>>
>>> Why shouldn't an exists watch be triggered by a node being deleted?
>>> That is a really common use case in my code, so I want to rule it
>>> out as the cause of a bug I'm hunting for.
>>>
>>> Thanks,
>>>
>>> Stu Hood Architecture Software Developer Mailtrust, a Division of
>>> Rackspace
>>>
>>
> 
> 

Re: Exists Watch Triggered by Delete

Posted by Stu Hood <st...@mailtrust.com>.
Thanks a bunch Patrick! I really appreciate your work on ZooKeeper... you have made a tremendous impact on the project.

I will try the ZOOKEEPER-221 patch.

Thanks,
Stu


-----Original Message-----
From: "Patrick Hunt" <ph...@apache.org>
Sent: Wednesday, November 12, 2008 6:37pm
To: zookeeper-user@hadoop.apache.org
Subject: Re: Exists Watch Triggered by Delete

Stu Hood wrote:
>> The comment you referenced in your original email is true - that
>> code should never execute as the existsWatches list only contains
>> watches for NONODE watch registrations (which obv couldn't be
>> deleted since it doesn't exist).
> So I am experiencing a bug then?

Wow, this is a dumb mistake. The log message you pointed to is being 
output _every time_ this code is called. There needs to be a conditional 
on the log message to check the result of the remove call.

I have entered a JIRA and have already submitted a patch:
https://issues.apache.org/jira/browse/ZOOKEEPER-221

So unfortunately we can't tell if this is the source of the issue you 
are seeing. Can you run with the patch attached to ZOOKEEPER-221?

> It may be better not to let the programmer know about those two
> lists, and to see if the abstraction can be improved instead.
> Sometimes I feel that I have to know too much about the internal
> working of ZooKeeper to use its API.

That's a good point. We're working to improve the docs & code, I've made 
a note of it. Perhaps we should move that to "internals" doc and rework 
this section of the guide...
https://issues.apache.org/jira/browse/ZOOKEEPER-220

Patrick

> 
> 
> -----Original Message----- From: "Patrick Hunt" <ph...@apache.org> 
> Sent: Wednesday, November 12, 2008 2:11pm To:
> zookeeper-user@hadoop.apache.org Subject: Re: Exists Watch Triggered
> by Delete
> 
> Hi Stu,
> 
> The zk server maintains 2 lists of watches, data and child watches: 
> http://hadoop.apache.org/zookeeper/docs/r3.0.0/zookeeperProgrammers.html#ch_zkWatches
>  (after reviewing this doc I've entered a jira to clarify that the
> server is maintaining 2 lists being referenced). From the server
> perspective if you register a watch on a node by calling getData &
> exists, only a single watch, a data watch, is stored by the server.
> 
> The client is maintaining lists of watches as well. This is
> essentially to enable the auto watch reset and multi-watcher features
> added in v3.
> 
> Take a look at class ExistsWatchRegistration, it will register client
>  side dataWatches for exists calls -- unless the result code is not 0
> (ie NONODE), in which case it will register using existsWatches
> (again, client side).
> 
> The comment you referenced in your original email is true - that code
>  should never execute as the existsWatches list only contains watches
> for NONODE watch registrations (which obv couldn't be deleted since
> it doesn't exist).
> 
> Hope this helps,
> 
> Patrick
> 
> Stu Hood wrote:
>> I'm running the 3.0.0 release, and I'm receiving a warning thrown
>> by this block of code:
>>> case NodeDeleted: synchronized (dataWatches) { 
>>> addTo(dataWatches.remove(path), result); } // XXX This shouldn't
>>> be needed, but just in case synchronized (existWatches) { 
>>> addTo(existWatches.remove(path), result); LOG.warn("We are
>>> triggering an exists watch for delete! Shouldn't happen!"); } 
>>> synchronized (childWatches) { addTo(childWatches.remove(path),
>>> result); } break;
>> --
>> 
>> Why shouldn't an exists watch be triggered by a node being deleted?
>> That is a really common use case in my code, so I want to rule it
>> out as the cause of a bug I'm hunting for.
>> 
>> Thanks,
>> 
>> Stu Hood Architecture Software Developer Mailtrust, a Division of
>> Rackspace
>> 
> 
> 



Re: Exists Watch Triggered by Delete

Posted by Patrick Hunt <ph...@apache.org>.
Stu Hood wrote:
>> The comment you referenced in your original email is true - that
>> code should never execute as the existsWatches list only contains
>> watches for NONODE watch registrations (which obv couldn't be
>> deleted since it doesn't exist).
> So I am experiencing a bug then?

Wow, this is a dumb mistake. The log message you pointed to is being 
output _every time_ this code is called. There needs to be a conditional 
on the log message to check the result of the remove call.

I have entered a JIRA and have already submitted a patch:
https://issues.apache.org/jira/browse/ZOOKEEPER-221

So unfortunately we can't tell if this is the source of the issue you 
are seeing. Can you run with the patch attached to ZOOKEEPER-221?

> It may be better not to let the programmer know about those two
> lists, and to see if the abstraction can be improved instead.
> Sometimes I feel that I have to know too much about the internal
> working of ZooKeeper to use its API.

That's a good point. We're working to improve the docs & code, I've made 
a note of it. Perhaps we should move that to "internals" doc and rework 
this section of the guide...
https://issues.apache.org/jira/browse/ZOOKEEPER-220

Patrick

> 
> 
> -----Original Message----- From: "Patrick Hunt" <ph...@apache.org> 
> Sent: Wednesday, November 12, 2008 2:11pm To:
> zookeeper-user@hadoop.apache.org Subject: Re: Exists Watch Triggered
> by Delete
> 
> Hi Stu,
> 
> The zk server maintains 2 lists of watches, data and child watches: 
> http://hadoop.apache.org/zookeeper/docs/r3.0.0/zookeeperProgrammers.html#ch_zkWatches
>  (after reviewing this doc I've entered a jira to clarify that the
> server is maintaining 2 lists being referenced). From the server
> perspective if you register a watch on a node by calling getData &
> exists, only a single watch, a data watch, is stored by the server.
> 
> The client is maintaining lists of watches as well. This is
> essentially to enable the auto watch reset and multi-watcher features
> added in v3.
> 
> Take a look at class ExistsWatchRegistration, it will register client
>  side dataWatches for exists calls -- unless the result code is not 0
> (ie NONODE), in which case it will register using existsWatches
> (again, client side).
> 
> The comment you referenced in your original email is true - that code
>  should never execute as the existsWatches list only contains watches
> for NONODE watch registrations (which obv couldn't be deleted since
> it doesn't exist).
> 
> Hope this helps,
> 
> Patrick
> 
> Stu Hood wrote:
>> I'm running the 3.0.0 release, and I'm receiving a warning thrown
>> by this block of code:
>>> case NodeDeleted: synchronized (dataWatches) { 
>>> addTo(dataWatches.remove(path), result); } // XXX This shouldn't
>>> be needed, but just in case synchronized (existWatches) { 
>>> addTo(existWatches.remove(path), result); LOG.warn("We are
>>> triggering an exists watch for delete! Shouldn't happen!"); } 
>>> synchronized (childWatches) { addTo(childWatches.remove(path),
>>> result); } break;
>> --
>> 
>> Why shouldn't an exists watch be triggered by a node being deleted?
>> That is a really common use case in my code, so I want to rule it
>> out as the cause of a bug I'm hunting for.
>> 
>> Thanks,
>> 
>> Stu Hood Architecture Software Developer Mailtrust, a Division of
>> Rackspace
>> 
> 
> 

Re: Exists Watch Triggered by Delete

Posted by Stu Hood <st...@mailtrust.com>.
> The comment you referenced in your original email is true - that code 
> should never execute as the existsWatches list only contains watches for 
> NONODE watch registrations (which obv couldn't be deleted since it 
> doesn't exist).
So I am experiencing a bug then?

It may be better not to let the programmer know about those two lists, and to see if the abstraction can be improved instead. Sometimes I feel that I have to know too much about the internal working of ZooKeeper to use its API.

Thanks Patrick,
Stu


-----Original Message-----
From: "Patrick Hunt" <ph...@apache.org>
Sent: Wednesday, November 12, 2008 2:11pm
To: zookeeper-user@hadoop.apache.org
Subject: Re: Exists Watch Triggered by Delete

Hi Stu,

The zk server maintains 2 lists of watches, data and child watches:
http://hadoop.apache.org/zookeeper/docs/r3.0.0/zookeeperProgrammers.html#ch_zkWatches
(after reviewing this doc I've entered a jira to clarify that the server 
is maintaining 2 lists being referenced). From the server perspective if 
you register a watch on a node by calling getData & exists, only a 
single watch, a data watch, is stored by the server.

The client is maintaining lists of watches as well. This is essentially 
to enable the auto watch reset and multi-watcher features added in v3.

Take a look at class ExistsWatchRegistration, it will register client 
side dataWatches for exists calls -- unless the result code is not 0 (ie 
NONODE), in which case it will register using existsWatches (again, 
client side).

The comment you referenced in your original email is true - that code 
should never execute as the existsWatches list only contains watches for 
NONODE watch registrations (which obv couldn't be deleted since it 
doesn't exist).

Hope this helps,

Patrick

Stu Hood wrote:
> I'm running the 3.0.0 release, and I'm receiving a warning thrown by this block of code:
>> case NodeDeleted:
>>     synchronized (dataWatches) {
>>         addTo(dataWatches.remove(path), result);
>>     }
>>     // XXX This shouldn't be needed, but just in case
>>     synchronized (existWatches) {
>>         addTo(existWatches.remove(path), result);
>>         LOG.warn("We are triggering an exists watch for delete! Shouldn't happen!");
>>     }
>>     synchronized (childWatches) {
>>         addTo(childWatches.remove(path), result);
>>     }
>>     break;
> 
> --
> 
> Why shouldn't an exists watch be triggered by a node being deleted? That is a really common use case in my code, so I want to rule it out as the cause of a bug I'm hunting for.
> 
> Thanks,
> 
> Stu Hood
> Architecture Software Developer
> Mailtrust, a Division of Rackspace
> 



Re: Exists Watch Triggered by Delete

Posted by Patrick Hunt <ph...@apache.org>.
Hi Stu,

The zk server maintains 2 lists of watches, data and child watches:
http://hadoop.apache.org/zookeeper/docs/r3.0.0/zookeeperProgrammers.html#ch_zkWatches
(after reviewing this doc I've entered a jira to clarify that the server 
is maintaining 2 lists being referenced). From the server perspective if 
you register a watch on a node by calling getData & exists, only a 
single watch, a data watch, is stored by the server.

The client is maintaining lists of watches as well. This is essentially 
to enable the auto watch reset and multi-watcher features added in v3.

Take a look at class ExistsWatchRegistration, it will register client 
side dataWatches for exists calls -- unless the result code is not 0 (ie 
NONODE), in which case it will register using existsWatches (again, 
client side).

The comment you referenced in your original email is true - that code 
should never execute as the existsWatches list only contains watches for 
NONODE watch registrations (which obv couldn't be deleted since it 
doesn't exist).

Hope this helps,

Patrick

Stu Hood wrote:
> I'm running the 3.0.0 release, and I'm receiving a warning thrown by this block of code:
>> case NodeDeleted:
>>     synchronized (dataWatches) {
>>         addTo(dataWatches.remove(path), result);
>>     }
>>     // XXX This shouldn't be needed, but just in case
>>     synchronized (existWatches) {
>>         addTo(existWatches.remove(path), result);
>>         LOG.warn("We are triggering an exists watch for delete! Shouldn't happen!");
>>     }
>>     synchronized (childWatches) {
>>         addTo(childWatches.remove(path), result);
>>     }
>>     break;
> 
> --
> 
> Why shouldn't an exists watch be triggered by a node being deleted? That is a really common use case in my code, so I want to rule it out as the cause of a bug I'm hunting for.
> 
> Thanks,
> 
> Stu Hood
> Architecture Software Developer
> Mailtrust, a Division of Rackspace
>