[
https://issues.apache.org/jira/browse/HBASE-3053?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12917059#action_12917059
]
HBase Review Board commented on HBASE-3053:
-------------------------------------------
Message from: [email protected]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/925/#review1378
-----------------------------------------------------------
This is great. Follows the ugly pattern that was in place for RSs. Just some
comments below... see what you think.
trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java
<http://review.cloudera.org/r/925/#comment4564>
This needs to be data member? Its not just used once locally?
trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java
<http://review.cloudera.org/r/925/#comment4565>
Why do this in constructor? Why not declare and allocate at same time?
trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java
<http://review.cloudera.org/r/925/#comment4566>
No biggie but I like the previous syntax
trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java
<http://review.cloudera.org/r/925/#comment4567>
Can you read this from zk instead?
trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<http://review.cloudera.org/r/925/#comment4568>
Go ahead and remove it. It used to be used figuring if fresh startup but
now its just logged to help debugging what state was like on master startup.
trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<http://review.cloudera.org/r/925/#comment4569>
No need for this comment. CHeck the javadoc on this method. It used to be
detailed steps IIRC. The detail no longer is correct.
trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<http://review.cloudera.org/r/925/#comment4570>
Yeah, this 3. is a bit odd in here w/o its 1. and 2.
trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<http://review.cloudera.org/r/925/#comment4571>
Does this if span too much? Could you test stopped before calling
initialization? loop already does stopped?
trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<http://review.cloudera.org/r/925/#comment4572>
What services are started in teh constructor now?
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<http://review.cloudera.org/r/925/#comment4575>
Do we lose messages here? Important we not drop split, etc. FYI, we need
to do some more thought around split message and master failover. See over in
master where we register servers on region server report for note.
trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java
<http://review.cloudera.org/r/925/#comment4576>
Yeah, whats this about?
trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java
<http://review.cloudera.org/r/925/#comment4577>
How about a test where there are RIT and master joins and assertion it does
the fixup?
There is white space above the 'yay'.
- stack
> Add ability to have multiple Masters LocalHBaseCluster for test writing
> -----------------------------------------------------------------------
>
> Key: HBASE-3053
> URL: https://issues.apache.org/jira/browse/HBASE-3053
> Project: HBase
> Issue Type: Improvement
> Components: master, test
> Reporter: Jonathan Gray
> Assignee: Jonathan Gray
> Fix For: 0.90.0
>
>
> To really be able to unit test the new master properly, we need to be able to
> have multiple masters running at once within a single logical cluster.
> Should expose some methods like getActiveMaster() and isActiveMaster() as
> well as a simple way to kill an individual master / kill the active master.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
