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
