----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.cloudera.org/r/1187/#review1840 -----------------------------------------------------------
Going to continue review on v2 patch trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java <http://review.cloudera.org/r/1187/#comment5962> whitespace (and we don't have IOException in javadoc but that's not your fault) trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java <http://review.cloudera.org/r/1187/#comment5964> What does this mean? done vs. not done? I think we should be more descriptive in the logging (if done, then we've completed assignment of regions on cluster startup). But if not done, on startup, what does this mean? There's comment later that RIT timeouts should fix it up, so should be in log message here? Or on startup case of bulk assign, should we fail startup here if this doesn't pass? trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java <http://review.cloudera.org/r/1187/#comment5961> Can we move all this stuff into a separate class? AssignmentManager is getting huge. Maybe BulkAssign could be class that contains all these other class definitions? Also gives good opportunity in class comment to describe in general how this stuff works. - Jonathan On 2010-11-08 11:47:12, stack wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.cloudera.org/r/1187/ > ----------------------------------------------------------- > > (Updated 2010-11-08 11:47:12) > > > Review request for hbase and Jonathan Gray. > > > Summary > ------- > > Renamed ZKTableDisable as ZKTable, making it a generic zk util for managing > 'tables'. > Added enabing/disabling states to table the current set of enabled/disabled > only. > > M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java > (createSetData): Added. > M src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java > Removed offlining region utility methods no longer used. > (We do it now over in MetaEditor) > M src/main/java/org/apache/hadoop/hbase/HRegionInfo.java > Javadoc. > M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java > Add a base abstract class to do 'bulk assignments'. Redo > assignAllUserRegions to use subclass of new bulk assigner class. > Added isTableEnabled, disablingTable, enablingTable. > M > src/main/java/org/apache/hadoop/hbase/master/handler/DisableTableHandler.java > Redid to use new bulk assigner class. > M src/main/java/org/apache/hadoop/hbase/master/handler/EnableTableHandler.java > A Added TestZKTable > > > This addresses bug hbase-3112. > http://issues.apache.org/jira/browse/hbase-3112 > > > Diffs > ----- > > trunk/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 1032652 > trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java > 1032652 > trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java > 1032652 > trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java 1032652 > > trunk/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java > 1032652 > > trunk/src/main/java/org/apache/hadoop/hbase/master/handler/DisableTableHandler.java > 1032652 > > trunk/src/main/java/org/apache/hadoop/hbase/master/handler/EnableTableHandler.java > 1032652 > trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java > 1032652 > trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKTable.java > PRE-CREATION > trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKTableDisable.java > 1032652 > trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1032652 > trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java > 1032652 > trunk/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKTable.java > PRE-CREATION > > Diff: http://review.cloudera.org/r/1187/diff > > > Testing > ------- > > > Thanks, > > stack > >
