----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.cloudera.org/r/829/#review1173 -----------------------------------------------------------
I like this. Naming seems fine to me. We're making great strides in cleaning these things up, great stuff Todd. I guess we're in need of standardizing the methods in HM and HRS like toString, get*Name, etc... src/main/java/org/apache/hadoop/hbase/master/HMaster.java <http://review.cloudera.org/r/829/#comment4026> The toString() is supposed to give a unique name for the instance, especially useful in unit tests with multiple masters and such. Not sure where all we're using getServerName but might make sense for these to be the same. - Jonathan On 2010-09-13 18:36:29, Todd Lipcon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.cloudera.org/r/829/ > ----------------------------------------------------------- > > (Updated 2010-09-13 18:36:29) > > > Review request for hbase, stack and Jonathan Gray. > > > Summary > ------- > > This isn't really cleaned up, but wanted to gather opinions before I put in > more effort. Does this kind of refactor seem good? Is the Guava "Service" > class what we want to use or should we just write our own similar interface? > > > This addresses bug HBASE-2993. > http://issues.apache.org/jira/browse/HBASE-2993 > > > Diffs > ----- > > src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java bc0a62f > src/main/java/org/apache/hadoop/hbase/master/HMaster.java bc21a1e > src/main/java/org/apache/hadoop/hbase/master/HMasterCommandLine.java > c675db9 > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java > bba7b67 > src/main/java/org/apache/hadoop/hbase/service/ServiceUtils.java > PRE-CREATION > src/main/java/org/apache/hadoop/hbase/service/ServiceWithMainThread.java > PRE-CREATION > src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java 4a9f1c3 > src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java b7abb51 > src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 9cc1168 > src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 9407c1e > src/test/java/org/apache/hadoop/hbase/TestHBaseTestingUtility.java 1cd88d3 > src/test/java/org/apache/hadoop/hbase/TestInfoServers.java daffe02 > src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 6908111 > > src/test/java/org/apache/hadoop/hbase/master/TestKillingServersFromMaster.java > f5fd243 > > src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java > a172e2c > > src/test/java/org/apache/hadoop/hbase/regionserver/DisabledTestRegionServerExit.java > 5b8b464 > src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestLogRolling.java > 287f1fb > > Diff: http://review.cloudera.org/r/829/diff > > > Testing > ------- > > Tried to run unit tests, but plenty failed. I think they're failing on trunk, > too, though. Will do more testing and another round of review before I > actually claim this is ready to commit. > > > Thanks, > > Todd > >
