hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bikas Saha (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-2681) Add ZK client for leader election
Date Sat, 14 Jan 2012 03:26:40 GMT

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

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

Some more comments from chat session
# Change the method name to init(). Annotate it @Before. It will be automatically called before
tests.
# Use @Expected for tests that expect exception
Done
# Add class level javadoc.
This is already in the second patch. If by this you mean comments before the class declaration.
# #Init need not catch IOException. Just throw it. The test will fail.
I know it wont throw the exception in the test. But it has to be handled to keep the compiler
happy. So I handled it locally there instead of adding "throws IOException in every method
of the test"
# You can reduce several lines of code by using a static byte[] DATA;
done
# can you add test where jointElection() is called twice and the second call is NO-OP
# Many times where processResult is called back to back can be in for loop
this helps me walk the scenarios better than in a loop
# Why should 4 errors of connection loss result in fatalError?
Because the elector has tried its best to connect to Zookeeper and failed. We can revisit
this based observed failures at a later time.
# testStatNodeError already covers some part of testCreateNodeResultRetryBecomeActive
yes. thats because it is trying to walk through a logical scenario. so I let it be.
# Instead of catching InterruptedException, you can just throw it
same code cleanliness as above. I know this exception will not get thrown in the test. so
want to make local changes to keep the compiler happy.

>>Please use System.arraycopy() instead of byte[] clone.
done.
>>Split process into two different methods processZkEvent and processZnodeEvent?
The function is still small enough to let it be. Will do this later when more logic might
get added if we do group participation. At that point processZnodeEvent itself will need division
into lock znode and parent znode.

>>can you add test where jointElection() is called twice and the second call is NO-OP
it was there in test processResult callback but got changed to enterNeutralMode when I changed
that test. now I enhanced testCreateNodeResultBecomeActive() to check that there is no double
master call and added another test to check that there is no double slave call for expected
scenarios. now all 3 states are covered.

Will upload the patch with all these changes.
Thanks

                
> 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, 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

        

Mime
View raw message