> On 2010-10-28 13:43:01, stack wrote:
> > +1  Looks great.  A nice little bomb thrown into the midst of new master.
> 
> stack wrote:
>     Oh, reconcile this patch with the one I just posted here on rb.  Add in 
> the extra logging in close and open handlers in particular.  Helps debugging 
> knowing which handler for which server is running.

Will make your changes from below on commit.  Thanks for review Stack!


> On 2010-10-28 13:43:01, stack wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java, line 259
> > <http://review.cloudera.org/r/1090/diff/4/?file=16161#file16161line259>
> >
> >     You sure the RS you find here is same as the one you passed in?  Do 
> > rst.getRegionServer.

In practice, it has always been the right one.  I will add a check of 
HServerInfo that it's definitely same.


> On 2010-10-28 13:43:01, stack wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java, line 351
> > <http://review.cloudera.org/r/1090/diff/4/?file=16161#file16161line351>
> >
> >     Ditto here.

K.  Ditto here for me too.


> On 2010-10-28 13:43:01, stack wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/catalog/CatalogTracker.java, 
> > line 430
> > <http://review.cloudera.org/r/1090/diff/4/?file=16162#file16162line430>
> >
> >     This is fine but maybe we should make the equating sloppier...  just do 
> > a contains 'Connection reset'?

Sure.  Will do.


> On 2010-10-28 13:43:01, stack wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java, line 
> > 99
> > <http://review.cloudera.org/r/1090/diff/4/?file=16165#file16165line99>
> >
> >     Isn't there an accessor for this?  Why you need to make this 
> > non-private?

I'm using it in my unit test.  This is how I know when the master has started 
and is done processing dead server.  See TestRollingRestart. 
waitForRSShutdownToStartAndFinish()


> On 2010-10-28 13:43:01, stack wrote:
> > trunk/src/test/java/org/apache/hadoop/hbase/master/TestRollingRestart.java, 
> > line 353
> > <http://review.cloudera.org/r/1090/diff/4/?file=16170#file16170line353>
> >
> >     oh nelly

yeah this test is a fun one.  i'm going to commit it @ 30 total regions but I 
have it passing at 1000 and killing my eclipse at 2000 but working up to the 
point of death.


> On 2010-10-28 13:43:01, stack wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java,
> >  line 819
> > <http://review.cloudera.org/r/1090/diff/4/?file=16166#file16166line819>
> >
> >     Add this string to the one above... to the 'Serving as' string.. its 
> > trying to be all vital statistics about an HRS post construction.

k.


- Jonathan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1090/#review1688
-----------------------------------------------------------


On 2010-10-28 12:00:06, Jonathan Gray wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1090/
> -----------------------------------------------------------
> 
> (Updated 2010-10-28 12:00:06)
> 
> 
> Review request for hbase and stack.
> 
> 
> Summary
> -------
> 
> First go at a TestRollingRestart.  Needs more work / harder tests per 
> comments in the test.
> 
> 
> Diffs
> -----
> 
>   trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java 1027683 
>   trunk/src/main/java/org/apache/hadoop/hbase/catalog/CatalogTracker.java 
> 1027683 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 
> 1027683 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java 1027683 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 
> 1027683 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 
> 1027683 
>   trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1027683 
>   trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1027683 
>   trunk/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 
> 1027683 
>   trunk/src/test/java/org/apache/hadoop/hbase/master/TestRollingRestart.java 
> PRE-CREATION 
> 
> Diff: http://review.cloudera.org/r/1090/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jonathan
> 
>

Reply via email to