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

Reply via email to