There isnt any documentation on the interface tagging other than the running 
comments. I will try to get hold of one of the hadoop folks to get me a dump of 
the info and will create a jira!


On 8/11/10 9:56 AM, "Patrick Hunt" <> wrote:

wrt defining interface stability we should adopt something like hadoop
is now doing:

Mahadev, do you know if this is documented somewhere? "final"
documentation, rather than the running commentary thats on this jira? We
could adopt something similar/same. Can you create a jira for that?


On 08/11/2010 08:23 AM, Thomas Koch wrote:
> Hallo Mahadev,
> thank you for your nice answer. Yes, we'll of cause preserve compatibility.
> Otherwise there is no chance to get accepted.
> I assume the following things must keep their interfaces:
> ZooKeeper (It'll call the new interface in the background), ASyncCallback,
> Watcher
> We may want to change: ClientCnxn (faktor out some things, remove dep on
> ZooKeeper)
> I think other classes should not be involved at all in our issues. My collegue
> Patrick was so kind to fill the jira issues.
> Best regards,
> Thomas
> Mahadev Konar:
>> Also, I am assuming you have backwards compatability in mind when you
>> suggest these changes right?
>> The interfaces of zookeeper client should not be changing as part of this,
>> though the recursive delete hasn't been introduced yet (its only available
>> in 3.4, so we can move it out into a helper class).
>> Thanks
>> mahadev
>> On 8/11/10 7:40 AM, "Mahadev Konar"<>  wrote:
>> HI Thomas,
>>    I read through the list of issues you posted, most of them seem
>> reasonable to fix. The one's you have mentioned below might take quite a
>> bit of time to fix and again a lot of testing! (just a warning :)). It
>> would be great if you'd want to clean this up for 3.4. Please go ahead and
>> file a jira. These improvements would be good to have in the zookeeper
>> java client.
>> For deleteRecursive, I definitely agree that it should be a helper class. I
>> don't believe it should be in the direct zookeeper api!
>> Thanks
>> mahadev
>> On 8/11/10 2:45 AM, "Thomas Koch"<>  wrote:
>> Hi,
>> I started yesterday to work on my idea of an alternative ZooKeeper client
>> interface.[1] Instead of methods on a ZooKeeper class, a user should
>> instantiate an Operation (Create, Delete, ...) and forward it to an
>> Executor which handles session loss errors and alikes.
>> By doing that, I got shocked by the sheer number of WTF issues I found. I'm
>> sorry for ranting now, but it gets quicker to the poing.
>> - Hostlist as string
>> The hostlist is parsed in the ctor of ClientCnxn. This violates the rule of
>> not doing (too much) work in a ctor. Instead the ClientCnxn should receive
>> an object of class "HostSet". HostSet could then be instantiated e.g. with
>> a comma separated string.
>> - cyclic dependency ClientCnxn, ZooKeeper
>> ZooKeeper instantiates ClientCnxn in its ctor with this and therefor builds
>> a cyclic dependency graph between both objects. This means, you can't have
>> the one without the other. So why did you bother do make them to separate
>> classes in the first place?
>> ClientCnxn accesses ZooKeeper.state. State should rather be a property of
>> ClientCnxn. And ClientCnxn accesses zooKeeper.get???Watches() in its method
>> primeConnection(). I've not yet checked, how this dependency should be
>> resolved better.
>> - Chroot is an attribute of ClientCnxn
>> I'd like to have one process that uses ZooKeeper for different things
>> (managing a list of work, locking some unrelated locks elsewhere). So I've
>> components that do this work inside the same process. These components
>> should get the same zookeeper-client reference chroot'ed for their needs.
>> So it'd be much better, if the ClientCnxn would not care about the chroot.
>> - deleteRecursive does not belong to the other methods
>> DeleteRecursive has been committed to trunk already as a method to the
>> zookeeper class. So in the API it has the same level as the atomic
>> operations create, delete, getData, setData, etc. The user must get the
>> false impression, that deleteRecursive is also an atomic operation.
>> It would be better to have deleteRecursive in some helper class but not
>> that deep in zookeeper's core code. Maybe I'd like to have another policy
>> on how to react if deleteRecursive fails in the middle of its work?
>> - massive code duplication in zookeeper class
>> Each operation calls validatePath, handles the chroot, calls ClientCnxn and
>> checks the return header for error. I'd like to address this with the
>> operation classes:
>> Each operation should receive a prechecked Path object. Calling ClientCnxn
>> and error checking is not (or only partly) the concern of the operation
>> but of an "executor" like class.
>> - stat is returned by parameter
>> Since one can return only one value in java it's the only choice to do so.
>> Still it feels more like C then like Java. However with operator classes
>> one could simply get the result values with getter functions after the
>> execution.
>> - stat calls static method on org.apache.zookeeper.server.DataTree
>> It's a huge jump from client code to the internal server class DataTree.
>> Shouldn't there rather be some class related to the protobuffer stat class
>> that knows how to copy a stat?
>> - Session class?
>> Maybe it'd make sense to combine hostlist, sessionId, sessionPassword and
>> sessionTimeout in a Session class so that the ctor of ClientCnxn won't get
>> too long?
>> I may have missed some items. :-)
>> Once again, please excuse my harsh tone. May I put the above issues in jira
>> and would you accept (backwards compatible) patches for it for 3.4.0?
>> Zookeeper is a fascinating project. Cudos to the devs. I've only looked in
>> the client side code, which is what most users of zookeeper will ever see
>> if they see any zookeeper internal code at all. So it may make sense to
>> make this piece of the project as nice and clean as possible.
>> [1]
>> dev/201005.mbox/
>> Best regards,
>> Thomas Koch,

Reply via email to