Mailing-List: contact zookeeper-user-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: zookeeper-user@hadoop.apache.org
Received-SPF: neutral (nike.apache.org: local policy)
Message-ID: <4963CDFE.8000701@apache.org>
Date: Tue, 06 Jan 2009 13:32:46 -0800
From: Patrick Hunt <phunt@apache.org>
User-Agent: Thunderbird 2.0.0.18 (X11/20081125)
MIME-Version: 1.0
To: zookeeper-user@hadoop.apache.org
Subject: Re: Simpler ZooKeeper event interface....
References: <30c6373b0901060926h691ac01h38b36dafba8edbf9@mail.gmail.com>
In-Reply-To: <30c6373b0901060926h691ac01h38b36dafba8edbf9@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi Kevin, a couple of issues I noticed while looking at the pastebin:

1) you are ignoring the result codes in the callbacks, this could get 
you into trouble (say you do a getData on a node that has been deleted 
ie someone changes then immed. deletes the node)

2) I'm confused by one of your comments, you mention:
   //read the current value.  NOTE that this could easily be a blocking
   //read here but we might as well have one code path.
   zk.getData( event.getPath(), true, this, null );

however you are using an async API call, what blocking are you referring 
to? If you are ok w/blocking use the synchronous API, your code would be 
simpler (no callbacks!)

3) it's possible for your code to get notified of a change, but never 
process the change. This might happen if:
  a) a node changed watch fires
  b) your client code runs an async getData
  c) you are disconnected from the server

Patrick

Kevin Burton wrote:
> Hey guys.
> 
> I think I'm finally in the position to push ZK into production for a while
> to test it out.
> 
> My biggest feedback (other than the small bugs I found) was that the API
> could be a bit simpler.
> 
> I codified my thoughts here:
> 
> http://pastebin.com/f2ecea8c7
> http://pastebin.com/f62a01e9
> 
> Basically, I was thinking that one could receive an onData event to receive
> the initial value.
> 
> Then all future events would call onData.....
> 
> I was thinking that an onExists() method might also be nice.
> 
> The current API could be made cleaner with:
> 
>  - one or two standalone Listener interfaces with onFoo methods for each
> event type.
>  - the processResults() method is the same for each interface right now
> which is somewhat confusing.  Using onFoo is more self documenting.
>  - using the main thread by using poll() to wait for events from ZooKeeper.
> I use a ConcurrentLinkedQueue in my implementation.
> 
> Also, is there a race condition between when the client receives an event
> for an update and before it can request a new one?  I was thinking session
> local based events would solve this problem (you register your watch once
> per session and then get all events until it is unregistered).
> 
> I think this can be solved in my code by reading the current version of the
> value from the getData() method I call when I register the new watch and
> comparing it to the last version I saw. If it was incremented then I would
> call onData again.
> 
> The problem here is that I might miss two updates (but at least I would
> receive the last stable value).
> 
> Kevin
>