From e2a414f82d3e0c88703c22f9b0000fb05c755da0 Mon Sep 17 00:00:00 2001 From: Clara Xiong Date: Thu, 7 Oct 2021 11:36:26 -0700 Subject: [PATCH 1/4] HBASE-26327 Replicas cohosted on a rack shouldn't keep triggering Balancer --- .../hbase/master/balancer/StochasticLoadBalancer.java | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java index eaa923e74fcf..3b0be8bd33b3 100644 --- a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java +++ b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java @@ -310,14 +310,7 @@ void updateMetricsSize(int size) { private boolean areSomeRegionReplicasColocated(BalancerClusterState c) { regionReplicaHostCostFunction.prepare(c); - if (Math.abs(regionReplicaHostCostFunction.cost()) > CostFunction.COST_EPSILON) { - return true; - } - regionReplicaRackCostFunction.prepare(c); - if (Math.abs(regionReplicaRackCostFunction.cost()) > CostFunction.COST_EPSILON) { - return true; - } - return false; + return (Math.abs(regionReplicaHostCostFunction.cost()) > CostFunction.COST_EPSILON); } private String getBalanceReason(double total, double sumMultiplier) { From c2a15caffecfabc2b587df2b7945463967e93937 Mon Sep 17 00:00:00 2001 From: Clara Xiong Date: Thu, 7 Oct 2021 12:54:11 -0700 Subject: [PATCH 2/4] update test case for the intended behavior --- .../balancer/TestStochasticLoadBalancerRegionReplica.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplica.java b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplica.java index ac6ad4b75ace..60e6374fbb8b 100644 --- a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplica.java +++ b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplica.java @@ -161,7 +161,7 @@ public void testNeedsBalanceForColocatedReplicas() { map.put(s2, regionsOnS2); // add another server so that the cluster has some host on another rack map.put(ServerName.valueOf("host2", 1000, 11111), randomRegions(1)); - assertTrue( + assertFalse( loadBalancer.needsBalance(HConstants.ENSEMBLE_TABLE_NAME, new BalancerClusterState(map, null, null, new ForTestRackManagerOne()))); } From 871c9db42383bc7384ab026fe730b3f6d9f50594 Mon Sep 17 00:00:00 2001 From: Clara Xiong Date: Thu, 7 Oct 2021 14:10:33 -0700 Subject: [PATCH 3/4] fix for assert --- .../balancer/TestStochasticLoadBalancerRegionReplica.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplica.java b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplica.java index 60e6374fbb8b..723883b09371 100644 --- a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplica.java +++ b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplica.java @@ -161,8 +161,7 @@ public void testNeedsBalanceForColocatedReplicas() { map.put(s2, regionsOnS2); // add another server so that the cluster has some host on another rack map.put(ServerName.valueOf("host2", 1000, 11111), randomRegions(1)); - assertFalse( - loadBalancer.needsBalance(HConstants.ENSEMBLE_TABLE_NAME, + assertTrue(!loadBalancer.needsBalance(HConstants.ENSEMBLE_TABLE_NAME, new BalancerClusterState(map, null, null, new ForTestRackManagerOne()))); } From 11af88815e7c1c873655e5c9588f9dc2596376b9 Mon Sep 17 00:00:00 2001 From: Clara Xiong Date: Fri, 22 Oct 2021 15:03:46 -0700 Subject: [PATCH 4/4] code review feedback --- .../balancer/TestStochasticLoadBalancerRegionReplica.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplica.java b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplica.java index 723883b09371..58eed9e63796 100644 --- a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplica.java +++ b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplica.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.master.balancer; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import java.util.ArrayList; @@ -161,7 +162,7 @@ public void testNeedsBalanceForColocatedReplicas() { map.put(s2, regionsOnS2); // add another server so that the cluster has some host on another rack map.put(ServerName.valueOf("host2", 1000, 11111), randomRegions(1)); - assertTrue(!loadBalancer.needsBalance(HConstants.ENSEMBLE_TABLE_NAME, + assertFalse(loadBalancer.needsBalance(HConstants.ENSEMBLE_TABLE_NAME, new BalancerClusterState(map, null, null, new ForTestRackManagerOne()))); }