From 6e1907919ab4904e03d0b28d940f0826c201a7c7 Mon Sep 17 00:00:00 2001
From: tedyu <yuzhihong@gmail.com>
Date: Tue, 27 Jun 2017 07:03:59 -0700
Subject: [PATCH] HBASE-18272 Fix issue about
 RSGroupBasedLoadBalancer#roundRobinAssignment where BOGUS_SERVER_NAME is
 involved in two groups (chenxu)

---
 .../rsgroup/RSGroupBasedLoadBalancer.java     |  8 ++++-
 .../TestRSGroupBasedLoadBalancer.java         | 34 +++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

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 e2dd91c10c4..24baa62b98f 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
@@ -167,7 +167,13 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer {
                 regionMap.get(groupKey),
                 serverMap.get(groupKey));
         if(result != null) {
-          assignments.putAll(result);
+          if(result.containsKey(LoadBalancer.BOGUS_SERVER_NAME) &&
+              assignments.containsKey(LoadBalancer.BOGUS_SERVER_NAME)){
+            assignments.get(LoadBalancer.BOGUS_SERVER_NAME).addAll(
+              result.get(LoadBalancer.BOGUS_SERVER_NAME));
+          } else {
+            assignments.putAll(result);
+          }
         }
       }
     }
diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java
index 0f1e8497b80..9dd2c5c0520 100644
--- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java
+++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java
@@ -34,6 +34,7 @@ import org.apache.hadoop.hbase.rsgroup.RSGroupInfo;
 import org.apache.hadoop.hbase.rsgroup.RSGroupInfoManager;
 import org.apache.hadoop.hbase.master.assignment.AssignmentManager;
 import org.apache.hadoop.hbase.master.HMaster;
+import org.apache.hadoop.hbase.master.LoadBalancer;
 import org.apache.hadoop.hbase.master.MasterServices;
 import org.apache.hadoop.hbase.master.RegionPlan;
 import org.apache.hadoop.hbase.net.Address;
@@ -52,6 +53,8 @@ import java.security.SecureRandom;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -244,6 +247,37 @@ public class TestRSGroupBasedLoadBalancer {
     assertRetainedAssignment(inputForTest, servers, newAssignment);
   }
 
+  /**
+   * Test BOGUS_SERVER_NAME among groups do not overwrite each other
+   * @throws Exception
+   */
+  @Test
+  public void testRoundRobinAssignment() throws Exception {
+    List<ServerName> onlineServers = new ArrayList<ServerName>(servers.size());
+    onlineServers.addAll(servers);
+    List<HRegionInfo> regions = randomRegions(25);
+    int bogusRegion = 0;
+    for(HRegionInfo region : regions){
+      String group = tableMap.get(region.getTable());
+      if("dg3".equals(group) || "dg4".equals(group)){
+        bogusRegion++;
+      }
+    }
+    Set<Address> offlineServers = new HashSet<Address>();
+    offlineServers.addAll(groupMap.get("dg3").getServers());
+    offlineServers.addAll(groupMap.get("dg4").getServers());
+    for(Iterator<ServerName> it =  onlineServers.iterator(); it.hasNext();){
+      ServerName server = it.next();
+      Address address = server.getAddress();
+      if(offlineServers.contains(address)){
+        it.remove();
+      }
+    }
+    Map<ServerName, List<HRegionInfo>> assignments = loadBalancer
+        .roundRobinAssignment(regions, onlineServers);
+    assertEquals(bogusRegion, assignments.get(LoadBalancer.BOGUS_SERVER_NAME).size());
+  }
+
   /**
    * Asserts a valid retained assignment plan.
    * <p>