hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jonathan Gray" <jg...@apache.org>
Subject Re: Review Request: hbase-3112 Enable and disable of table needs a bit of loving in new master
Date Mon, 08 Nov 2010 19:50:55 GMT

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


Mime
View raw message