[jira] [Commented] (HBASE-5992) Generalization of region move implementation + manage draining servers in bulk assign

["stack (JIRA)" <ji...@apache.org>]

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

stack commented on HBASE-5992:
------------------------------

FYI, you don't need to change the style here:

{code}
-      if (rst.isAlive()) liveServers.add(rst);
-      else LOG.info("Not alive " + rst.getName());
+      if (rst.isAlive()){
+        liveServers.add(rst);
+      }
{code}

Its ok having no curlies if statement fits one line.

In hbase code base, we have spaces around operators... i.e. this needs spaces {code} "+encodedName+" {code}

The refactoring that makes results int this method, {code}+    final List<ServerName> destServers = serverManager.createDestinationServersList(serverToExclude);{code} looks good.

Ditto for this change {code}+    this.serverManager.removeDeadNotExpiredServers(servers);{code} moving 'server' state out of AM and up into SM where it belongs.

Ugh.  There was a bunch of dup'd code that this patch removes.

createDestinationServersList and removeDeadNotExpiredServers do not need to be public methods (oh, hang on, createDestinationServersList needs to be so accessible from handlers.... does removeDeadNotExpiredServers also need to be?)

What does the big refactoring of TestDrainingServer do?

Patch looks good to me.











                
> Generalization of region move implementation + manage draining servers in bulk assign
> -------------------------------------------------------------------------------------
>
>                 Key: HBASE-5992
>                 URL: https://issues.apache.org/jira/browse/HBASE-5992
>             Project: HBase
>          Issue Type: Bug
>          Components: master, regionserver
>    Affects Versions: 0.96.0
>            Reporter: nkeywal
>            Assignee: nkeywal
>            Priority: Minor
>         Attachments: 5992.v2.patch
>
>
> The region move implementation now has now a similar behavior whatever the destination server is specified or not. This allows:
>  - to benefit from the improvement in HBASE-5877
>  - as a side effect to have the coprocessors calls when the destination server is not specified
>  
> This includes various fixes around draining servers. Draining servers were not excluded during a bulk assign. This is now fixed.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira