HBASE-17706 TableSkewCostFunction improperly computes max skew - revert due to test failure
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/a69c23ab Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/a69c23ab Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/a69c23ab Branch: refs/heads/hbase-12439 Commit: a69c23abfeed9246f308bf470b72a9a8afa46f5d Parents: 7f0e6f1 Author: tedyu <[email protected]> Authored: Thu Mar 16 19:07:59 2017 -0700 Committer: tedyu <[email protected]> Committed: Thu Mar 16 19:07:59 2017 -0700 ---------------------------------------------------------------------- .../hbase/master/balancer/BaseLoadBalancer.java | 21 ++++++++------- .../master/balancer/StochasticLoadBalancer.java | 19 ++++++------- .../balancer/TestStochasticLoadBalancer.java | 28 -------------------- 3 files changed, 20 insertions(+), 48 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/a69c23ab/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java ---------------------------------------------------------------------- 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 c6086f6..b0e088c 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 @@ -739,15 +739,18 @@ public abstract class BaseLoadBalancer implements LoadBalancer { } numRegionsPerServerPerTable[newServer][tableIndex]++; - // if old server had max num regions, assume (for now) - // max num regions went down since we moved the region - if (oldServer >= 0 && - (numRegionsPerServerPerTable[oldServer][tableIndex] + 1) == numMaxRegionsPerTable[tableIndex]) { - numMaxRegionsPerTable[tableIndex]--; - } - // Now check if new server sets new max - numMaxRegionsPerTable[tableIndex] = - Math.max(numMaxRegionsPerTable[tableIndex], numRegionsPerServerPerTable[newServer][tableIndex]); + //check whether this caused maxRegionsPerTable in the new Server to be updated + if (numRegionsPerServerPerTable[newServer][tableIndex] > numMaxRegionsPerTable[tableIndex]) { + numMaxRegionsPerTable[tableIndex] = numRegionsPerServerPerTable[newServer][tableIndex]; + } else if (oldServer >= 0 && (numRegionsPerServerPerTable[oldServer][tableIndex] + 1) + == numMaxRegionsPerTable[tableIndex]) { + //recompute maxRegionsPerTable since the previous value was coming from the old server + for (int serverIndex = 0 ; serverIndex < numRegionsPerServerPerTable.length; serverIndex++) { + if (numRegionsPerServerPerTable[serverIndex][tableIndex] > numMaxRegionsPerTable[tableIndex]) { + numMaxRegionsPerTable[tableIndex] = numRegionsPerServerPerTable[serverIndex][tableIndex]; + } + } + } // update for servers int primary = regionIndexToPrimaryIndex[region]; http://git-wip-us.apache.org/repos/asf/hbase/blob/a69c23ab/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java ---------------------------------------------------------------------- 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 2a3582e..8cbdd1e 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 @@ -272,6 +272,14 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { @Override protected boolean needsBalance(Cluster cluster) { + ClusterLoadState cs = new ClusterLoadState(cluster.clusterState); + if (cs.getNumServers() < MIN_SERVER_BALANCE) { + if (LOG.isDebugEnabled()) { + LOG.debug("Not running balancer because only " + cs.getNumServers() + + " active regionserver(s)"); + } + return false; + } if (areSomeRegionReplicasColocated(cluster)) { return true; } @@ -298,17 +306,6 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { + minCostNeedBalance); return false; } - - ClusterLoadState cs = new ClusterLoadState(cluster.clusterState); - if (cs.getNumServers() < MIN_SERVER_BALANCE) { - if (LOG.isDebugEnabled()) { - LOG.debug("Not running balancer because only " + cs.getNumServers() - + " active regionserver(s)"); - } - return false; - } - - return true; } http://git-wip-us.apache.org/repos/asf/hbase/blob/a69c23ab/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java ---------------------------------------------------------------------- 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 081db76..37ff35f 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 @@ -50,7 +50,6 @@ import org.apache.hadoop.hbase.master.RegionPlan; import org.apache.hadoop.hbase.master.balancer.BaseLoadBalancer.Cluster; import org.apache.hadoop.hbase.master.balancer.StochasticLoadBalancer.CandidateGenerator; import org.apache.hadoop.hbase.master.balancer.StochasticLoadBalancer.TableSkewCandidateGenerator; -import org.apache.hadoop.hbase.master.balancer.StochasticLoadBalancer.LoadCandidateGenerator; import org.apache.hadoop.hbase.testclassification.FlakeyTests; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.util.Bytes; @@ -239,33 +238,6 @@ public class TestStochasticLoadBalancer extends BalancerTestBase { } @Test - public void testTableSkewCostProperlyDecreases() { - int replication = 1; - Configuration conf = HBaseConfiguration.create(); - StochasticLoadBalancer.CostFunction - costFunction = new StochasticLoadBalancer.TableSkewCostFunction(conf); - CandidateGenerator generator = new LoadCandidateGenerator(); - // Start out with 100 regions on one server and 0 regions on the other - int numNodes = 2; - int numTables = 1; - int numRegions = 100; - int numRegionsPerServer = 0; - - Map<ServerName, List<HRegionInfo>> serverMap = createServerMap(numNodes, numRegions, numRegionsPerServer, replication, numTables); - BaseLoadBalancer.Cluster cluster = new Cluster(serverMap, null, null, null); - costFunction.init(cluster); - double cost = costFunction.cost(); - assertEquals(1.0, cost, .0001); - for (int i = 0; i < 100; i++) { - Cluster.Action action = generator.generate(cluster); - cluster.doAction(action); - costFunction.postAction(action); - cost = costFunction.cost(); - } - assertTrue(cost < 0.5); - } - - @Test public void testRegionLoadCost() { List<BalancerRegionLoad> regionLoads = new ArrayList<>(); for (int i = 1; i < 5; i++) {
