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

Xiang Li commented on HBASE-22009:
----------------------------------

Hi [~xucang], I uploaded the patch v000 for branch-1, which is dedicated for 
that branch. The reason why the previous patch fails branch-1 is due to the 
following statement:
{code:java}
if (!serversInOtherGroup.contains(server))
{code}
"serversInOtherGroup" is a Set of Address, but "server" is an instance of 
ServerName. Set#contains() could accepts an "object" as the input, so no 
class/type check here... and serversInOtherGroup.contains(server) always 
returns false.
 The patch for master branch has the same statement, but its "server" is an 
instance of Address, which is correct...The differences between master and 
branch-1 causes the confusion here... Sorry, I should have noticed those and 
uploaded the patch for branch-1 at the very beginning...
 I also make some changes on the variable names, trying to avoid confusion in 
the future.
 Please review the patch v000 for branch-1 at your convenience. It passes all 
UT on my local machines.

> Improve RSGroupInfoManagerImpl#getDefaultServers()
> --------------------------------------------------
>
>                 Key: HBASE-22009
>                 URL: https://issues.apache.org/jira/browse/HBASE-22009
>             Project: HBase
>          Issue Type: Improvement
>          Components: rsgroup
>            Reporter: Xiang Li
>            Assignee: Xiang Li
>            Priority: Minor
>             Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
>         Attachments: HBASE-22009.master.000.patch, 
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet<Address> getDefaultServers() throws IOException {
>   SortedSet<Address> defaultServers = Sets.newTreeSet();
>   for (ServerName serverName : getOnlineRS()) {
>     Address server = Address.fromParts(serverName.getHostname(), 
> serverName.getPort());
>     boolean found = false;
>     for (RSGroupInfo rsgi : listRSGroups()) {
>       if (!RSGroupInfo.DEFAULT_GROUP.equals(rsgi.getName()) && 
> rsgi.containsServer(server)) {
>         found = true;
>         break;
>       }
>     }
>     if (!found) {
>       defaultServers.add(server);
>     }
>   }
>   return defaultServers;
> }
> {code}
> That is a logic of 2 nest loops. And for each server, listRSGroups() 
> allocates a new LinkedList and calls Map#values(), both of which are very 
> heavy operations.
> Maybe the inner loop could be moved out, that is
> # Build a list of servers of other groups than default group
> # Iterate each online servers and check if it is in the list above. If it is 
> not, then it belongs to default group.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to