hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From st...@duboce.net
Subject Re: Review Request: hbase-3112 Enable and disable of table needs a bit of loving in new master
Date Tue, 09 Nov 2010 05:16:27 GMT


> 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
> 
>


Mime
View raw message