Skip to content

Commit 1a6a4e5

Browse files
rmdmattinglyRay Mattingly
rmdmattingly
and
Ray Mattingly
committed
HBASE-29070 Balancer cost function epsilon is imprecise (#6597)
Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>
1 parent b23d585 commit 1a6a4e5

16 files changed

Lines changed: 191 additions & 95 deletions

‎hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/StochasticBalancerTestBase.java‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.apache.hadoop.hbase.master.balancer;
19+
20+
import static org.junit.Assert.assertNull;
21+
import static org.junit.Assert.assertTrue;
22+
23+
import java.time.Duration;
24+
import java.util.List;
25+
import java.util.Map;
26+
import org.apache.hadoop.hbase.HBaseConfiguration;
27+
import org.apache.hadoop.hbase.ServerName;
28+
import org.apache.hadoop.hbase.TableName;
29+
import org.apache.hadoop.hbase.client.RegionInfo;
30+
import org.apache.hadoop.hbase.master.RackManager;
31+
import org.apache.hadoop.hbase.master.RegionPlan;
32+
import org.apache.hadoop.net.DNSToSwitchMapping;
33+
import org.junit.BeforeClass;
34+
import org.slf4j.Logger;
35+
import org.slf4j.LoggerFactory;
36+
37+
public class StochasticBalancerTestBase extends BalancerTestBase {
38+
39+
private static final Logger LOG = LoggerFactory.getLogger(StochasticBalancerTestBase.class);
40+
private static final Duration MAX_MAX_RUN_TIME = Duration.ofSeconds(60);
41+
42+
protected static StochasticLoadBalancer loadBalancer;
43+
44+
protected static DummyMetricsStochasticBalancer dummyMetricsStochasticBalancer =
45+
new DummyMetricsStochasticBalancer();
46+
47+
@BeforeClass
48+
public static void beforeAllTests() throws Exception {
49+
conf = HBaseConfiguration.create();
50+
conf.setClass("hbase.util.ip.to.rack.determiner", MockMapping.class, DNSToSwitchMapping.class);
51+
conf.setFloat("hbase.master.balancer.stochastic.localityCost", 0);
52+
conf.setBoolean("hbase.master.balancer.stochastic.runMaxSteps", true);
53+
conf.setLong(StochasticLoadBalancer.MAX_RUNNING_TIME_KEY, 250);
54+
loadBalancer = new StochasticLoadBalancer(dummyMetricsStochasticBalancer);
55+
loadBalancer.setClusterInfoProvider(new DummyClusterInfoProvider(conf));
56+
loadBalancer.initialize();
57+
}
58+
59+
protected void testWithClusterWithIteration(int numNodes, int numRegions, int numRegionsPerServer,
60+
int replication, int numTables, boolean assertFullyBalanced,
61+
boolean assertFullyBalancedForReplicas) {
62+
Map<ServerName, List<RegionInfo>> serverMap =
63+
createServerMap(numNodes, numRegions, numRegionsPerServer, replication, numTables);
64+
testWithClusterWithIteration(serverMap, null, assertFullyBalanced,
65+
assertFullyBalancedForReplicas);
66+
}
67+
68+
protected void increaseMaxRunTimeOrFail() {
69+
Duration current = getCurrentMaxRunTime();
70+
assertTrue(current.toMillis() < MAX_MAX_RUN_TIME.toMillis());
71+
Duration newMax = Duration.ofMillis(current.toMillis() * 2);
72+
if (newMax.toMillis() > MAX_MAX_RUN_TIME.toMillis()) {
73+
setMaxRunTime(MAX_MAX_RUN_TIME);
74+
} else {
75+
setMaxRunTime(newMax);
76+
}
77+
}
78+
79+
protected void testWithClusterWithIteration(Map<ServerName, List<RegionInfo>> serverMap,
80+
RackManager rackManager, boolean assertFullyBalanced, boolean assertFullyBalancedForReplicas) {
81+
List<ServerAndLoad> list = convertToList(serverMap);
82+
LOG.info("Mock Cluster : " + printMock(list) + " " + printStats(list));
83+
84+
loadBalancer.setRackManager(rackManager);
85+
// Run the balancer.
86+
Map<TableName, Map<ServerName, List<RegionInfo>>> LoadOfAllTable =
87+
(Map) mockClusterServersWithTables(serverMap);
88+
List<RegionPlan> plans = loadBalancer.balanceCluster(LoadOfAllTable);
89+
if (plans == null) {
90+
LOG.debug("First plans are null. Trying more balancer time, or will fail");
91+
increaseMaxRunTimeOrFail();
92+
testWithClusterWithIteration(serverMap, rackManager, assertFullyBalanced,
93+
assertFullyBalancedForReplicas);
94+
return;
95+
}
96+
97+
List<ServerAndLoad> balancedCluster = null;
98+
// Run through iteration until done. Otherwise will be killed as test time out
99+
while (plans != null && (assertFullyBalanced || assertFullyBalancedForReplicas)) {
100+
// Apply the plan to the mock cluster.
101+
balancedCluster = reconcile(list, plans, serverMap);
102+
103+
// Print out the cluster loads to make debugging easier.
104+
LOG.info("Mock after balance: " + printMock(balancedCluster));
105+
106+
LoadOfAllTable = (Map) mockClusterServersWithTables(serverMap);
107+
plans = loadBalancer.balanceCluster(LoadOfAllTable);
108+
}
109+
110+
// Print out the cluster loads to make debugging easier.
111+
LOG.info("Mock Final balance: " + printMock(balancedCluster));
112+
113+
if (assertFullyBalanced) {
114+
assertNull("Given a requirement to be fully balanced, second attempt at plans should "
115+
+ "produce none.", plans);
116+
}
117+
if (assertFullyBalancedForReplicas) {
118+
assertRegionReplicaPlacement(serverMap, rackManager);
119+
}
120+
}
121+
122+
private Duration getCurrentMaxRunTime() {
123+
return Duration.ofMillis(conf.getLong(StochasticLoadBalancer.MAX_RUNNING_TIME_KEY,
124+
StochasticLoadBalancer.DEFAULT_MAX_RUNNING_TIME));
125+
}
126+
}

‎hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/CostFunction.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@
2525
@InterfaceAudience.Private
2626
abstract class CostFunction {
2727

28-
public static final double COST_EPSILON = 0.0001;
28+
public static double getCostEpsilon(double cost) {
29+
return Math.ulp(cost);
30+
}
2931

3032
private float multiplier = 0;
3133

@@ -91,13 +93,14 @@ protected void regionMoved(int region, int oldServer, int newServer) {
9193
* @return The scaled value.
9294
*/
9395
protected static double scale(double min, double max, double value) {
96+
double costEpsilon = getCostEpsilon(max);
9497
if (
95-
max <= min || value <= min || Math.abs(max - min) <= COST_EPSILON
96-
|| Math.abs(value - min) <= COST_EPSILON
98+
max <= min || value <= min || Math.abs(max - min) <= costEpsilon
99+
|| Math.abs(value - min) <= costEpsilon
97100
) {
98101
return 0;
99102
}
100-
if (max <= min || Math.abs(max - min) <= COST_EPSILON) {
103+
if (max <= min || Math.abs(max - min) <= costEpsilon) {
101104
return 0;
102105
}
103106

‎hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -358,10 +358,8 @@ void updateMetricsSize(int size) {
358358

359359
private boolean areSomeRegionReplicasColocated(BalancerClusterState c) {
360360
regionReplicaHostCostFunction.prepare(c);
361-
if (Math.abs(regionReplicaHostCostFunction.cost()) > CostFunction.COST_EPSILON) {
362-
return true;
363-
}
364-
return (Math.abs(regionReplicaHostCostFunction.cost()) > CostFunction.COST_EPSILON);
361+
double cost = Math.abs(regionReplicaHostCostFunction.cost());
362+
return cost > CostFunction.getCostEpsilon(cost);
365363
}
366364

367365
@RestrictedApi(explanation = "Should only be called in tests", link = "",

‎hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/BalancerTestBase.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import static org.mockito.Mockito.mock;
2424
import static org.mockito.Mockito.when;
2525

26+
import java.time.Duration;
2627
import java.util.ArrayList;
2728
import java.util.HashMap;
2829
import java.util.HashSet;
@@ -31,10 +32,10 @@
3132
import java.util.List;
3233
import java.util.Map;
3334
import java.util.Map.Entry;
35+
import java.util.NavigableSet;
3436
import java.util.Queue;
3537
import java.util.Random;
3638
import java.util.Set;
37-
import java.util.SortedSet;
3839
import java.util.TreeMap;
3940
import java.util.TreeSet;
4041
import java.util.concurrent.ThreadLocalRandom;
@@ -287,6 +288,11 @@ public void assertRegionReplicaPlacement(Map<ServerName, List<RegionInfo>> serve
287288
}
288289
}
289290

291+
protected void setMaxRunTime(Duration maxRunTime) {
292+
conf.setLong(StochasticLoadBalancer.MAX_RUNNING_TIME_KEY, maxRunTime.toMillis());
293+
loadBalancer.loadConf(conf);
294+
}
295+
290296
protected String printStats(List<ServerAndLoad> servers) {
291297
int numServers = servers.size();
292298
int totalRegions = 0;
@@ -309,7 +315,10 @@ protected List<ServerAndLoad> convertToList(final Map<ServerName, List<RegionInf
309315
}
310316

311317
protected String printMock(List<ServerAndLoad> balancedCluster) {
312-
SortedSet<ServerAndLoad> sorted = new TreeSet<>(balancedCluster);
318+
if (balancedCluster == null) {
319+
return "null";
320+
}
321+
NavigableSet<ServerAndLoad> sorted = new TreeSet<>(balancedCluster);
313322
ServerAndLoad[] arr = sorted.toArray(new ServerAndLoad[sorted.size()]);
314323
StringBuilder sb = new StringBuilder(sorted.size() * 4 + 4);
315324
sb.append("{ ");

‎hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerBalanceCluster.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import static org.junit.Assert.assertNull;
2121

22+
import java.time.Duration;
2223
import java.util.List;
2324
import java.util.Map;
2425
import org.apache.hadoop.hbase.HBaseClassTestRule;
@@ -51,8 +52,8 @@ public class TestStochasticLoadBalancerBalanceCluster extends BalancerTestBase {
5152
*/
5253
@Test
5354
public void testBalanceCluster() throws Exception {
54-
conf.setLong("hbase.master.balancer.stochastic.maxRunningTime", 2 * 60 * 1000); // 2 min
5555
conf.setLong(StochasticLoadBalancer.MAX_STEPS_KEY, 20000000L);
56+
setMaxRunTime(Duration.ofMillis(1500));
5657
loadBalancer.onConfigurationChange(conf);
5758

5859
for (int[] mockCluster : clusterStateMocks) {

‎hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerHeterogeneousCost.java‎

Lines changed: 5 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -49,18 +49,13 @@
4949
import org.junit.ClassRule;
5050
import org.junit.Test;
5151
import org.junit.experimental.categories.Category;
52-
import org.slf4j.Logger;
53-
import org.slf4j.LoggerFactory;
5452

5553
@Category({ MasterTests.class, MediumTests.class })
5654
public class TestStochasticLoadBalancerHeterogeneousCost extends BalancerTestBase {
5755
@ClassRule
5856
public static final HBaseClassTestRule CLASS_RULE =
5957
HBaseClassTestRule.forClass(TestStochasticLoadBalancerHeterogeneousCost.class);
6058

61-
private static final Logger LOG =
62-
LoggerFactory.getLogger(TestStochasticLoadBalancerHeterogeneousCost.class);
63-
private static final double ALLOWED_WINDOW = 1.20;
6459
private static final HBaseTestingUtility HTU = new HBaseTestingUtility();
6560
private static String RULES_FILE;
6661

@@ -172,65 +167,7 @@ private void testHeterogeneousWithCluster(final int numNodes, final int numRegio
172167
TestStochasticLoadBalancerHeterogeneousCostRules.createRulesFile(RULES_FILE, rules);
173168
final Map<ServerName, List<RegionInfo>> serverMap =
174169
this.createServerMap(numNodes, numRegions, numRegionsPerServer, 1, 1);
175-
this.testWithCluster(serverMap, null, true, false);
176-
}
177-
178-
protected void testWithCluster(final Map<ServerName, List<RegionInfo>> serverMap,
179-
final RackManager rackManager, final boolean assertFullyBalanced,
180-
final boolean assertFullyBalancedForReplicas) {
181-
final List<ServerAndLoad> list = this.convertToList(serverMap);
182-
LOG.info("Mock Cluster : " + this.printMock(list) + " " + this.printStats(list));
183-
184-
BalancerTestBase.loadBalancer.setRackManager(rackManager);
185-
186-
// Run the balancer.
187-
final List<RegionPlan> plans =
188-
BalancerTestBase.loadBalancer.balanceTable(HConstants.ENSEMBLE_TABLE_NAME, serverMap);
189-
assertNotNull(plans);
190-
191-
// Check to see that this actually got to a stable place.
192-
if (assertFullyBalanced || assertFullyBalancedForReplicas) {
193-
// Apply the plan to the mock cluster.
194-
final List<ServerAndLoad> balancedCluster = this.reconcile(list, plans, serverMap);
195-
196-
// Print out the cluster loads to make debugging easier.
197-
LOG.info("Mock Balanced cluster : " + this.printMock(balancedCluster));
198-
199-
if (assertFullyBalanced) {
200-
final List<RegionPlan> secondPlans =
201-
BalancerTestBase.loadBalancer.balanceTable(HConstants.ENSEMBLE_TABLE_NAME, serverMap);
202-
assertNull(secondPlans);
203-
204-
// create external cost function to retrieve limit
205-
// for each RS
206-
final HeterogeneousRegionCountCostFunction cf =
207-
new HeterogeneousRegionCountCostFunction(conf);
208-
assertNotNull(cf);
209-
BalancerClusterState cluster = new BalancerClusterState(serverMap, null, null, null);
210-
cf.prepare(cluster);
211-
212-
// checking that we all hosts have a number of regions below their limit
213-
for (final ServerAndLoad serverAndLoad : balancedCluster) {
214-
final ServerName sn = serverAndLoad.getServerName();
215-
final int numberRegions = serverAndLoad.getLoad();
216-
final int limit = cf.findLimitForRS(sn);
217-
218-
double usage = (double) numberRegions / (double) limit;
219-
LOG.debug(
220-
sn.getHostname() + ":" + numberRegions + "/" + limit + "(" + (usage * 100) + "%)");
221-
222-
// as the balancer is stochastic, we cannot check exactly the result of the balancing,
223-
// hence the allowedWindow parameter
224-
assertTrue("Host " + sn.getHostname() + " should be below "
225-
+ cf.overallUsage * ALLOWED_WINDOW * 100 + "%; " + cf.overallUsage + ", " + usage + ", "
226-
+ numberRegions + ", " + limit, usage <= cf.overallUsage * ALLOWED_WINDOW);
227-
}
228-
}
229-
230-
if (assertFullyBalancedForReplicas) {
231-
this.assertRegionReplicaPlacement(serverMap, rackManager);
232-
}
233-
}
170+
this.testWithClusterWithIteration(serverMap, null, true, false);
234171
}
235172

236173
@Override
@@ -313,6 +250,10 @@ static class StochasticLoadTestBalancer extends StochasticLoadBalancer {
313250
private FairRandomCandidateGenerator fairRandomCandidateGenerator =
314251
new FairRandomCandidateGenerator();
315252

253+
StochasticLoadTestBalancer() {
254+
super(new DummyMetricsStochasticBalancer());
255+
}
256+
316257
@Override
317258
protected CandidateGenerator getRandomGenerator() {
318259
return fairRandomCandidateGenerator;

‎hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerLargeCluster.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
*/
1818
package org.apache.hadoop.hbase.master.balancer;
1919

20+
import java.time.Duration;
2021
import org.apache.hadoop.hbase.HBaseClassTestRule;
2122
import org.apache.hadoop.hbase.testclassification.MasterTests;
2223
import org.apache.hadoop.hbase.testclassification.MediumTests;
@@ -38,8 +39,8 @@ public void testLargeCluster() {
3839
int numRegionsPerServer = 80; // all servers except one
3940
int numTables = 100;
4041
int replication = 1;
41-
conf.setLong("hbase.master.balancer.stochastic.maxRunningTime", 6 * 60 * 1000);
4242
conf.setLong(StochasticLoadBalancer.MAX_STEPS_KEY, 20000000L);
43+
setMaxRunTime(Duration.ofSeconds(30));
4344
loadBalancer.onConfigurationChange(conf);
4445
testWithClusterWithIteration(numNodes, numRegions, numRegionsPerServer, replication, numTables,
4546
true, true);

‎hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerMidCluster.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
*/
1818
package org.apache.hadoop.hbase.master.balancer;
1919

20+
import java.time.Duration;
2021
import org.apache.hadoop.hbase.HBaseClassTestRule;
2122
import org.apache.hadoop.hbase.testclassification.LargeTests;
2223
import org.apache.hadoop.hbase.testclassification.MasterTests;
@@ -38,7 +39,8 @@ public void testMidCluster() {
3839
int numRegionsPerServer = 60; // all servers except one
3940
int replication = 1;
4041
int numTables = 40;
41-
testWithCluster(numNodes, numRegions, numRegionsPerServer, replication, numTables, true, true);
42+
testWithClusterWithIteration(numNodes, numRegions, numRegionsPerServer, replication, numTables,
43+
true, true);
4244
}
4345

4446
@Test
@@ -50,7 +52,8 @@ public void testMidCluster2() {
5052
int numTables = 400;
5153
// num large num regions means may not always get to best balance with one run
5254
boolean assertFullyBalanced = false;
53-
testWithCluster(numNodes, numRegions, numRegionsPerServer, replication, numTables,
55+
setMaxRunTime(Duration.ofMillis(2500));
56+
testWithClusterWithIteration(numNodes, numRegions, numRegionsPerServer, replication, numTables,
5457
assertFullyBalanced, false);
5558
}
5659

@@ -61,7 +64,8 @@ public void testMidCluster3() {
6164
int numRegionsPerServer = 9; // all servers except one
6265
int replication = 1;
6366
int numTables = 110;
64-
testWithCluster(numNodes, numRegions, numRegionsPerServer, replication, numTables, true, true);
67+
testWithClusterWithIteration(numNodes, numRegions, numRegionsPerServer, replication, numTables,
68+
true, true);
6569
// TODO(eclark): Make sure that the tables are well distributed.
6670
}
6771
}

‎hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerRegionReplica.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,8 @@ public void testRegionReplicasOnSmallCluster() {
171171
int replication = 3; // 3 replicas per region
172172
int numRegionsPerServer = 80; // all regions are mostly balanced
173173
int numTables = 10;
174-
testWithCluster(numNodes, numRegions, numRegionsPerServer, replication, numTables, true, true);
174+
testWithClusterWithIteration(numNodes, numRegions, numRegionsPerServer, replication, numTables,
175+
true, true);
175176
}
176177

177178
private static class ForTestRackManagerOne extends RackManager {

0 commit comments

Comments
 (0)