[ 
https://issues.apache.org/jira/browse/HBASE-4298?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13107149#comment-13107149
 ] 

Ted Yu commented on HBASE-4298:
-------------------------------

I reviewed the patch for trunk.

For AssignmentManager.java:
{code}
+        ", exclude=" + drainingServers + ") available servers");
{code}
I think we only need to log the number of draining servers.

For ServerManager.java:
{code}
+  /** Map of region servers that should not get any more new regions */
+  private final Map<ServerName, HServerLoad> drainingServers =
+    new ConcurrentHashMap<ServerName, HServerLoad>();
{code}
The javadoc should state that keys of the map are region servers.

I think removeServerFromDrainList() should return a boolean. 
ServerManager.isServerOnline(sn) should be used instead of checking 
HServerLoad. If sn isn't online, the method should return false. Otherwise true 
is returned.

You may consider doing similar action in addServerToDrainList().

I wonder if Map is needed for drainingServers because it is private and 
getDrainingServersList() only returns the keySet.

For DrainingServerTracker.java, please remove year.
The handling of calling this.serverManager methods is different between add() 
and remove(): one inside synchronized block, one outside. Is there a reason ?

More to follow.

> Support to drain RS nodes through ZK
> ------------------------------------
>
>                 Key: HBASE-4298
>                 URL: https://issues.apache.org/jira/browse/HBASE-4298
>             Project: HBase
>          Issue Type: Improvement
>          Components: master
>    Affects Versions: 0.90.4
>         Environment: all
>            Reporter: Aravind Gottipati
>            Priority: Critical
>              Labels: patch
>             Fix For: 0.92.0, 0.90.5
>
>
> HDFS currently has a way to exclude certain datanodes and prevent them from 
> getting new blocks.  HDFS goes one step further and even drains these nodes 
> for you.  This enhancement is a step in that direction.
> The idea is that we mark nodes in zookeeper as draining nodes.  This means 
> that they don't get any more new regions.  These draining nodes look exactly 
> the same as the corresponding nodes in /rs, except they live under /draining.
> Eventually, support for draining them can be added.  I am submitting two 
> patches for review - one for the 0.90 branch and one for trunk (in git).
> Here are the two patches
> 0.90 - 
> https://github.com/aravind/hbase/commit/181041e72e7ffe6a4da6d82b431ef7f8c99e62d2
> trunk - 
> https://github.com/aravind/hbase/commit/e127b25ae3b4034103b185d8380f3b7267bc67d5
> I have tested both these patches and they work as advertised.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to