From 5f45c8293d45aa67d8dc26a34d3012b55e3b7c3e Mon Sep 17 00:00:00 2001 From: Beata Sudi Date: Wed, 22 Apr 2020 06:21:53 +0200 Subject: [PATCH] HBASE-24139 : Balancer should avoid leaving idle region servers (#1511) Signed-off-by: Viraj Jasani Signed-off-by: Sean Busbey Signed-off-by: Anoop Sam John --- .../master/balancer/BaseLoadBalancer.java | 18 ++++++++++++++++++ .../balancer/StochasticLoadBalancer.java | 4 ++++ .../balancer/TestStochasticLoadBalancer.java | 7 ++++++- 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java index d03e07a2fe9..7d05c41e0e0 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java @@ -1192,6 +1192,10 @@ public abstract class BaseLoadBalancer implements LoadBalancer { return false; } if(areSomeRegionReplicasColocated(c)) return true; + if(idleRegionServerExist(c)) { + return true; + } + // Check if we even need to do any load balancing // HBASE-3681 check sloppiness first float average = cs.getLoadAverage(); // for logging @@ -1223,6 +1227,20 @@ public abstract class BaseLoadBalancer implements LoadBalancer { return false; } + protected final boolean idleRegionServerExist(Cluster c){ + boolean isServerExistsWithMoreRegions = false; + boolean isServerExistsWithZeroRegions = false; + for (int[] serverList: c.regionsPerServer){ + if (serverList.length > 1) { + isServerExistsWithMoreRegions = true; + } + if (serverList.length == 0) { + isServerExistsWithZeroRegions = true; + } + } + return isServerExistsWithMoreRegions && isServerExistsWithZeroRegions; + } + /** * Generates a bulk assignment plan to be used on cluster startup using a * simple round-robin assignment. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java index 142f8cc200a..56b7ae46168 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java @@ -324,6 +324,10 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { return true; } + if (idleRegionServerExist(cluster)){ + return true; + } + double total = 0.0; float sumMultiplier = 0.0f; for (CostFunction c : costFunctions) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java index 91ef956691c..9b8a7b92ef6 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java @@ -255,7 +255,8 @@ public class TestStochasticLoadBalancer extends BalancerTestBase { Map>> LoadOfAllTable = (Map) mockClusterServersWithTables(servers); List plans = loadBalancer.balanceCluster(LoadOfAllTable); - assertTrue(plans == null || plans.isEmpty()); + boolean emptyPlans = plans == null || plans.isEmpty(); + assertTrue(emptyPlans || needsBalanceIdleRegion(mockCluster)); } } } finally { @@ -483,6 +484,10 @@ public class TestStochasticLoadBalancer extends BalancerTestBase { contains(DummyCostFunction.class.getSimpleName())); } + private boolean needsBalanceIdleRegion(int[] cluster){ + return (Arrays.stream(cluster).anyMatch(x -> x>1)) && (Arrays.stream(cluster).anyMatch(x -> x<1)); + } + // This mock allows us to test the LocalityCostFunction private class MockCluster extends BaseLoadBalancer.Cluster {