[ 
https://issues.apache.org/jira/browse/HDFS-2681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13185757#comment-13185757
 ] 

Bikas Saha commented on HDFS-2681:
----------------------------------

>> General - How is multithreaded use for this library handled?
All public methods are synchronized

>>Add javadoc to the class on how to use the class. Also callback interface can 
>>be described in more detail on when the call back is made and perhaps some 
>>description of what is expected of the app. notifyError() particularly needs 
>>better documentation on when to expect this callback.
Please document the enumerations.
Done in second patch

>>Constructor should check for null - at least for call back passed. Otherwise 
>>you will get null pointer exception.
Done

>>joinElection() you may want to copy the byte[] data passed or at least 
>>document that the data[] must not be changed by the caller.
Done

>>#getNewZooKeeper() seems unnecessary and can be removed. Creation of 
>>ZooKeeper() can be moved to createConnection() it self.
This is to pass in a mock zookeeper for testing

>>Make member variable that are initialized only once in the constructor final.
Done in second patch

>>activeData could be better name for appData.
All app's can pass in data (which may go into future per app nodes). Only 
active app's data makes it to the lock. So I think the name is good.

>>Please check if all the params are documented in methods. For example 
>>constructor is missing one of the params in the doc. Same is true with 
>>exceptions thrown.
Done in second patch

>>quitElection() should not check zkClient non null, as terminateConnection 
>>already checks it.
Yeah. I forgot to remove that check after I refactored stuff into the reset() 
method

>>getActiveData() - how about not throwing KeeperException? Also 
>>ActiveNotFoundException should wrap the exception caught from ZK.
Its hard to differentiate exceptions inside KeeperException. There is not much 
the elector can do about them. The only commonly expected exception would be 
getting leader data when no leader exists and that has been handled as part of 
the elector API via a new exception.

                
> Add ZK client for leader election
> ---------------------------------
>
>                 Key: HDFS-2681
>                 URL: https://issues.apache.org/jira/browse/HDFS-2681
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: ha
>    Affects Versions: HA branch (HDFS-1623)
>            Reporter: Suresh Srinivas
>            Assignee: Bikas Saha
>             Fix For: HA branch (HDFS-1623)
>
>         Attachments: HDFS-2681.HDFS-1623.patch, HDFS-2681.HDFS-1623.patch, 
> Zookeeper based Leader Election and Monitoring Library.pdf
>
>
> ZKClient needs to support the following capabilities:
> # Ability to create a znode for co-ordinating leader election.
> # Ability to monitor and receive call backs when active znode status changes.
> # Ability to get information about the active node.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to