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!
Thanks mahadev On 8/11/10 9:56 AM, "Patrick Hunt" <ph...@apache.org> wrote: wrt defining interface stability we should adopt something like hadoop is now doing: https://issues.apache.org/jira/browse/HADOOP-5073 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? Patrick 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"<maha...@yahoo-inc.com> 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"<tho...@koch.ro> 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] http://mail-archives.apache.org/mod_mbox/hadoop-zookeeper- >> dev/201005.mbox/%3c201005261509.54236.tho...@koch.ro%3e >> >> Best regards, >> >> Thomas Koch, http://www.koch.ro
