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.

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);
     }
   }
 

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.
    */

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() {
+  public static void beforeAllTests() throws IOException {
-    BalancerTestBase.conf = HBaseConfiguration.create();
+    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<ServerName, List<RegionInfo>> serverMap =
         this.createServerMap(numNodes, numRegions, numRegionsPerServer, 1, 1);
     final List<RegionPlan> 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<String> rules) throws IOException {
 
-    TestStochasticLoadBalancerHeterogeneousCostRules.createSimpleRulesFile(rules);
+    TestStochasticLoadBalancerHeterogeneousCostRules.createRulesFile(RULES_FILE, rules);
     final Map<ServerName, List<RegionInfo>> 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 + "%",
+              + cf.overallUsage * ALLOWED_WINDOW * 100 + "%; " + cf.overallUsage +
-            usage <= cf.overallUsage * allowedWindow);
+              ", " + usage + ", " + numberRegions + ", " + limit,
+            usage <= cf.overallUsage * ALLOWED_WINDOW);
         }
       }
 

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.FileSystems;
-import java.nio.file.Path;
+import java.nio.file.NoSuchFileException;
-import java.nio.file.Paths;
-import java.util.ArrayList;
 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 final String DEFAULT_RULES_FILE_NAME = "hbase-balancer.rules";
-  static Configuration conf;
   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 {
+  public static void beforeClass() throws IOException {
-    createSimpleRulesFile(new ArrayList<>());
+    // Ensure test dir is created
-    conf = new Configuration();
+    HTU.getTestFileSystem().mkdirs(HTU.getDataTestDir());
-    conf.set(HeterogeneousRegionCountCostFunction.HBASE_MASTER_BALANCER_HETEROGENEOUS_RULES_FILE,
-      DEFAULT_RULES_TMP_LOCATION);
   }
 
-  static void createSimpleRulesFile(final List<String> lines) throws IOException {
+  @Before
-    cleanup();
+  public void before() throws IOException {
-    final Path file = Paths.get(DEFAULT_RULES_TMP_LOCATION);
+    // New rules file name per test.
-    Files.write(file, lines, Charset.forName("UTF-8"));
+    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);
+   * @param file Name of file to write rules into.
-    file.delete();
+   * @return Full file name of the rules file which is <code>dir</code> + DEFAULT_RULES_FILE_NAME.
+   */
+  static String createRulesFile(String file, final List<String> 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() {
+   * @param file Name of file to write rules into.
-    cleanup();
+   * @return Full file name of the rules file which is <code>dir</code> + 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() {
+  public void testNoRules() throws IOException {
-    cleanup();
+    // Override what is in the configuration with the name of a non-existent file!
-    this.costFunction = new HeterogeneousRegionCountCostFunction(conf);
+    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<>());
+    // See {@link #before} above. It sets this.rulesFilename, and
-    this.costFunction = new HeterogeneousRegionCountCostFunction(conf);
+    // 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"));
+    createRulesFile(this.rulesFilename, Collections.singletonList("bad rules format"));
-    this.costFunction = new HeterogeneousRegionCountCostFunction(conf);
+    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"));
+    createRulesFile(this.rulesFilename, Arrays.asList("srv[1-2] 10",
-    this.costFunction = new HeterogeneousRegionCountCostFunction(conf);
+      "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"));
+    // See {@link #before} above. It sets this.rulesFilename, and
-    this.costFunction = new HeterogeneousRegionCountCostFunction(conf);
+    // 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"));
+    // See {@link #before} above. It sets this.rulesFilename, and
-    this.costFunction = new HeterogeneousRegionCountCostFunction(conf);
+    // 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"));
+    // See {@link #before} above. It sets this.rulesFilename, and
-    this.costFunction = new HeterogeneousRegionCountCostFunction(conf);
+    // 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();
+      Configuration configuration = HTU.getConfiguration();
-    hBaseTestingUtility.startMiniDFSCluster(3);
 
-    MiniDFSCluster cluster = hBaseTestingUtility.getDFSCluster();
+      // start costFunction
-    DistributedFileSystem fs = cluster.getFileSystem();
+      configuration.set(
-
+        HeterogeneousRegionCountCostFunction.HBASE_MASTER_BALANCER_HETEROGENEOUS_RULES_FILE,
-    String path = cluster.getURI() + DEFAULT_RULES_TMP_LOCATION;
+        path.toString());
-
+      this.costFunction = new HeterogeneousRegionCountCostFunction(configuration);
-    // writing file
+      this.costFunction.loadRules();
-    FSDataOutputStream stream = fs.create(new org.apache.hadoop.fs.Path(path));
+      Assert.assertEquals(1, this.costFunction.getNumberOfRulesLoaded());
-    stream.write("server1 10".getBytes());
+    } finally {
-    stream.flush();
+      HTU.shutdownMiniCluster();
-    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();
   }
 }