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 Mon, 08 Nov 2010 20:01:32 GMT


> On 2010-11-08 11:50:55, Jonathan Gray wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java, line
1143
> > <http://review.cloudera.org/r/1187/diff/1/?file=16872#file16872line1143>
> >
> >     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?

Removed it.  It confuses (see above for exhibit A).

If problem doing bulk assign, we'll crash out master.  Otherwise, timeout of RIT should fix
bulk assign stragglers.


> On 2010-11-08 11:50:55, Jonathan Gray wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java, line
1149
> > <http://review.cloudera.org/r/1187/diff/1/?file=16872#file16872line1149>
> >
> >     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.

BulkAssigner class comment says -- perhaps a little curtly -- what it does?  I'll move it
out.  The implementations though I'll leave beside where they are used -- in class.


- stack


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1187/#review1840
-----------------------------------------------------------


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


Mime
View raw message