----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.cloudera.org/r/915/#review1353 -----------------------------------------------------------
Here's a few comments on yours. Actually, testing this patch on cluster brought up some issues. I think I should recast. I have some ideas on how. v2 coming. Will incorporate your belows. trunk/src/main/java/org/apache/hadoop/hbase/catalog/CatalogTracker.java <http://review.cloudera.org/r/915/#comment4495> I can change it (you get my intent but it still confused so I should change it). trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java <http://review.cloudera.org/r/915/#comment4496> Yeah, what you say. Let me fix up comments. trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java <http://review.cloudera.org/r/915/#comment4497> will do - stack On 2010-09-28 15:52:46, Jonathan Gray wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.cloudera.org/r/915/ > ----------------------------------------------------------- > > (Updated 2010-09-28 15:52:46) > > > Review request for hbase, stack and Jonathan Gray. > > > Summary > ------- > > This is patch from Stack, just putting up on rb. > > M src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java > Add test of case where HRegionInterface connection throws a > ConnectionException. Also tests two new verify root and meta > locations added to CatalogTracker. > M src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java > Change order in which we start up trackers in ZK. Also add blocking > until master is up to make it less likely we'll start before master > comes up, especially around the cluster start up situation. > M src/main/java/org/apache/hadoop/hbase/master/HMaster.java > Introduce new state on startup, the case where the cluster is > NOT a fresh startup and its NOT a cluster where all is fully > assigned. The repair the master needs run to fixup this new > state is not yet done; we throw a NotImplementedException for > now. TODO. Added new isRunningCluster checker used figuring > what the cluster condition is when master is joining. Not > comprehensive but good enough for now. > M src/main/java/org/apache/hadoop/hbase/catalog/CatalogTracker.java > Javadoc. > Added new verifyRootRegionLocation and verifyMetaRegionLocation. > Needed to verify whats in zk is actually locations of catalog > regions. > M src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java > Add fact that the verifying method, getRegionInfo, can throw > ConnectException > > > This addresses bug HBASE-3047. > http://issues.apache.org/jira/browse/HBASE-3047 > > > Diffs > ----- > > trunk/src/main/java/org/apache/hadoop/hbase/catalog/CatalogTracker.java > 1002359 > trunk/src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java > 1002359 > trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java 1002359 > trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java > 1002359 > trunk/src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java > 1002359 > > Diff: http://review.cloudera.org/r/915/diff > > > Testing > ------- > > > Thanks, > > Jonathan > >