From c2e3d52080ed28dbffbb83314dff4a75f072f400 Mon Sep 17 00:00:00 2001
From: Xiang Li <lixiang@freewheel.tv>
Date: Sat, 3 Feb 2018 04:46:55 +0000
Subject: [PATCH] HBASE-19917 Improve RSGroupBasedLoadBalancer#filterServers()
 to be more efficient

Signed-off-by: tedyu <yuzhihong@gmail.com>
---
 .../rsgroup/RSGroupBasedLoadBalancer.java     | 22 +++++++++++--------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java
index 550f7343b22..c47972d8b10 100644
--- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java
+++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java
@@ -20,7 +20,6 @@ package org.apache.hadoop.hbase.rsgroup;
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -29,6 +28,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
+
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.ClusterMetrics;
 import org.apache.hadoop.hbase.HBaseIOException;
@@ -288,16 +288,20 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer {
    *          List of servers which are online.
    * @return the list
    */
-  private List<ServerName> filterServers(Collection<Address> servers,
-      Collection<ServerName> onlineServers) {
-    ArrayList<ServerName> finalList = new ArrayList<ServerName>();
-    for (Address server : servers) {
-      for(ServerName curr: onlineServers) {
-        if(curr.getAddress().equals(server)) {
-          finalList.add(curr);
-        }
+  private List<ServerName> filterServers(Set<Address> servers,
+                                         List<ServerName> onlineServers) {
+    /**
+     * servers is actually a TreeSet (see {@link org.apache.hadoop.hbase.rsgroup.RSGroupInfo}),
+     * having its contains()'s time complexity as O(logn), which is good enough.
+     * TODO: consider using HashSet to pursue O(1) for contains() throughout the calling chain
+     * if needed. */
+    ArrayList<ServerName> finalList = new ArrayList<>();
+    for (ServerName onlineServer : onlineServers) {
+      if (servers.contains(onlineServer.getAddress())) {
+        finalList.add(onlineServer);
       }
     }
+
     return finalList;
   }