However it should be pretty easy to apply though given our stability and
relatively small size. :-) Seems like it would be useful for users and
developers both...

Patrick

On Fri, Sep 3, 2010 at 11:49 PM, Mahadev Konar <maha...@yahoo-inc.com>wrote:

> I was able to get hold of one of the hadoop developers. So the gist of the
> story is,
>
> They have interface tagging saying
>
> Something like
>
> @Audience.limitedPrivate(target="pig")
>
> Wherin this interface is defined for pig and is only to be used by pig
> oflks.
>
> Interfaces can be defined as public, stable, unstable, ......
>
> This is quite useful but given out interfaces havent chanhged in a long
> time
> this might not be that helpful for us.
>
> Thanks
> mahadev
>
>
> On 8/31/10 3:47 PM, "Mahadev Konar" <maha...@yahoo-inc.com> wrote:
>
> > 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
> >
> >
>
>

Reply via email to