From c6e1caf4fc9898a4ef7e47b99602cd0c667c4fe6 Mon Sep 17 00:00:00 2001 From: David Manning Date: Thu, 28 Apr 2022 23:06:19 -0700 Subject: [PATCH 1/4] HBASE-26989 TestStochasticLoadBalancer fixes for performance and consistency --- .../balancer/TestStochasticLoadBalancer.java | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java index f6a4c573417c..c4467a1dd5d0 100644 --- a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java +++ b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java @@ -235,6 +235,7 @@ public void testUpdateBalancerLoadInfo() { BalancerClusterState clusterState = mockCluster(cluster); Map>> LoadOfAllTable = (Map) mockClusterServersWithTables(servers); + boolean oldIsByTable = conf.getBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, false); try { boolean[] perTableBalancerConfigs = { true, false }; for (boolean isByTable : perTableBalancerConfigs) { @@ -259,7 +260,7 @@ public void testUpdateBalancerLoadInfo() { assertEquals(curOverallCost, curOverallCostInMetrics, 0.001); } } finally { - conf.unset(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE); + conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, oldIsByTable); loadBalancer.onConfigurationChange(conf); } } @@ -267,6 +268,7 @@ public void testUpdateBalancerLoadInfo() { @Test public void testUpdateStochasticCosts() { float minCost = conf.getFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 0.05f); + boolean oldIsByTable = conf.getBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, false); try { int[] cluster = new int[] { 10, 0 }; Map> servers = mockClusterServers(cluster); @@ -289,7 +291,7 @@ public void testUpdateStochasticCosts() { } finally { // reset config conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", minCost); - conf.unset(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE); + conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, oldIsByTable); loadBalancer.onConfigurationChange(conf); } } @@ -297,6 +299,7 @@ public void testUpdateStochasticCosts() { @Test public void testUpdateStochasticCostsIfBalanceNotRan() { float minCost = conf.getFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 0.05f); + boolean oldIsByTable = conf.getBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, false); try { int[] cluster = new int[] { 10, 10 }; Map> servers = mockClusterServers(cluster); @@ -319,17 +322,22 @@ public void testUpdateStochasticCostsIfBalanceNotRan() { } finally { // reset config conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", minCost); - conf.unset(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE); + conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, oldIsByTable); loadBalancer.onConfigurationChange(conf); } } @Test public void testNeedBalance() { + boolean oldIsByTable = conf.getBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, false); float minCost = conf.getFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 0.05f); float slop = conf.getFloat(HConstants.LOAD_BALANCER_SLOP_KEY, 0.2f); + boolean runMaxSteps = conf.getBoolean("hbase.master.balancer.stochastic.runMaxSteps", false); + long maxSteps = conf.getLong("hbase.master.balancer.stochastic.maxSteps", 1000000); conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 1.0f); conf.setFloat(HConstants.LOAD_BALANCER_SLOP_KEY, -1f); + conf.setBoolean("hbase.master.balancer.stochastic.runMaxSteps", false); + conf.setLong("hbase.master.balancer.stochastic.maxSteps", 5000L); try { // Test with/without per table balancer. boolean[] perTableBalancerConfigs = { true, false }; @@ -342,9 +350,11 @@ public void testNeedBalance() { } } finally { // reset config - conf.unset(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE); + conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, oldIsByTable); conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", minCost); conf.setFloat(HConstants.LOAD_BALANCER_SLOP_KEY, slop); + conf.setBoolean("hbase.master.balancer.stochastic.runMaxSteps", runMaxSteps); + conf.setLong("hbase.master.balancer.stochastic.maxSteps", maxSteps); loadBalancer.onConfigurationChange(conf); } } @@ -352,6 +362,8 @@ public void testNeedBalance() { @Test public void testBalanceOfSloppyServers() { float minCost = conf.getFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 0.025f); + boolean runMaxSteps = conf.getBoolean("hbase.master.balancer.stochastic.runMaxSteps", false); + long maxSteps = conf.getLong("hbase.master.balancer.stochastic.maxSteps", 1000000); try { // We are testing slop checks, so don't "accidentally" balance due to a minCost calculation. // During development, imbalance of a 100 server cluster, with one node having 10 regions @@ -359,6 +371,8 @@ public void testBalanceOfSloppyServers() { // validate slop checks without this override. We override just to ensure we will always // validate slop check here, and for small clusters as well. conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 1.0f); + conf.setBoolean("hbase.master.balancer.stochastic.runMaxSteps", false); + conf.setLong("hbase.master.balancer.stochastic.maxSteps", 5000L); loadBalancer.onConfigurationChange(conf); for (int[] mockCluster : clusterStateMocksWithNoSlop) { assertTrue(hasEmptyBalancerPlans(mockCluster)); @@ -369,6 +383,8 @@ public void testBalanceOfSloppyServers() { } finally { // reset config conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", minCost); + conf.setBoolean("hbase.master.balancer.stochastic.runMaxSteps", runMaxSteps); + conf.setLong("hbase.master.balancer.stochastic.maxSteps", maxSteps); loadBalancer.onConfigurationChange(conf); } } @@ -381,14 +397,21 @@ public void testSloppyTablesLoadBalanceByTable() { new int[] { 2, 8, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5 }, }; float minCost = conf.getFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 0.025f); + boolean runMaxSteps = conf.getBoolean("hbase.master.balancer.stochastic.runMaxSteps", false); + long maxSteps = conf.getLong("hbase.master.balancer.stochastic.maxSteps", 1000000); + boolean oldIsByTable = conf.getBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, false); try { conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 1.0f); + conf.setBoolean("hbase.master.balancer.stochastic.runMaxSteps", false); + conf.setLong("hbase.master.balancer.stochastic.maxSteps", 5000L); conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, true); loadBalancer.onConfigurationChange(conf); assertFalse(hasEmptyBalancerPlans(regionsPerServerPerTable)); } finally { conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", minCost); - conf.unset(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE); + conf.setBoolean("hbase.master.balancer.stochastic.runMaxSteps", runMaxSteps); + conf.setLong("hbase.master.balancer.stochastic.maxSteps", maxSteps); + conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, oldIsByTable); loadBalancer.onConfigurationChange(conf); } } From b02fa4c86da6e1ce0eff8fb5e683fe1890cb9d8a Mon Sep 17 00:00:00 2001 From: David Manning Date: Fri, 29 Apr 2022 16:24:20 -0700 Subject: [PATCH 2/4] restore configuration before each test --- .../balancer/TestStochasticLoadBalancer.java | 263 +++++++----------- 1 file changed, 104 insertions(+), 159 deletions(-) diff --git a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java index c4467a1dd5d0..66116d5f2181 100644 --- a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java +++ b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java @@ -49,6 +49,10 @@ import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.util.Bytes; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Before; +import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -66,6 +70,18 @@ public class TestStochasticLoadBalancer extends StochasticBalancerTestBase { // Mapping of locality test -> expected locality private float[] expectedLocalities = { 1.0f, 0.0f, 0.50f, 0.25f, 1.0f }; + private static Configuration storedConfiguration; + + @BeforeClass + public static void saveInitialConfiguration() { + storedConfiguration = new Configuration(conf); + } + + @Before + public void beforeEachTest() { + conf = new Configuration(storedConfiguration); + loadBalancer.onConfigurationChange(conf); + } /** * Data set for testLocalityCost: [test][0][0] = mapping of server to number of regions it hosts @@ -184,9 +200,6 @@ public void testCPRequestCost() { assertEquals(2, regionsMoveFromServerA.size()); assertEquals(2, targetServers.size()); assertTrue(regionsOnServerA.containsAll(regionsMoveFromServerA)); - // reset config - conf.setFloat("hbase.master.balancer.stochastic.cpRequestCost", 5f); - loadBalancer.onConfigurationChange(conf); } @Test @@ -235,157 +248,107 @@ public void testUpdateBalancerLoadInfo() { BalancerClusterState clusterState = mockCluster(cluster); Map>> LoadOfAllTable = (Map) mockClusterServersWithTables(servers); - boolean oldIsByTable = conf.getBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, false); - try { - boolean[] perTableBalancerConfigs = { true, false }; - for (boolean isByTable : perTableBalancerConfigs) { - conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, isByTable); - loadBalancer.onConfigurationChange(conf); - dummyMetricsStochasticBalancer.clearDummyMetrics(); - loadBalancer.updateBalancerLoadInfo(LoadOfAllTable); - assertTrue("Metrics should be recorded!", - dummyMetricsStochasticBalancer.getDummyCostsMap() != null - && !dummyMetricsStochasticBalancer.getDummyCostsMap().isEmpty()); - - String metricRecordKey; - if (isByTable) { - metricRecordKey = "table1#" + StochasticLoadBalancer.OVERALL_COST_FUNCTION_NAME; - } else { - metricRecordKey = HConstants.ENSEMBLE_TABLE_NAME + "#" - + StochasticLoadBalancer.OVERALL_COST_FUNCTION_NAME; - } - double curOverallCost = loadBalancer.computeCost(clusterState, Double.MAX_VALUE); - double curOverallCostInMetrics = - dummyMetricsStochasticBalancer.getDummyCostsMap().get(metricRecordKey); - assertEquals(curOverallCost, curOverallCostInMetrics, 0.001); - } - } finally { - conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, oldIsByTable); + boolean[] perTableBalancerConfigs = { true, false }; + for (boolean isByTable : perTableBalancerConfigs) { + conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, isByTable); loadBalancer.onConfigurationChange(conf); + dummyMetricsStochasticBalancer.clearDummyMetrics(); + loadBalancer.updateBalancerLoadInfo(LoadOfAllTable); + assertTrue("Metrics should be recorded!", + dummyMetricsStochasticBalancer.getDummyCostsMap() != null + && !dummyMetricsStochasticBalancer.getDummyCostsMap().isEmpty()); + + String metricRecordKey; + if (isByTable) { + metricRecordKey = "table1#" + StochasticLoadBalancer.OVERALL_COST_FUNCTION_NAME; + } else { + metricRecordKey = HConstants.ENSEMBLE_TABLE_NAME + "#" + + StochasticLoadBalancer.OVERALL_COST_FUNCTION_NAME; + } + double curOverallCost = loadBalancer.computeCost(clusterState, Double.MAX_VALUE); + double curOverallCostInMetrics = + dummyMetricsStochasticBalancer.getDummyCostsMap().get(metricRecordKey); + assertEquals(curOverallCost, curOverallCostInMetrics, 0.001); } } @Test public void testUpdateStochasticCosts() { - float minCost = conf.getFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 0.05f); - boolean oldIsByTable = conf.getBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, false); - try { - int[] cluster = new int[] { 10, 0 }; - Map> servers = mockClusterServers(cluster); - BalancerClusterState clusterState = mockCluster(cluster); - conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 1.0f); - conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, false); - loadBalancer.onConfigurationChange(conf); - dummyMetricsStochasticBalancer.clearDummyMetrics(); - List plans = - loadBalancer.balanceCluster((Map) mockClusterServersWithTables(servers)); - - assertTrue("Balance plan should not be empty!", plans != null && !plans.isEmpty()); - assertTrue("There should be metrics record in MetricsStochasticBalancer", - !dummyMetricsStochasticBalancer.getDummyCostsMap().isEmpty()); - - double overallCostOfCluster = loadBalancer.computeCost(clusterState, Double.MAX_VALUE); - double overallCostInMetrics = dummyMetricsStochasticBalancer.getDummyCostsMap().get( - HConstants.ENSEMBLE_TABLE_NAME + "#" + StochasticLoadBalancer.OVERALL_COST_FUNCTION_NAME); - assertEquals(overallCostOfCluster, overallCostInMetrics, 0.001); - } finally { - // reset config - conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", minCost); - conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, oldIsByTable); - loadBalancer.onConfigurationChange(conf); - } + int[] cluster = new int[] { 10, 0 }; + Map> servers = mockClusterServers(cluster); + BalancerClusterState clusterState = mockCluster(cluster); + conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 1.0f); + conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, false); + loadBalancer.onConfigurationChange(conf); + dummyMetricsStochasticBalancer.clearDummyMetrics(); + List plans = + loadBalancer.balanceCluster((Map) mockClusterServersWithTables(servers)); + + assertTrue("Balance plan should not be empty!", plans != null && !plans.isEmpty()); + assertTrue("There should be metrics record in MetricsStochasticBalancer", + !dummyMetricsStochasticBalancer.getDummyCostsMap().isEmpty()); + + double overallCostOfCluster = loadBalancer.computeCost(clusterState, Double.MAX_VALUE); + double overallCostInMetrics = dummyMetricsStochasticBalancer.getDummyCostsMap().get( + HConstants.ENSEMBLE_TABLE_NAME + "#" + StochasticLoadBalancer.OVERALL_COST_FUNCTION_NAME); + assertEquals(overallCostOfCluster, overallCostInMetrics, 0.001); } @Test public void testUpdateStochasticCostsIfBalanceNotRan() { - float minCost = conf.getFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 0.05f); - boolean oldIsByTable = conf.getBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, false); - try { - int[] cluster = new int[] { 10, 10 }; - Map> servers = mockClusterServers(cluster); - BalancerClusterState clusterState = mockCluster(cluster); - conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", Float.MAX_VALUE); - conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, false); - loadBalancer.onConfigurationChange(conf); - dummyMetricsStochasticBalancer.clearDummyMetrics(); - List plans = - loadBalancer.balanceCluster((Map) mockClusterServersWithTables(servers)); - - assertTrue("Balance plan should be empty!", plans == null || plans.isEmpty()); - assertTrue("There should be metrics record in MetricsStochasticBalancer!", - !dummyMetricsStochasticBalancer.getDummyCostsMap().isEmpty()); - - double overallCostOfCluster = loadBalancer.computeCost(clusterState, Double.MAX_VALUE); - double overallCostInMetrics = dummyMetricsStochasticBalancer.getDummyCostsMap().get( - HConstants.ENSEMBLE_TABLE_NAME + "#" + StochasticLoadBalancer.OVERALL_COST_FUNCTION_NAME); - assertEquals(overallCostOfCluster, overallCostInMetrics, 0.001); - } finally { - // reset config - conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", minCost); - conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, oldIsByTable); - loadBalancer.onConfigurationChange(conf); - } + int[] cluster = new int[] { 10, 10 }; + Map> servers = mockClusterServers(cluster); + BalancerClusterState clusterState = mockCluster(cluster); + conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", Float.MAX_VALUE); + conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, false); + loadBalancer.onConfigurationChange(conf); + dummyMetricsStochasticBalancer.clearDummyMetrics(); + List plans = + loadBalancer.balanceCluster((Map) mockClusterServersWithTables(servers)); + + assertTrue("Balance plan should be empty!", plans == null || plans.isEmpty()); + assertTrue("There should be metrics record in MetricsStochasticBalancer!", + !dummyMetricsStochasticBalancer.getDummyCostsMap().isEmpty()); + + double overallCostOfCluster = loadBalancer.computeCost(clusterState, Double.MAX_VALUE); + double overallCostInMetrics = dummyMetricsStochasticBalancer.getDummyCostsMap().get( + HConstants.ENSEMBLE_TABLE_NAME + "#" + StochasticLoadBalancer.OVERALL_COST_FUNCTION_NAME); + assertEquals(overallCostOfCluster, overallCostInMetrics, 0.001); } @Test public void testNeedBalance() { - boolean oldIsByTable = conf.getBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, false); - float minCost = conf.getFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 0.05f); - float slop = conf.getFloat(HConstants.LOAD_BALANCER_SLOP_KEY, 0.2f); - boolean runMaxSteps = conf.getBoolean("hbase.master.balancer.stochastic.runMaxSteps", false); - long maxSteps = conf.getLong("hbase.master.balancer.stochastic.maxSteps", 1000000); conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 1.0f); conf.setFloat(HConstants.LOAD_BALANCER_SLOP_KEY, -1f); conf.setBoolean("hbase.master.balancer.stochastic.runMaxSteps", false); conf.setLong("hbase.master.balancer.stochastic.maxSteps", 5000L); - try { - // Test with/without per table balancer. - boolean[] perTableBalancerConfigs = { true, false }; - for (boolean isByTable : perTableBalancerConfigs) { - conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, isByTable); - loadBalancer.onConfigurationChange(conf); - for (int[] mockCluster : clusterStateMocks) { - assertTrue(hasEmptyBalancerPlans(mockCluster) || needsBalanceIdleRegion(mockCluster)); - } - } - } finally { - // reset config - conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, oldIsByTable); - conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", minCost); - conf.setFloat(HConstants.LOAD_BALANCER_SLOP_KEY, slop); - conf.setBoolean("hbase.master.balancer.stochastic.runMaxSteps", runMaxSteps); - conf.setLong("hbase.master.balancer.stochastic.maxSteps", maxSteps); + // Test with/without per table balancer. + boolean[] perTableBalancerConfigs = { true, false }; + for (boolean isByTable : perTableBalancerConfigs) { + conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, isByTable); loadBalancer.onConfigurationChange(conf); + for (int[] mockCluster : clusterStateMocks) { + assertTrue(hasEmptyBalancerPlans(mockCluster) || needsBalanceIdleRegion(mockCluster)); + } } } @Test public void testBalanceOfSloppyServers() { - float minCost = conf.getFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 0.025f); - boolean runMaxSteps = conf.getBoolean("hbase.master.balancer.stochastic.runMaxSteps", false); - long maxSteps = conf.getLong("hbase.master.balancer.stochastic.maxSteps", 1000000); - try { - // We are testing slop checks, so don't "accidentally" balance due to a minCost calculation. - // During development, imbalance of a 100 server cluster, with one node having 10 regions - // and the rest having 5, is 0.0048. With minCostNeedBalance default of 0.025, test should - // validate slop checks without this override. We override just to ensure we will always - // validate slop check here, and for small clusters as well. - conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 1.0f); - conf.setBoolean("hbase.master.balancer.stochastic.runMaxSteps", false); - conf.setLong("hbase.master.balancer.stochastic.maxSteps", 5000L); - loadBalancer.onConfigurationChange(conf); - for (int[] mockCluster : clusterStateMocksWithNoSlop) { - assertTrue(hasEmptyBalancerPlans(mockCluster)); - } - for (int[] mockCluster : clusterStateMocksWithSlop) { - assertFalse(hasEmptyBalancerPlans(mockCluster)); - } - } finally { - // reset config - conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", minCost); - conf.setBoolean("hbase.master.balancer.stochastic.runMaxSteps", runMaxSteps); - conf.setLong("hbase.master.balancer.stochastic.maxSteps", maxSteps); - loadBalancer.onConfigurationChange(conf); + // We are testing slop checks, so don't "accidentally" balance due to a minCost calculation. + // During development, imbalance of a 100 server cluster, with one node having 10 regions + // and the rest having 5, is 0.0048. With minCostNeedBalance default of 0.025, test should + // validate slop checks without this override. We override just to ensure we will always + // validate slop check here, and for small clusters as well. + conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 1.0f); + conf.setBoolean("hbase.master.balancer.stochastic.runMaxSteps", false); + conf.setLong("hbase.master.balancer.stochastic.maxSteps", 5000L); + loadBalancer.onConfigurationChange(conf); + for (int[] mockCluster : clusterStateMocksWithNoSlop) { + assertTrue(hasEmptyBalancerPlans(mockCluster)); + } + for (int[] mockCluster : clusterStateMocksWithSlop) { + assertFalse(hasEmptyBalancerPlans(mockCluster)); } } @@ -396,24 +359,12 @@ public void testSloppyTablesLoadBalanceByTable() { 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5 }, new int[] { 2, 8, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5 }, }; - float minCost = conf.getFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 0.025f); - boolean runMaxSteps = conf.getBoolean("hbase.master.balancer.stochastic.runMaxSteps", false); - long maxSteps = conf.getLong("hbase.master.balancer.stochastic.maxSteps", 1000000); - boolean oldIsByTable = conf.getBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, false); - try { - conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 1.0f); - conf.setBoolean("hbase.master.balancer.stochastic.runMaxSteps", false); - conf.setLong("hbase.master.balancer.stochastic.maxSteps", 5000L); - conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, true); - loadBalancer.onConfigurationChange(conf); - assertFalse(hasEmptyBalancerPlans(regionsPerServerPerTable)); - } finally { - conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", minCost); - conf.setBoolean("hbase.master.balancer.stochastic.runMaxSteps", runMaxSteps); - conf.setLong("hbase.master.balancer.stochastic.maxSteps", maxSteps); - conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, oldIsByTable); - loadBalancer.onConfigurationChange(conf); - } + conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 1.0f); + conf.setBoolean("hbase.master.balancer.stochastic.runMaxSteps", false); + conf.setLong("hbase.master.balancer.stochastic.maxSteps", 5000L); + conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, true); + loadBalancer.onConfigurationChange(conf); + assertFalse(hasEmptyBalancerPlans(regionsPerServerPerTable)); } private boolean hasEmptyBalancerPlans(int[] mockCluster) { @@ -530,7 +481,6 @@ public void testSkewCost() { @Test public void testCostAfterUndoAction() { final int runs = 10; - loadBalancer.onConfigurationChange(conf); for (int[] mockCluster : clusterStateMocks) { BalancerClusterState cluster = mockCluster(mockCluster); loadBalancer.initCosts(cluster); @@ -645,17 +595,12 @@ public void testLosingRs() throws Exception { @Test public void testAdditionalCostFunction() { - try { - conf.set(StochasticLoadBalancer.COST_FUNCTIONS_COST_FUNCTIONS_KEY, - DummyCostFunction.class.getName()); + conf.set(StochasticLoadBalancer.COST_FUNCTIONS_COST_FUNCTIONS_KEY, + DummyCostFunction.class.getName()); - loadBalancer.onConfigurationChange(conf); - assertTrue(Arrays.asList(loadBalancer.getCostFunctionNames()) - .contains(DummyCostFunction.class.getSimpleName())); - } finally { - conf.unset(StochasticLoadBalancer.COST_FUNCTIONS_COST_FUNCTIONS_KEY); - loadBalancer.onConfigurationChange(conf); - } + loadBalancer.onConfigurationChange(conf); + assertTrue(Arrays.asList(loadBalancer.getCostFunctionNames()) + .contains(DummyCostFunction.class.getSimpleName())); } @Test From 513b477b578c5a47f720edd3565cf4af1c735b10 Mon Sep 17 00:00:00 2001 From: David Manning Date: Mon, 2 May 2022 10:21:54 -0700 Subject: [PATCH 3/4] resolve import merge conflict mistake --- .../hbase/master/balancer/TestStochasticLoadBalancer.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java index 66116d5f2181..05b5cc120d5f 100644 --- a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java +++ b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java @@ -51,8 +51,6 @@ import org.apache.hadoop.hbase.util.Bytes; import org.junit.Before; import org.junit.BeforeClass; -import org.junit.Before; -import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Test; import org.junit.experimental.categories.Category; From a8755702bac984b8fa2eb0575adbc771caba4789 Mon Sep 17 00:00:00 2001 From: David Manning Date: Mon, 2 May 2022 10:23:55 -0700 Subject: [PATCH 4/4] spotless:apply --- .../hbase/master/balancer/TestStochasticLoadBalancer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java index 05b5cc120d5f..21f3a3b66c9a 100644 --- a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java +++ b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java @@ -260,8 +260,8 @@ public void testUpdateBalancerLoadInfo() { if (isByTable) { metricRecordKey = "table1#" + StochasticLoadBalancer.OVERALL_COST_FUNCTION_NAME; } else { - metricRecordKey = HConstants.ENSEMBLE_TABLE_NAME + "#" - + StochasticLoadBalancer.OVERALL_COST_FUNCTION_NAME; + metricRecordKey = + HConstants.ENSEMBLE_TABLE_NAME + "#" + StochasticLoadBalancer.OVERALL_COST_FUNCTION_NAME; } double curOverallCost = loadBalancer.computeCost(clusterState, Double.MAX_VALUE); double curOverallCostInMetrics =