Patrick Hunt commented on ZOOKEEPER-78:

These comments are relative to the java code:

cleanup imports

writelocktest indentation problem line 87

do we want to talk about leaderelection in locks, or just implement another le 
in recipes (small wrapper)?

znodename.java - log failure in exception handlers

protocolsupport retrydelay eats interrupt, ie lock cannot be interrupted

shouldn't writelock.lock throw connectionloss if closed?
  the javadoc is not clear - what happens in the "later" case? does this block? 
what happens if expired or discon? etc...

line 148 of writelock
                    long sessionId = zookeeper.getSessionId();
what ensures that sessionid you get back is valid? (ie non-zero)
This is pretty large anony class - why not have a specific static class 
Plus all in one method rather than broken out (even indentation is deep)
Would be easier to maintain if broken into several methods

writelock line 208
                                    try {
                                    } catch (Exception e) {
                                        LOG.warn("Failed to acquire lock: " + 
e, e);
what happens if lock returns false (isclosed for example)?

Some of these classes look reusable (prot/name/op) - should they be in this 
recipe or elsewhere?
If we want to enable additional recipes to be easily added based on the work 
you've done (blueprint)
 we should try to make these easily reusable.

> added a high level protocol/feature - for easy Leader Election or exclusive 
> Write Lock creation
> -----------------------------------------------------------------------------------------------
>                 Key: ZOOKEEPER-78
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-78
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>    Affects Versions: 3.0.0
>            Reporter: james strachan
>            Assignee: Mahadev konar
>             Fix For: 3.2.0
>         Attachments: patch_with_including_Benjamin's_fix.patch, 
> using_zookeeper_facade.patch, ZOOKEEPER-78.patch, ZOOKEEPER-78.patch, 
> ZOOKEEPER-78.patch, ZOOKEEPER-78.patch, ZOOKEEPER-78.patch, 
> ZOOKEEPER-78.patch, ZOOKEEPER-78.patch
> Here's a patch which adds a little WriteLock helper class for performing 
> leader elections or creating exclusive locks in some directory znode. Note 
> its an early cut; am sure we can improve it over time. The aim is to avoid 
> folks having to use the low level ZK stuff but provide a simpler high level 
> abstraction.

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