> On 2010-11-08 17:04:25, Jonathan Gray wrote: > > 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.
I want the state-checking methods to flag bad state transitions. On hitting zk on each assign, pardon me, I didn't grok what you were on about when you talked this up earlier. I get it now. Will fix. Tests are failing because I changed sematics; disable/enable are now async where before they were sync. Will do your suggestion of creating a sync version if only to make tests pass (should only be for use of tests). > On 2010-11-08 17:04:25, Jonathan Gray wrote: > > trunk/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java, line 447 > > <http://review.cloudera.org/r/1187/diff/3/?file=16953#file16953line447> > > > > 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? ok. removed mention of states. > On 2010-11-08 17:04:25, Jonathan Gray wrote: > > trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java, > > line 659 > > <http://review.cloudera.org/r/1187/diff/3/?file=16955#file16955line659> > > > > 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. Will fix in next patch. - stack ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.cloudera.org/r/1187/#review1853 ----------------------------------------------------------- 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 > >
