From 3a1a39d40d8a4733c5e657793eaec1c4cf40dde8 Mon Sep 17 00:00:00 2001 From: stack Date: Tue, 4 Feb 2020 16:39:33 -0800 Subject: [PATCH] HBASE-23789 [Flakey Tests] ERROR [Time-limited test] balancer.HeterogeneousRegionCountCostFunction(199): cannot read rules file located at ' /tmp/hbase-balancer.rules '; ADDENDUM Missed adding these files. --- .../HeterogeneousRegionCountCostFunction.java | 2 +- .../hadoop/hbase/HBaseTestingUtility.java | 4 +- ...ochasticLoadBalancerHeterogeneousCost.java | 32 ++-- ...ticLoadBalancerHeterogeneousCostRules.java | 177 +++++++++++------- 4 files changed, 133 insertions(+), 82 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/HeterogeneousRegionCountCostFunction.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/HeterogeneousRegionCountCostFunction.java index e457987fc84..a9bb6851ad2 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/HeterogeneousRegionCountCostFunction.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/HeterogeneousRegionCountCostFunction.java @@ -249,7 +249,7 @@ public class HeterogeneousRegionCountCostFunction extends StochasticLoadBalancer LOG.info("Cluster can hold " + this.cluster.numRegions + "/" + this.totalCapacity + " regions (" + Math.round(overallUsage * 100) + "%)"); if (overallUsage >= 1) { - LOG.warn("Cluster is overused"); + LOG.warn("Cluster is overused, {}", overallUsage); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java index a4d789fd1e2..575e65dc3d6 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java @@ -401,9 +401,9 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility { * value unintentionally -- but not anything can do about it at moment; * single instance only is how the minidfscluster works. * - * We also create the underlying directory for + * We also create the underlying directory names for * hadoop.log.dir, mapreduce.cluster.local.dir and hadoop.tmp.dir, and set the values - * in the conf, and as a system property for hadoop.tmp.dir + * in the conf, and as a system property for hadoop.tmp.dir (We do not create them!). * * @return The calculated data test build directory, if newly-created. */ diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerHeterogeneousCost.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerHeterogeneousCost.java index d30519227ef..51b43514f4b 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerHeterogeneousCost.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerHeterogeneousCost.java @@ -1,4 +1,4 @@ -/** +/* * Licensed to the Apache Software Foundation (ASF) under one or more contributor license * agreements. See the NOTICE file distributed with this work for additional information regarding * copyright ownership. The ASF licenses this file to you under the Apache License, Version 2.0 (the @@ -17,7 +17,6 @@ package org.apache.hadoop.hbase.master.balancer; import static junit.framework.TestCase.assertNotNull; import static junit.framework.TestCase.assertTrue; import static org.junit.Assert.assertNull; - import java.io.IOException; import java.util.Arrays; import java.util.Collections; @@ -29,8 +28,9 @@ import java.util.Random; import java.util.TreeMap; import java.util.concurrent.ThreadLocalRandom; +import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.hbase.HBaseClassTestRule; -import org.apache.hadoop.hbase.HBaseConfiguration; +import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.RegionReplicaUtil; @@ -47,29 +47,32 @@ import org.slf4j.LoggerFactory; @Category({ MasterTests.class, MediumTests.class }) public class TestStochasticLoadBalancerHeterogeneousCost extends BalancerTestBase { - @ClassRule public static final HBaseClassTestRule CLASS_RULE = HBaseClassTestRule.forClass(TestStochasticLoadBalancerHeterogeneousCost.class); private static final Logger LOG = LoggerFactory.getLogger(TestStochasticLoadBalancerHeterogeneousCost.class); - private static final double allowedWindow = 1.20; + private static final double ALLOWED_WINDOW = 1.20; + private static final HBaseTestingUtility HTU = new HBaseTestingUtility(); + private static String RULES_FILE; @BeforeClass - public static void beforeAllTests() { - BalancerTestBase.conf = HBaseConfiguration.create(); + public static void beforeAllTests() throws IOException { + BalancerTestBase.conf = HTU.getConfiguration(); BalancerTestBase.conf.setFloat("hbase.master.balancer.stochastic.regionCountCost", 0); BalancerTestBase.conf.setFloat("hbase.master.balancer.stochastic.primaryRegionCountCost", 0); BalancerTestBase.conf.setFloat("hbase.master.balancer.stochastic.tableSkewCost", 0); BalancerTestBase.conf.setBoolean("hbase.master.balancer.stochastic.runMaxSteps", true); BalancerTestBase.conf.set(StochasticLoadBalancer.COST_FUNCTIONS_COST_FUNCTIONS_KEY, HeterogeneousRegionCountCostFunction.class.getName()); - + // Need to ensure test dir has been created. + assertTrue(FileSystem.get(HTU.getConfiguration()).mkdirs(HTU.getDataTestDir())); + RULES_FILE = HTU.getDataTestDir( + TestStochasticLoadBalancerHeterogeneousCostRules.DEFAULT_RULES_FILE_NAME).toString(); BalancerTestBase.conf.set( HeterogeneousRegionCountCostFunction.HBASE_MASTER_BALANCER_HETEROGENEOUS_RULES_FILE, - TestStochasticLoadBalancerHeterogeneousCostRules.DEFAULT_RULES_TMP_LOCATION); - + RULES_FILE); BalancerTestBase.loadBalancer = new StochasticLoadBalancer(); BalancerTestBase.loadBalancer.setConf(BalancerTestBase.conf); } @@ -142,7 +145,7 @@ public class TestStochasticLoadBalancerHeterogeneousCost extends BalancerTestBas final int numRegions = 120; final int numRegionsPerServer = 60; - TestStochasticLoadBalancerHeterogeneousCostRules.createSimpleRulesFile(rules); + TestStochasticLoadBalancerHeterogeneousCostRules.createRulesFile(RULES_FILE); final Map> serverMap = this.createServerMap(numNodes, numRegions, numRegionsPerServer, 1, 1); final List plans = BalancerTestBase.loadBalancer.balanceCluster(serverMap); @@ -154,7 +157,7 @@ public class TestStochasticLoadBalancerHeterogeneousCost extends BalancerTestBas private void testHeterogeneousWithCluster(final int numNodes, final int numRegions, final int numRegionsPerServer, final List rules) throws IOException { - TestStochasticLoadBalancerHeterogeneousCostRules.createSimpleRulesFile(rules); + TestStochasticLoadBalancerHeterogeneousCostRules.createRulesFile(RULES_FILE, rules); final Map> serverMap = this.createServerMap(numNodes, numRegions, numRegionsPerServer, 1, 1); this.testWithCluster(serverMap, null, true, false); @@ -207,8 +210,9 @@ public class TestStochasticLoadBalancerHeterogeneousCost extends BalancerTestBas // as the balancer is stochastic, we cannot check exactly the result of the balancing, // hence the allowedWindow parameter assertTrue("Host " + sn.getHostname() + " should be below " - + cf.overallUsage * allowedWindow * 100 + "%", - usage <= cf.overallUsage * allowedWindow); + + cf.overallUsage * ALLOWED_WINDOW * 100 + "%; " + cf.overallUsage + + ", " + usage + ", " + numberRegions + ", " + limit, + usage <= cf.overallUsage * ALLOWED_WINDOW); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerHeterogeneousCostRules.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerHeterogeneousCostRules.java index a647079aadc..fd635d2a48e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerHeterogeneousCostRules.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerHeterogeneousCostRules.java @@ -1,4 +1,4 @@ -/** +/* * Licensed to the Apache Software Foundation (ASF) under one or more contributor license * agreements. See the NOTICE file distributed with this work for additional information regarding * copyright ownership. The ASF licenses this file to you under the Apache License, Version 2.0 (the @@ -14,148 +14,195 @@ */ package org.apache.hadoop.hbase.master.balancer; -import java.io.File; import java.io.IOException; import java.nio.charset.Charset; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.util.ArrayList; +import java.nio.file.FileSystems; +import java.nio.file.NoSuchFileException; import java.util.Arrays; import java.util.Collections; import java.util.List; - import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FSDataOutputStream; +import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hdfs.DistributedFileSystem; import org.apache.hadoop.hdfs.MiniDFSCluster; -import org.junit.AfterClass; import org.junit.Assert; +import org.junit.Before; import org.junit.BeforeClass; import org.junit.ClassRule; +import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.junit.rules.TestName; + +import static junit.framework.TestCase.assertTrue; @Category({ MasterTests.class, MediumTests.class }) public class TestStochasticLoadBalancerHeterogeneousCostRules extends BalancerTestBase { - @ClassRule public static final HBaseClassTestRule CLASS_RULE = HBaseClassTestRule.forClass(TestStochasticLoadBalancerHeterogeneousCostRules.class); + @Rule + public TestName name = new TestName(); - static final String DEFAULT_RULES_TMP_LOCATION = "/tmp/hbase-balancer.rules"; - static Configuration conf; + static final String DEFAULT_RULES_FILE_NAME = "hbase-balancer.rules"; private HeterogeneousRegionCountCostFunction costFunction; + private static final HBaseTestingUtility HTU = new HBaseTestingUtility(); + + /** + * Make a file for rules that is inside a temporary test dir named for the method so it doesn't + * clash w/ other rule files. + */ + private String rulesFilename; @BeforeClass - public static void beforeAllTests() throws Exception { - createSimpleRulesFile(new ArrayList<>()); - conf = new Configuration(); - conf.set(HeterogeneousRegionCountCostFunction.HBASE_MASTER_BALANCER_HETEROGENEOUS_RULES_FILE, - DEFAULT_RULES_TMP_LOCATION); + public static void beforeClass() throws IOException { + // Ensure test dir is created + HTU.getTestFileSystem().mkdirs(HTU.getDataTestDir()); } - static void createSimpleRulesFile(final List lines) throws IOException { - cleanup(); - final Path file = Paths.get(DEFAULT_RULES_TMP_LOCATION); - Files.write(file, lines, Charset.forName("UTF-8")); + @Before + public void before() throws IOException { + // New rules file name per test. + this.rulesFilename = HTU.getDataTestDir( + this.name.getMethodName() + "." + DEFAULT_RULES_FILE_NAME).toString(); + // Set the created rules filename into the configuration. + HTU.getConfiguration().set( + HeterogeneousRegionCountCostFunction.HBASE_MASTER_BALANCER_HETEROGENEOUS_RULES_FILE, + this.rulesFilename); } - protected static void cleanup() { - final File file = new File(DEFAULT_RULES_TMP_LOCATION); - file.delete(); + /** + * @param file Name of file to write rules into. + * @return Full file name of the rules file which is dir + DEFAULT_RULES_FILE_NAME. + */ + static String createRulesFile(String file, final List lines) throws IOException { + cleanup(file); + java.nio.file.Path path = + java.nio.file.Files.createFile(FileSystems.getDefault().getPath(file)); + return java.nio.file.Files.write(path, lines, Charset.forName("UTF-8")).toString(); } - @AfterClass - public static void afterAllTests() { - cleanup(); + /** + * @param file Name of file to write rules into. + * @return Full file name of the rules file which is dir + DEFAULT_RULES_FILE_NAME. + */ + static String createRulesFile(String file) throws IOException { + return createRulesFile(file, Collections.emptyList()); + } + + private static void cleanup(String file) throws IOException { + try { + java.nio.file.Files.delete(FileSystems.getDefault().getPath(file)); + } catch (NoSuchFileException nsfe) { + System.out.println("FileNotFoundException for " + file); + } } @Test - public void testNoRules() { - cleanup(); - this.costFunction = new HeterogeneousRegionCountCostFunction(conf); + public void testNoRules() throws IOException { + // Override what is in the configuration with the name of a non-existent file! + HTU.getConfiguration().set( + HeterogeneousRegionCountCostFunction.HBASE_MASTER_BALANCER_HETEROGENEOUS_RULES_FILE, + "non-existent-file!"); + this.costFunction = new HeterogeneousRegionCountCostFunction(HTU.getConfiguration()); this.costFunction.loadRules(); Assert.assertEquals(0, this.costFunction.getNumberOfRulesLoaded()); } @Test public void testBadFormatInRules() throws IOException { - createSimpleRulesFile(new ArrayList<>()); - this.costFunction = new HeterogeneousRegionCountCostFunction(conf); + // See {@link #before} above. It sets this.rulesFilename, and + // HeterogeneousRegionCountCostFunction.HBASE_MASTER_BALANCER_HETEROGENEOUS_RULES_FILE, + // in the configuration. + this.costFunction = new HeterogeneousRegionCountCostFunction(HTU.getConfiguration()); this.costFunction.loadRules(); Assert.assertEquals(0, this.costFunction.getNumberOfRulesLoaded()); - createSimpleRulesFile(Collections.singletonList("bad rules format")); - this.costFunction = new HeterogeneousRegionCountCostFunction(conf); + createRulesFile(this.rulesFilename, Collections.singletonList("bad rules format")); + this.costFunction = new HeterogeneousRegionCountCostFunction(HTU.getConfiguration()); this.costFunction.loadRules(); Assert.assertEquals(0, this.costFunction.getNumberOfRulesLoaded()); - createSimpleRulesFile(Arrays.asList("srv[1-2] 10", "bad_rules format", "a")); - this.costFunction = new HeterogeneousRegionCountCostFunction(conf); + createRulesFile(this.rulesFilename, Arrays.asList("srv[1-2] 10", + "bad_rules format", "a")); + this.costFunction = new HeterogeneousRegionCountCostFunction(HTU.getConfiguration()); this.costFunction.loadRules(); Assert.assertEquals(1, this.costFunction.getNumberOfRulesLoaded()); } @Test public void testTwoRules() throws IOException { - createSimpleRulesFile(Arrays.asList("^server1$ 10", "^server2 21")); - this.costFunction = new HeterogeneousRegionCountCostFunction(conf); + // See {@link #before} above. It sets this.rulesFilename, and + // HeterogeneousRegionCountCostFunction.HBASE_MASTER_BALANCER_HETEROGENEOUS_RULES_FILE, + // in the configuration. + // See {@link #before} above. It sets + // HeterogeneousRegionCountCostFunction.HBASE_MASTER_BALANCER_HETEROGENEOUS_RULES_FILE, + // in the configuration. + createRulesFile(this.rulesFilename, Arrays.asList("^server1$ 10", "^server2 21")); + this.costFunction = new HeterogeneousRegionCountCostFunction(HTU.getConfiguration()); this.costFunction.loadRules(); Assert.assertEquals(2, this.costFunction.getNumberOfRulesLoaded()); } @Test public void testBadRegexp() throws IOException { - createSimpleRulesFile(Collections.singletonList("server[ 1")); - this.costFunction = new HeterogeneousRegionCountCostFunction(conf); + // See {@link #before} above. It sets this.rulesFilename, and + // HeterogeneousRegionCountCostFunction.HBASE_MASTER_BALANCER_HETEROGENEOUS_RULES_FILE, + // in the configuration. + // See {@link #before} above. It sets + // HeterogeneousRegionCountCostFunction.HBASE_MASTER_BALANCER_HETEROGENEOUS_RULES_FILE, + // in the configuration. + createRulesFile(this.rulesFilename, Collections.singletonList("server[ 1")); + this.costFunction = new HeterogeneousRegionCountCostFunction(HTU.getConfiguration()); this.costFunction.loadRules(); Assert.assertEquals(0, this.costFunction.getNumberOfRulesLoaded()); } @Test public void testNoOverride() throws IOException { - createSimpleRulesFile(Arrays.asList("^server1$ 10", "^server2 21")); - this.costFunction = new HeterogeneousRegionCountCostFunction(conf); + // See {@link #before} above. It sets this.rulesFilename, and + // HeterogeneousRegionCountCostFunction.HBASE_MASTER_BALANCER_HETEROGENEOUS_RULES_FILE, + // in the configuration. + createRulesFile(this.rulesFilename, Arrays.asList("^server1$ 10", "^server2 21")); + this.costFunction = new HeterogeneousRegionCountCostFunction(HTU.getConfiguration()); this.costFunction.loadRules(); Assert.assertEquals(2, this.costFunction.getNumberOfRulesLoaded()); // loading malformed configuration does not overload current - cleanup(); + cleanup(this.rulesFilename); this.costFunction.loadRules(); Assert.assertEquals(2, this.costFunction.getNumberOfRulesLoaded()); } @Test public void testLoadingFomHDFS() throws Exception { + HTU.startMiniDFSCluster(3); + try { + MiniDFSCluster cluster = HTU.getDFSCluster(); + DistributedFileSystem fs = cluster.getFileSystem(); + // Writing file + Path path = new Path(fs.getHomeDirectory(), DEFAULT_RULES_FILE_NAME); + FSDataOutputStream stream = fs.create(path); + stream.write("server1 10".getBytes()); + stream.flush(); + stream.close(); - HBaseTestingUtility hBaseTestingUtility = new HBaseTestingUtility(); - hBaseTestingUtility.startMiniDFSCluster(3); + Configuration configuration = HTU.getConfiguration(); - MiniDFSCluster cluster = hBaseTestingUtility.getDFSCluster(); - DistributedFileSystem fs = cluster.getFileSystem(); - - String path = cluster.getURI() + DEFAULT_RULES_TMP_LOCATION; - - // writing file - FSDataOutputStream stream = fs.create(new org.apache.hadoop.fs.Path(path)); - stream.write("server1 10".getBytes()); - stream.flush(); - stream.close(); - - Configuration configuration = hBaseTestingUtility.getConfiguration(); - - // start costFunction - configuration.set( - HeterogeneousRegionCountCostFunction.HBASE_MASTER_BALANCER_HETEROGENEOUS_RULES_FILE, path); - this.costFunction = new HeterogeneousRegionCountCostFunction(configuration); - this.costFunction.loadRules(); - Assert.assertEquals(1, this.costFunction.getNumberOfRulesLoaded()); - - hBaseTestingUtility.shutdownMiniCluster(); + // start costFunction + configuration.set( + HeterogeneousRegionCountCostFunction.HBASE_MASTER_BALANCER_HETEROGENEOUS_RULES_FILE, + path.toString()); + this.costFunction = new HeterogeneousRegionCountCostFunction(configuration); + this.costFunction.loadRules(); + Assert.assertEquals(1, this.costFunction.getNumberOfRulesLoaded()); + } finally { + HTU.shutdownMiniCluster(); + } } }