[
https://issues.apache.org/jira/browse/HBASE-3053?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12918381#action_12918381
]
HBase Review Board commented on HBASE-3053:
-------------------------------------------
Message from: "Jonathan Gray" <[email protected]>
bq. On 2010-10-01 14:44:53, stack wrote:
bq. > trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java, line
65
bq. > <http://review.cloudera.org/r/925/diff/5/?file=13275#file13275line65>
bq. >
bq. > This needs to be data member? Its not just used once locally?
I copied from same way it's done for RS. It is used in constructor and then
also in addMaster() method which is public and can be called by a client after
initialization.
bq. On 2010-10-01 14:44:53, stack wrote:
bq. > trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java, line
141
bq. > <http://review.cloudera.org/r/925/diff/5/?file=13275#file13275line141>
bq. >
bq. > Why do this in constructor? Why not declare and allocate at same
time?
Again, copied from how it's done for RSs. Can move it into the declaration for
both.
bq. On 2010-10-01 14:44:53, stack wrote:
bq. > trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java, line
224
bq. > <http://review.cloudera.org/r/925/diff/5/?file=13275#file13275line224>
bq. >
bq. > No biggie but I like the previous syntax
Me too. I was smacked two years ago doing one-line ifs and such, told that
HBase form was to always have braces. I had my IDE setup to not allow the
former but have changed it now to permit one liners like this (when I hit code
cleanup, it did this automatically). Will revert.
bq. On 2010-10-01 14:44:53, stack wrote:
bq. > trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java, line
267
bq. > <http://review.cloudera.org/r/925/diff/5/?file=13275#file13275line267>
bq. >
bq. > Can you read this from zk instead?
Yes, but I don't think that would be very useful for unit tests since you'd be
relying on correct behavior/interaction of masters w/ ZK. The way this is done
is super simple... when a master becomes active, it flips a boolean. My
thought was to try and make this as fool proof as possible.
bq. On 2010-10-01 14:44:53, stack wrote:
bq. > trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java, line 217
bq. > <http://review.cloudera.org/r/925/diff/5/?file=13276#file13276line217>
bq. >
bq. > No need for this comment. CHeck the javadoc on this method. It
used to be detailed steps IIRC. The detail no longer is correct.
Revamped all comments. Check next diff.
bq. On 2010-10-01 14:44:53, stack wrote:
bq. > trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java, line 322
bq. > <http://review.cloudera.org/r/925/diff/5/?file=13276#file13276line322>
bq. >
bq. > Does this if span too much? Could you test stopped before calling
initialization? loop already does stopped?
The if is immediately after we break out of blocking to become the active
master. This check is for the case where we are backup master but then we are
being shutdown. We get interrupted from blocking on becoming master, then we
do this check. What is inside the if{} is the stuff we don't want to run (only
run if you are actually active master). The stuff below the if is the stuff
that is run in both cases of a master shutdown (backup master being shutdown,
or active master being shutdown in master or cluster shutdown).
Not the prettiest, tried a couple ways, but this ended up being simplest. Will
add some comments.
bq. On 2010-10-01 14:44:53, stack wrote:
bq. > trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java, line 345
bq. > <http://review.cloudera.org/r/925/diff/5/?file=13276#file13276line345>
bq. >
bq. > What services are started in teh constructor now?
Changed comment. It's services that are started whether backup or active
master.
bq. On 2010-10-01 14:44:53, stack wrote:
bq. >
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java,
line 674
bq. > <http://review.cloudera.org/r/925/diff/5/?file=13278#file13278line674>
bq. >
bq. > 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.
Shouldn't be any different than before. If we get IOE, we wouldn't have any
messages returned from the method.
Agree we need to more thought on this. Why piggyback this on heartbeat? Maybe
direct RPC and master can return something that says success? If fail, then
it'll retry from RS?
bq. On 2010-10-01 14:44:53, stack wrote:
bq. >
trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java,
line 133
bq. > <http://review.cloudera.org/r/925/diff/5/?file=13283#file13283line133>
bq. >
bq. > How about a test where there are RIT and master joins and assertion
it does the fixup?
bq. >
bq. > There is white space above the 'yay'.
That's the plan.
bq. On 2010-10-01 14:44:53, stack wrote:
bq. >
trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java,
line 86
bq. > <http://review.cloudera.org/r/925/diff/5/?file=13283#file13283line86>
bq. >
bq. > Yeah, whats this about?
Will try and get rid of it all. The test is just a first go. I'd like to get
this committed because HMaster and HRS keep changing and I have to rebase the
changes.
So I'm looking for a +1 to commit to get the changes in and then I'll continue
working on fancier tests with this stuff.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/925/#review1378
-----------------------------------------------------------
> 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.
