From f0ba7922f3963276304aee1d354413bcf966e563 Mon Sep 17 00:00:00 2001
From: tedyu <yuzhihong@gmail.com>
Date: Fri, 12 Jan 2018 12:10:22 -0800
Subject: [PATCH] HBASE-19752 RSGroupBasedLoadBalancer#getMisplacedRegions()
 should handle the case where rs group cannot be determined

---
 .../hbase/rsgroup/RSGroupBasedLoadBalancer.java | 17 ++++++++++++-----
 .../balancer/TestRSGroupBasedLoadBalancer.java  | 17 +++++++++++++++++
 2 files changed, 29 insertions(+), 5 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 65053b87ccf..550f7343b22 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
@@ -301,7 +301,8 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer {
     return finalList;
   }
 
-  private Set<RegionInfo> getMisplacedRegions(
+  @VisibleForTesting
+  public Set<RegionInfo> getMisplacedRegions(
       Map<RegionInfo, ServerName> regions) throws IOException {
     Set<RegionInfo> misplacedRegions = new HashSet<>();
     for(Map.Entry<RegionInfo, ServerName> region : regions.entrySet()) {
@@ -309,10 +310,16 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer {
       ServerName assignedServer = region.getValue();
       RSGroupInfo info = rsGroupInfoManager.getRSGroup(rsGroupInfoManager.
               getRSGroupOfTable(regionInfo.getTable()));
-      if (assignedServer != null &&
-          (info == null || !info.containsServer(assignedServer.getAddress()))) {
-        RSGroupInfo otherInfo = null;
-        otherInfo = rsGroupInfoManager.getRSGroupOfServer(assignedServer.getAddress());
+      if (assignedServer == null) {
+        LOG.debug("There is no assigned server for {}", region);
+        continue;
+      }
+      RSGroupInfo otherInfo = rsGroupInfoManager.getRSGroupOfServer(assignedServer.getAddress());
+      if (info == null && otherInfo == null) {
+        LOG.warn("Couldn't obtain rs group information for {} on {}", region, assignedServer);
+        continue;
+      }
+      if ((info == null || !info.containsServer(assignedServer.getAddress()))) {
         LOG.debug("Found misplaced region: " + regionInfo.getRegionNameAsString() +
             " on server: " + assignedServer +
             " found in group: " +  otherInfo +
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 e783a57270d..f36199e522c 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
@@ -75,6 +75,7 @@ public class TestRSGroupBasedLoadBalancer {
   private static SecureRandom rand;
 
   static String[]  groups = new String[] { RSGroupInfo.DEFAULT_GROUP, "dg2", "dg3", "dg4" };
+  static TableName table0 = TableName.valueOf("dt0");
   static TableName[] tables =
       new TableName[] { TableName.valueOf("dt1"),
           TableName.valueOf("dt2"),
@@ -214,6 +215,20 @@ public class TestRSGroupBasedLoadBalancer {
     assertClusterAsBalanced(loadMap);
   }
 
+  @Test
+  public void testGetMisplacedRegions() throws Exception {
+    // Test case where region is not considered misplaced if RSGroupInfo cannot be determined
+    Map<RegionInfo, ServerName> inputForTest = new HashMap<>();
+    RegionInfo ri = RegionInfoBuilder.newBuilder(table0)
+        .setStartKey(new byte[16])
+        .setEndKey(new byte[16])
+        .setSplit(false)
+        .setRegionId(regionId++)
+        .build();
+    inputForTest.put(ri, servers.iterator().next());
+    Set<RegionInfo> misplacedRegions = loadBalancer.getMisplacedRegions(inputForTest);
+    assertFalse(misplacedRegions.contains(ri));
+  }
   /**
    * Test the cluster startup bulk assignment which attempts to retain assignment info.
    */
@@ -533,6 +548,8 @@ public class TestRSGroupBasedLoadBalancer {
       tableMap.put(tables[i], groupName);
       tds.add(htd);
     }
+    tableMap.put(table0, "");
+    tds.add(new HTableDescriptor(table0));
     return tds;
   }