HBASE-26147: Add dry_run_balancer and related Admin interfaces for running the balancer without executing any region moves

bbeaudreault · bbeaudreault · commit 9a6544b65ce0 · 2021-07-27T17:25:59.000-04:00
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
@@ -1253,6 +1253,15 @@ default boolean balancer() throws IOException {
    */
   boolean balance() throws IOException;
 
+  /**
+   * Invoke the balancer in dry run mode. Will show plan but not actually move any regions.
+   * Can NOT run for various reasons. Check logs.
+   *
+   * @return <code>true</code> if dry run ran, <code>false</code> otherwise.
+   * @throws IOException if a remote or network exception occurs
+   */
+  boolean dryRunBalance() throws IOException;
+
   /**
    * Invoke the balancer.  Will run the balancer and if regions to move, it will
    * go ahead and do the reassignments. If there is region in transition, force parameter of true
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java
@@ -20,6 +20,7 @@
 import static org.apache.hadoop.hbase.util.FutureUtils.addListener;
 
 import com.google.protobuf.RpcChannel;
+import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.EnumSet;
@@ -1270,6 +1271,14 @@ default CompletableFuture<Boolean> balance() {
    */
   CompletableFuture<Boolean> balance(boolean forcible);
 
+  /**
+   * Invoke the balancer in dry run mode. Will show plan but not actually move any regions.
+   * Can NOT run for various reasons. Check logs.
+   *
+   * @return <code>true</code> if dry run ran, <code>false</code> otherwise.
+   */
+  CompletableFuture<Boolean> dryRunBalance();
+
   /**
    * Query the current state of the balancer.
    * @return true if the balance switch is on, false otherwise. The return value will be wrapped by a
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java
@@ -688,6 +688,11 @@ public CompletableFuture<Boolean> balance(boolean forcible) {
     return wrap(rawAdmin.balance(forcible));
   }
 
+  @Override
+  public CompletableFuture<Boolean> dryRunBalance() {
+    return wrap(rawAdmin.dryRunBalance());
+  }
+
   @Override
   public CompletableFuture<Boolean> isBalancerEnabled() {
     return wrap(rawAdmin.isBalancerEnabled());
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java
@@ -1488,6 +1488,12 @@ public MasterProtos.BalanceResponse balance(RpcController controller,
         return stub.balance(controller, request);
       }
 
+      @Override
+      public MasterProtos.BalanceResponse dryRunBalance(RpcController controller,
+        MasterProtos.BalanceRequest request) throws ServiceException {
+        return stub.dryRunBalance(controller, request);
+      }
+
       @Override
       public MasterProtos.SetBalancerRunningResponse setBalancerRunning(
           RpcController controller, MasterProtos.SetBalancerRunningRequest request)
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
@@ -1498,6 +1498,17 @@ protected Boolean rpcCall() throws Exception {
     });
   }
 
+  @Override
+  public boolean dryRunBalance() throws IOException {
+    return executeCallable(new MasterCallable<Boolean>(getConnection(), getRpcControllerFactory()) {
+      @Override
+      protected Boolean rpcCall() throws Exception {
+        return master.dryRunBalance(getRpcController(),
+          RequestConverter.buildBalanceRequest(false)).getBalancerRan();
+      }
+    });
+  }
+
   @Override
   public boolean isBalancerEnabled() throws IOException {
     return executeCallable(new MasterCallable<Boolean>(getConnection(), getRpcControllerFactory()) {
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
@@ -3219,6 +3219,16 @@ public CompletableFuture<Boolean> balance(boolean forcible) {
             (s, c, req, done) -> s.balance(c, req, done), (resp) -> resp.getBalancerRan())).call();
   }
 
+  @Override
+  public CompletableFuture<Boolean> dryRunBalance() {
+    return this
+      .<Boolean> newMasterCaller()
+      .action(
+        (controller, stub) -> this.<BalanceRequest, BalanceResponse, Boolean> call(controller,
+          stub, RequestConverter.buildBalanceRequest(false),
+          (s, c, req, done) -> s.dryRunBalance(c, req, done), (resp) -> resp.getBalancerRan())).call();
+  }
+
   @Override
   public CompletableFuture<Boolean> isBalancerEnabled() {
     return this
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ShortCircuitMasterConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ShortCircuitMasterConnection.java
@@ -599,6 +599,12 @@ public BalanceResponse balance(RpcController controller, BalanceRequest request)
     return stub.balance(controller, request);
   }
 
+  @Override
+  public BalanceResponse dryRunBalance(RpcController controller, BalanceRequest request)
+    throws ServiceException {
+    return stub.dryRunBalance(controller, request);
+  }
+
   @Override
   public AssignRegionResponse assignRegion(RpcController controller, AssignRegionRequest request)
       throws ServiceException {
diff --git a/hbase-protocol-shaded/src/main/protobuf/Master.proto b/hbase-protocol-shaded/src/main/protobuf/Master.proto
@@ -841,6 +841,14 @@ service MasterService {
   rpc Balance(BalanceRequest)
     returns(BalanceResponse);
 
+  /**
+     * Execute a dry-run of the balancer. This executes the balancer along with all
+     * logging but does not actually move any regions. This is useful for debugging
+     * balancer changes. Can NOT run for various reasons. Check logs.
+     */
+    rpc DryRunBalance(BalanceRequest)
+      returns (BalanceResponse);
+
   /**
    * Turn the load balancer on or off.
    * If synchronous is true, it waits until current balance() call, if outstanding, to return.
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
@@ -1724,7 +1724,7 @@ private void balanceThrottling(long nextBalanceStartTime, int maxRegionsInTransi
   }
 
   public boolean balance() throws IOException {
-    return balance(false);
+    return balance(LoadBalancer.RunMode.NORMAL);
   }
 
   /**
@@ -1750,8 +1750,8 @@ public boolean skipRegionManagementAction(final String action) {
     return false;
   }
 
-  public boolean balance(boolean force) throws IOException {
-    if (loadBalancerTracker == null || !loadBalancerTracker.isBalancerOn()) {
+  public boolean balance(LoadBalancer.RunMode runMode) throws IOException {
+    if (loadBalancerTracker == null || (!loadBalancerTracker.isBalancerOn() && !runMode.isDryRun())) {
       return false;
     }
     if (skipRegionManagementAction("balancer")) {
@@ -1772,8 +1772,8 @@ public boolean balance(boolean force) throws IOException {
           toPrint = regionsInTransition.subList(0, max);
           truncated = true;
         }
-        if (!force || metaInTransition) {
-          LOG.info("Not running balancer (force=" + force + ", metaRIT=" + metaInTransition +
+        if (!runMode.isForced() || metaInTransition) {
+          LOG.info("Not running balancer (force=" + runMode.isForced() + ", metaRIT=" + metaInTransition +
             ") because " + regionsInTransition.size() + " region(s) in transition: " + toPrint +
             (truncated? "(truncated list)": ""));
           return false;
@@ -1809,6 +1809,10 @@ public boolean balance(boolean force) throws IOException {
 
       List<RegionPlan> plans = this.balancer.balanceCluster(assignments);
 
+      if (runMode.isDryRun()) {
+        return true;
+      }
+
       if (skipRegionManagementAction("balancer")) {
         // make one last check that the cluster isn't shutting down before proceeding.
         return false;
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java
@@ -86,6 +86,18 @@ public interface LoadBalancer extends Stoppable, ConfigurationObserver {
   // We deliberately use 'localhost' so the operation will fail fast
   ServerName BOGUS_SERVER_NAME = ServerName.valueOf("localhost,1,1");
 
+  enum RunMode {
+    NORMAL, FORCE, DRY_RUN;
+
+    boolean isForced() {
+      return this == FORCE;
+    }
+
+    boolean isDryRun() {
+      return this == DRY_RUN;
+    }
+  }
+
   /**
    * Set the current cluster status. This allows a LoadBalancer to map host name to a server
    */
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
@@ -676,7 +676,16 @@ public BalanceResponse balance(RpcController controller,
       BalanceRequest request) throws ServiceException {
     try {
       return BalanceResponse.newBuilder().setBalancerRan(master.balance(
-        request.hasForce()? request.getForce(): false)).build();
+        request.hasForce() && request.getForce() ? LoadBalancer.RunMode.FORCE : LoadBalancer.RunMode.NORMAL)).build();
+    } catch (IOException ex) {
+      throw new ServiceException(ex);
+    }
+  }
+
+  @Override public BalanceResponse dryRunBalance(RpcController controller, BalanceRequest request)
+    throws ServiceException {
+    try {
+      return BalanceResponse.newBuilder().setBalancerRan(master.balance(LoadBalancer.RunMode.DRY_RUN)).build();
     } catch (IOException ex) {
       throw new ServiceException(ex);
     }
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterDryRunBalancer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterDryRunBalancer.java
@@ -0,0 +1,97 @@
+package org.apache.hadoop.hbase.master;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import java.io.IOException;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.regionserver.HRegionServer;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.junit.After;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.mockito.Mockito;
+
+@Category({ MasterTests.class, MediumTests.class})
+public class TestMasterDryRunBalancer {
+  @ClassRule
+  public static final HBaseClassTestRule CLASS_RULE =
+    HBaseClassTestRule.forClass(TestMasterDryRunBalancer.class);
+
+  private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
+  private static final byte[] FAMILYNAME = Bytes.toBytes("fam");
+
+  @After
+  public void shutdown() throws Exception {
+    TEST_UTIL.shutdownMiniCluster();
+  }
+
+  @Test
+  public void testDryRunBalancer() throws Exception {
+    TEST_UTIL.startMiniCluster(2);
+
+    TableName tableName = createTable("testDryRunBalancer");
+    HMaster master = Mockito.spy(TEST_UTIL.getHBaseCluster().getMaster());
+
+    // dry run should be possible with balancer disabled
+    // disabling it will ensure the chore does not mess with our forced unbalance below
+    master.balanceSwitch(false);
+    assertFalse(master.isBalancerOn());
+
+    HRegionServer biasedServer = unbalance(master, tableName);
+
+    assertTrue(master.balance(LoadBalancer.RunMode.DRY_RUN));
+
+    // sanity check that we truly don't try to execute any plans
+    Mockito.verify(master, Mockito.never()).executeRegionPlansWithThrottling(Mockito.anyList());
+
+    // should still be unbalanced post dry run
+    assertServerContainsAllRegions(biasedServer.getServerName(), tableName);
+
+    TEST_UTIL.deleteTable(tableName);
+  }
+
+  private TableName createTable(String table) throws IOException {
+    TableName tableName = TableName.valueOf(table);
+    byte[] startKey = new byte[] { 0x00 };
+    byte[] stopKey = new byte[] { 0x7f };
+    TEST_UTIL.createTable(tableName, new byte[][] { FAMILYNAME }, 1, startKey, stopKey,
+      100);
+    return tableName;
+  }
+
+
+  private HRegionServer unbalance(HMaster master, TableName tableName) throws Exception {
+    while (master.getAssignmentManager().getRegionStates().getRegionsInTransitionCount() > 0) {
+      Thread.sleep(100);
+    }
+    HRegionServer biasedServer = TEST_UTIL.getMiniHBaseCluster().getRegionServer(0);
+
+    for (RegionInfo regionInfo : TEST_UTIL.getAdmin().getRegions(tableName)) {
+      master.move(regionInfo.getEncodedNameAsBytes(),
+        Bytes.toBytes(biasedServer.getServerName().getServerName()));
+    }
+
+    while (master.getAssignmentManager().getRegionStates().getRegionsInTransitionCount() > 0) {
+      Thread.sleep(100);
+    }
+
+    assertServerContainsAllRegions(biasedServer.getServerName(), tableName);
+
+    return biasedServer;
+  }
+
+  private void assertServerContainsAllRegions(ServerName serverName, TableName tableName)
+    throws IOException {
+    int numRegions = TEST_UTIL.getAdmin().getRegions(tableName).size();
+    assertEquals(numRegions,
+      TEST_UTIL.getMiniHBaseCluster().getRegionServer(serverName).getRegions(tableName).size());
+  }
+}
diff --git a/hbase-shell/src/main/ruby/shell/commands/dry_run_balancer.rb b/hbase-shell/src/main/ruby/shell/commands/dry_run_balancer.rb
@@ -0,0 +1,43 @@
+#
+#
+# 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
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+module Shell
+  module Commands
+    class DryRunBalancer < Command
+      def help
+        return <<-EOF
+Trigger a dry run of the cluster balancer. This will execute the balancer logic,
+without actually moving any regions in the cluster. The output of this is useful
+for debugging balancer changes, or even adjustments to balancer weights. You are
+effectively able to preview balancer changes to the cluster before actually
+executing them.
+This will NOT run if regions in transition. This also will NOT
+run if the balancer is enabled (as to make sure this logic does not clash with
+the actual balancer).
+EOF
+      end
+
+      def command()
+        did_balancer_run = !!admin.dryRunBalancer()
+        formatter.row([did_balancer_run.to_s])
+        did_balancer_run
+      end
+    end
+  end
+end
diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java
@@ -786,6 +786,10 @@ public boolean balance(boolean force) {
     throw new NotImplementedException("balance not supported in ThriftAdmin");
   }
 
+  @Override public boolean dryRunBalance() throws IOException {
+    throw new NotImplementedException("dryRunBalance not supported in ThriftAdmin");
+  }
+
   @Override
   public boolean isBalancerEnabled() {
     throw new NotImplementedException("isBalancerEnabled not supported in ThriftAdmin");
