HBASE-23944 The method setClusterLoad of SimpleLoadBalancer is incorr…

…ect when balance by table (apache#1243) Signed-off-by: Guanghao Zhang <zghao@apache.org>
infraio · Mar 9, 2020 · 352ab40 · 352ab40
1 parent 8e6e8dd
commit 352ab40
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 11 deletions.
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/SimpleLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/SimpleLoadBalancer.java
@@ -107,16 +107,21 @@ void setNextRegionForUnload(int nextRegionForUnload) {
   }
 
   @Override
-  public void setClusterLoad(Map<TableName, Map<ServerName, List<RegionInfo>>> clusterLoad){
+  public void setClusterLoad(Map<TableName, Map<ServerName, List<RegionInfo>>> clusterLoad) {
     serverLoadList = new ArrayList<>();
+    Map<ServerName, Integer> server2LoadMap = new HashMap<>();
     float sum = 0;
-    for(Map.Entry<TableName, Map<ServerName, List<RegionInfo>>> clusterEntry : clusterLoad.entrySet()){
-      for(Map.Entry<ServerName, List<RegionInfo>> entry : clusterEntry.getValue().entrySet()){
-        if(entry.getKey().equals(masterServerName)) continue; // we shouldn't include master as potential assignee
-        serverLoadList.add(new ServerAndLoad(entry.getKey(), entry.getValue().size()));
-        sum += entry.getValue().size();
+    for (Map.Entry<TableName, Map<ServerName, List<RegionInfo>>> clusterEntry : clusterLoad.entrySet()) {
+      for (Map.Entry<ServerName, List<RegionInfo>> entry : clusterEntry.getValue().entrySet()) {
+        if (entry.getKey().equals(masterServerName)) continue; // we shouldn't include master as potential assignee
+        int regionNum = entry.getValue().size();
+        server2LoadMap.compute(entry.getKey(), (k, v) -> v == null ? regionNum : regionNum + v);
+        sum += regionNum;
       }
     }
+    server2LoadMap.forEach((k, v) -> {
+      serverLoadList.add(new ServerAndLoad(k, v));
+    });
     avgLoadOverall = sum / serverLoadList.size();
   }
 

diff --git a/...server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestDefaultLoadBalancer.java b/...server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestDefaultLoadBalancer.java
@@ -171,20 +171,33 @@ public void testBalanceClusterOverall() throws Exception {
    */
   @Test
   public void testImpactOfBalanceClusterOverall() throws Exception {
+    testImpactOfBalanceClusterOverall(false);
+  }
+
+  @Test
+  public void testImpactOfBalanceClusterOverallWithClusterLoadPerTable() throws Exception {
+    testImpactOfBalanceClusterOverall(true);
+  }
+
+  private void testImpactOfBalanceClusterOverall(boolean useClusterLoadPerTable) throws Exception {
     Map<TableName, Map<ServerName, List<RegionInfo>>> clusterLoad = new TreeMap<>();
     Map<ServerName, List<RegionInfo>> clusterServers = mockUniformClusterServers(mockUniformCluster);
     List<ServerAndLoad> clusterList = convertToList(clusterServers);
     clusterLoad.put(TableName.valueOf(name.getMethodName()), clusterServers);
     // use overall can achieve both table and cluster level balance
-    HashMap<TableName, TreeMap<ServerName, List<RegionInfo>>> result1 = mockClusterServersWithTables(clusterServers);
-    loadBalancer.setClusterLoad(clusterLoad);
+    HashMap<TableName, TreeMap<ServerName, List<RegionInfo>>> clusterLoadPerTable = mockClusterServersWithTables(clusterServers);
+    if (useClusterLoadPerTable) {
+      loadBalancer.setClusterLoad((Map)clusterLoadPerTable);
+    } else {
+      loadBalancer.setClusterLoad(clusterLoad);
+    }
     List<RegionPlan> clusterplans1 = new ArrayList<RegionPlan>();
     List<Pair<TableName, Integer>> regionAmountList = new ArrayList<Pair<TableName, Integer>>();
-    for(TreeMap<ServerName, List<RegionInfo>> servers : result1.values()){
+    for (TreeMap<ServerName, List<RegionInfo>> servers : clusterLoadPerTable.values()) {
       List<ServerAndLoad> list = convertToList(servers);
       LOG.info("Mock Cluster : " + printMock(list) + " " + printStats(list));
       List<RegionPlan> partialplans = loadBalancer.balanceCluster(servers);
-      if(partialplans != null) clusterplans1.addAll(partialplans);
+      if (partialplans != null) clusterplans1.addAll(partialplans);
       List<ServerAndLoad> balancedClusterPerTable = reconcile(list, partialplans, servers);
       LOG.info("Mock Balance : " + printMock(balancedClusterPerTable));
       assertClusterAsBalanced(balancedClusterPerTable);
@@ -194,6 +207,6 @@ public void testImpactOfBalanceClusterOverall() throws Exception {
       }
     }
     List<ServerAndLoad> balancedCluster1 = reconcile(clusterList, clusterplans1, clusterServers);
-    assertTrue(assertClusterOverallAsBalanced(balancedCluster1, result1.keySet().size()));
+    assertTrue(assertClusterOverallAsBalanced(balancedCluster1, clusterLoadPerTable.keySet().size()));
   }
 }