Mahadev konar commented on ZOOKEEPER-550:

  good to see this. The patch looks good. A few comments/nits

 - the code DistributedQueue does not handle ZCONNECTIONLOSS for zookeeper 
operations. I am working on ZOOKEEPER-22 to eliminate CONNECTIONLOSS, so your 
code should work fine after ZOOKEEPER-22 is resolved. It is a lot of work to 
handle CONNECTIONLOSS and since ZOOKEEPER-22 is anyway slated for 3.3, it would 
be a waste of effort. So, I would suggest that put something like this in the 
README - "This recipe will work correctly only if ZOOKEEPER-22 is resolved", in 
case ZOOKEEPER-22 is not resolved by 3.3.

- the documentation in element() is misleading


        //When we get all the children, we try each as a possible head to 
        //By the time we try all of them, there may be new ones added.
        //Need to retry unless we see 0 children.
        //Otherwise we may throw the exception even in the case that the queue 
is never empty.

- for remove() and take() why is it deleting all the children nodes? I think I 
might be misunderstanding but it seems they are both deleting all the child 

- also the API should provide for creation of elements via zookeeper acl's. The 
constructor could take an argument as ACL's, which could be used if passed else 
the UNSAFE_ALL is fine.

> Java Queue Recipe
> -----------------
>                 Key: ZOOKEEPER-550
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-550
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>    Affects Versions: 3.2.1
>            Reporter: Steven Cheng
>            Assignee: Steven Cheng
>            Priority: Minor
>             Fix For: 3.3.0
>         Attachments: ZOOKEEPER-550.patch, ZOOKEEPER-550.patch, 
> ZOOKEEPER-550.patch
> This patch adds a recipe for creating a distributed queue with ZooKeeper 
> similar to the WriteLock recipe and some sequential tests.  This early 
> attempt follows the Java BlockingQueue interface, though it doesn't implement 
> it since I don't think there's a good reason for it to be Iterable.  

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

Reply via email to