----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.cloudera.org/r/1187/#review1853 -----------------------------------------------------------
Ship it! I can live without the state-checking methods in ZKTable but it's fine, no big deal as is. Only issue is that we're hitting ZK on each assign(). Otherwise, if tests are passing and it's working for you up on cluster, I'm +1 on this. If you add async/sync support, let's be sure to nail the javadoc. trunk/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java <http://review.cloudera.org/r/1187/#comment6017> why describe 'enabling' state in our client API? do we let out 'enabling' at all anywhere? and do we need to make mention that if it seems to never finish, this method should be retried? trunk/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java <http://review.cloudera.org/r/1187/#comment6019> same. i think the javadoc is descriptive enough with this explanation of states. just not sure if we need any javadoc about retrying? trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java <http://review.cloudera.org/r/1187/#comment6022> isTableDisabling is actually a ZK read rather than checking in-memory state. Should be move enabling/disabling to in-memory state as well? I'm not sure it's a good idea to have to read from ZK on every assign() call, especially for something as rare as disabling. - Jonathan On 2010-11-08 16:09:44, stack wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.cloudera.org/r/1187/ > ----------------------------------------------------------- > > (Updated 2010-11-08 16:09:44) > > > 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/HBaseAdmin.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/BulkAssigner.java > PRE-CREATION > 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/main/ruby/hbase/admin.rb 1032778 > trunk/src/main/ruby/shell.rb 1032778 > trunk/src/main/ruby/shell/commands/disable.rb 1032778 > trunk/src/main/ruby/shell/commands/enable.rb 1032778 > trunk/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.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 > >
