From 4edd4326e2c74e33ab67380b36fb4e6266158d4e Mon Sep 17 00:00:00 2001 From: Bryan Beaudreault Date: Tue, 27 Jul 2021 17:25:59 -0400 Subject: [PATCH 1/3] HBASE-26147: Add dry_run_balancer and related Admin interfaces for running the balancer without executing any region moves --- .../org/apache/hadoop/hbase/client/Admin.java | 27 +++- .../hadoop/hbase/client/AsyncAdmin.java | 21 +++- .../hadoop/hbase/client/AsyncHBaseAdmin.java | 4 +- .../hadoop/hbase/client/BalanceRequest.java | 96 +++++++++++++++ .../hadoop/hbase/client/HBaseAdmin.java | 20 +-- .../hbase/client/RawAsyncHBaseAdmin.java | 10 +- .../hbase/shaded/protobuf/ProtobufUtil.java | 15 +++ .../shaded/protobuf/RequestConverter.java | 10 -- .../src/main/protobuf/Master.proto | 3 +- .../hadoop/hbase/rsgroup/RSGroupAdmin.java | 13 +- .../hbase/rsgroup/RSGroupAdminClient.java | 9 +- .../hbase/rsgroup/RSGroupAdminEndpoint.java | 8 +- .../hbase/rsgroup/RSGroupAdminServer.java | 14 ++- .../hbase/rsgroup/RSGroupProtobufUtil.java | 17 +++ .../src/main/protobuf/RSGroupAdmin.proto | 2 + .../hbase/rsgroup/TestRSGroupsBalance.java | 83 ++++++++----- .../hbase/rsgroup/TestRSGroupsBase.java | 5 +- .../rsgroup/VerifyingRSGroupAdminClient.java | 5 +- .../hbase/coprocessor/MasterObserver.java | 13 +- .../apache/hadoop/hbase/master/HMaster.java | 34 ++++-- .../hbase/master/MasterCoprocessorHost.java | 17 +-- .../hbase/master/MasterRpcServices.java | 5 +- .../security/access/AccessController.java | 3 +- .../hbase/client/TestMultiParallel.java | 2 +- .../hbase/coprocessor/TestMasterObserver.java | 10 +- .../master/TestMasterDryRunBalancer.java | 115 ++++++++++++++++++ .../security/access/TestAccessController.java | 3 +- .../access/TestWithDisabledAuthorization.java | 3 +- hbase-shell/src/main/ruby/hbase/admin.rb | 7 +- .../src/main/ruby/hbase/balancer_utils.rb | 57 +++++++++ .../src/main/ruby/hbase/rsgroup_admin.rb | 7 +- .../ruby/shell/commands/balance_rsgroup.rb | 13 +- .../src/main/ruby/shell/commands/balancer.rb | 26 ++-- hbase-shell/src/test/ruby/hbase/admin_test.rb | 4 + .../test/ruby/hbase/balancer_utils_test.rb | 78 ++++++++++++ .../src/test/ruby/shell/rsgroup_shell_test.rb | 2 + .../hbase/thrift2/client/ThriftAdmin.java | 5 + 37 files changed, 629 insertions(+), 137 deletions(-) create mode 100644 hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceRequest.java create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterDryRunBalancer.java create mode 100644 hbase-shell/src/main/ruby/hbase/balancer_utils.rb create mode 100644 hbase-shell/src/test/ruby/hbase/balancer_utils_test.rb 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 index fb616128d994..31295dc79a7e 100644 --- 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 @@ -1251,7 +1251,19 @@ default boolean balancer() throws IOException { * @return true if balancer ran, false otherwise. * @throws IOException if a remote or network exception occurs */ - boolean balance() throws IOException; + default boolean balance() throws IOException { + return balance(BalanceRequest.defaultInstance()); + } + + /** + * Invoke the balancer with the given balance request. The BalanceRequest defines how the + * balancer will run. See {@link BalanceRequest} for more details. + * + * @param request defines how the balancer should run + * @return true if balancer ran, false otherwise. + * @throws IOException if a remote or network exception occurs + */ + boolean balance(BalanceRequest request) throws IOException; /** * Invoke the balancer. Will run the balancer and if regions to move, it will @@ -1262,7 +1274,7 @@ default boolean balancer() throws IOException { * @return true if balancer ran, false otherwise. * @throws IOException if a remote or network exception occurs * @deprecated Since 2.0.0. Will be removed in 3.0.0. - * Use {@link #balance(boolean)} instead. + * Use {@link #balance(BalanceRequest)} instead. */ @Deprecated default boolean balancer(boolean force) throws IOException { @@ -1277,8 +1289,17 @@ default boolean balancer(boolean force) throws IOException { * @param force whether we should force balance even if there is region in transition * @return true if balancer ran, false otherwise. * @throws IOException if a remote or network exception occurs + * @deprecated Since 2.5.0. Will be removed in 4.0.0. + * Use {@link #balance(BalanceRequest)} instead. */ - boolean balance(boolean force) throws IOException; + @Deprecated + default boolean balance(boolean force) throws IOException { + return balance( + BalanceRequest.newBuilder() + .setIgnoreRegionsInTransition(force) + .build() + ); + } /** * Query the current state of the balancer. 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 index 6b8fda79f07b..38683cdc6dc6 100644 --- 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 @@ -1257,7 +1257,7 @@ default CompletableFuture balancerSwitch(boolean on) { * {@link CompletableFuture}. */ default CompletableFuture balance() { - return balance(false); + return balance(BalanceRequest.defaultInstance()); } /** @@ -1267,8 +1267,25 @@ default CompletableFuture balance() { * @param forcible whether we should force balance even if there is region in transition. * @return True if balancer ran, false otherwise. The return value will be wrapped by a * {@link CompletableFuture}. + * @deprecated Since 2.5.0. Will be removed in 4.0.0. + * Use {@link #balance(BalanceRequest)} instead. + */ + default CompletableFuture balance(boolean forcible) { + return balance( + BalanceRequest.newBuilder() + .setIgnoreRegionsInTransition(forcible) + .build() + ); + } + + /** + * Invoke the balancer with the given balance request. The BalanceRequest defines how the + * balancer will run. See {@link BalanceRequest} for more details. + * + * @param request defines how the balancer should run + * @return true if balancer ran, false otherwise. */ - CompletableFuture balance(boolean forcible); + CompletableFuture balance(BalanceRequest request); /** * Query the current state of the balancer. 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 index c7b9897f501f..adf1227fa32c 100644 --- 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 @@ -684,8 +684,8 @@ public CompletableFuture balancerSwitch(boolean on, boolean drainRITs) } @Override - public CompletableFuture balance(boolean forcible) { - return wrap(rawAdmin.balance(forcible)); + public CompletableFuture balance(BalanceRequest request) { + return wrap(rawAdmin.balance(request)); } @Override diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceRequest.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceRequest.java new file mode 100644 index 000000000000..fe1b1180c0b0 --- /dev/null +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceRequest.java @@ -0,0 +1,96 @@ +/* + * + * 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. + */ +package org.apache.hadoop.hbase.client; + +import org.apache.yetus.audience.InterfaceAudience; +import org.apache.yetus.audience.InterfaceStability; + +/** + * Encapsulates options for executing an unscheduled run of the Balancer. + */ +@InterfaceAudience.Public +@InterfaceStability.Evolving +public final class BalanceRequest { + private static final BalanceRequest DEFAULT = BalanceRequest.newBuilder().build(); + + @InterfaceAudience.Public + @InterfaceStability.Evolving + public final static class Builder { + private boolean dryRun = false; + private boolean ignoreRegionsInTransition = false; + + private Builder() {} + + /** + * Creates a BalancerRequest which runs the balancer in dryRun mode. + * In this mode, the balancer will try to find a plan but WILL NOT + * execute any region moves or call any coprocessors. + * + * You can run in dryRun mode regardless of whether the balancer switch + * is enabled or disabled, but dryRun mode will not run over an existing + * request or chore. + * + * Dry run is useful for testing out new balance configs. See the logs + * on the active HMaster for the results of the dry run. + */ + public Builder setDryRun(boolean dryRun) { + this.dryRun = dryRun; + return this; + } + + /** + * Creates a BalancerRequest to cause the balancer to run even if there + * are regions in transition. + * + * WARNING: Advanced usage only, this could cause more issues than it fixes. + */ + public Builder setIgnoreRegionsInTransition(boolean ignoreRegionsInTransition) { + this.ignoreRegionsInTransition = ignoreRegionsInTransition; + return this; + } + + public BalanceRequest build() { + return new BalanceRequest(dryRun, ignoreRegionsInTransition); + } + } + + public static Builder newBuilder() { + return new Builder(); + } + + public static BalanceRequest defaultInstance() { + return DEFAULT; + } + + private final boolean dryRun; + private final boolean ignoreRegionsInTransition; + + private BalanceRequest(boolean dryRun, boolean ignoreRegionsInTransition) { + this.dryRun = dryRun; + this.ignoreRegionsInTransition = ignoreRegionsInTransition; + } + + public boolean isDryRun() { + return dryRun; + } + + public boolean isIgnoreRegionsInTransition() { + return ignoreRegionsInTransition; + } +} 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 index b7ed2d03ad97..d6be0bc351cd 100644 --- 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 @@ -1477,23 +1477,11 @@ protected Boolean rpcCall() throws Exception { } @Override - public boolean balance() throws IOException { + public boolean balance(BalanceRequest request) throws IOException { return executeCallable(new MasterCallable(getConnection(), getRpcControllerFactory()) { - @Override - protected Boolean rpcCall() throws Exception { - return master.balance(getRpcController(), - RequestConverter.buildBalanceRequest(false)).getBalancerRan(); - } - }); - } - - @Override - public boolean balance(final boolean force) throws IOException { - return executeCallable(new MasterCallable(getConnection(), getRpcControllerFactory()) { - @Override - protected Boolean rpcCall() throws Exception { - return master.balance(getRpcController(), - RequestConverter.buildBalanceRequest(force)).getBalancerRan(); + @Override protected Boolean rpcCall() throws Exception { + return master.balance(getRpcController(), ProtobufUtil.toBalanceRequest(request)) + .getBalancerRan(); } }); } 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 index 57d4bac5a3bc..286d891ebc4f 100644 --- 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 @@ -126,14 +126,13 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.ProcedureDescription; import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.RegionSpecifier.RegionSpecifierType; import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.TableSchema; +import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.AbortProcedureRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.AbortProcedureResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.AddColumnRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.AddColumnResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.AssignRegionRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.AssignRegionResponse; -import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.BalanceRequest; -import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.BalanceResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.ClearDeadServersRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.ClearDeadServersResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.CreateNamespaceRequest; @@ -3210,15 +3209,16 @@ public CompletableFuture balancerSwitch(boolean on, boolean drainRITs) } @Override - public CompletableFuture balance(boolean forcible) { + public CompletableFuture balance(BalanceRequest request) { return this . newMasterCaller() .action( - (controller, stub) -> this. call(controller, - stub, RequestConverter.buildBalanceRequest(forcible), + (controller, stub) -> this. call(controller, + stub, ProtobufUtil.toBalanceRequest(request), (s, c, req, done) -> s.balance(c, req, done), (resp) -> resp.getBalancerRan())).call(); } + @Override public CompletableFuture isBalancerEnabled() { return this diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java index c2544f6d008a..7e02a0e8fb96 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java @@ -46,6 +46,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.ByteBufferExtendedCell; import org.apache.hadoop.hbase.CacheEvictionStats; import org.apache.hadoop.hbase.CacheEvictionStatsBuilder; @@ -3753,4 +3754,18 @@ public static HBaseProtos.LogRequest toBalancerRejectionRequest(int limit) { .build(); } + public static MasterProtos.BalanceRequest toBalanceRequest(BalanceRequest request) { + return MasterProtos.BalanceRequest.newBuilder() + .setDryRun(request.isDryRun()) + .setIgnoreRit(request.isIgnoreRegionsInTransition()) + .build(); + } + + public static BalanceRequest toBalanceRequest(MasterProtos.BalanceRequest request) { + return BalanceRequest.newBuilder() + .setDryRun(request.hasDryRun() && request.getDryRun()) + .setIgnoreRegionsInTransition(request.hasIgnoreRit() && request.getIgnoreRit()) + .build(); + } + } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/RequestConverter.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/RequestConverter.java index bdae13bbfce6..5f2181e3ccd1 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/RequestConverter.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/RequestConverter.java @@ -106,7 +106,6 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.AddColumnRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.AssignRegionRequest; -import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.BalanceRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.ClearDeadServersRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.CreateNamespaceRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.CreateTableRequest; @@ -1576,15 +1575,6 @@ public static IsMasterRunningRequest buildIsMasterRunningRequest() { return IsMasterRunningRequest.newBuilder().build(); } - /** - * Creates a protocol buffer BalanceRequest - * - * @return a BalanceRequest - */ - public static BalanceRequest buildBalanceRequest(boolean force) { - return BalanceRequest.newBuilder().setForce(force).build(); - } - /** * Creates a protocol buffer SetBalancerRunningRequest * diff --git a/hbase-protocol-shaded/src/main/protobuf/Master.proto b/hbase-protocol-shaded/src/main/protobuf/Master.proto index 045feb4a4162..ad5971678c53 100644 --- a/hbase-protocol-shaded/src/main/protobuf/Master.proto +++ b/hbase-protocol-shaded/src/main/protobuf/Master.proto @@ -292,7 +292,8 @@ message IsInMaintenanceModeResponse { } message BalanceRequest { - optional bool force = 1; + optional bool ignore_rit = 1; + optional bool dry_run = 2; } message BalanceResponse { diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdmin.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdmin.java index e698f9ac6f25..6049e385199e 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdmin.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdmin.java @@ -23,6 +23,7 @@ import java.util.Set; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.net.Address; import org.apache.yetus.audience.InterfaceAudience; @@ -67,7 +68,17 @@ public interface RSGroupAdmin { * * @return boolean Whether balance ran or not */ - boolean balanceRSGroup(String groupName) throws IOException; + default boolean balanceRSGroup(String groupName) throws IOException { + return balanceRSGroup(groupName, BalanceRequest.defaultInstance()); + } + + /** + * Balance regions in the given RegionServer group, running based on + * the given {@link BalanceRequest}. + * + * @return boolean Whether balance ran or not + */ + boolean balanceRSGroup(String groupName, BalanceRequest request) throws IOException; /** * Lists current set of RegionServer groups. diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminClient.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminClient.java index 08357f64bc9b..44d9e90dacb9 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminClient.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminClient.java @@ -32,13 +32,13 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.net.Address; import org.apache.hadoop.hbase.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos; import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos.NameStringPair; import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.AddRSGroupRequest; -import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.BalanceRSGroupRequest; import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.GetRSGroupInfoOfServerRequest; import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.GetRSGroupInfoOfServerResponse; import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.GetRSGroupInfoOfTableRequest; @@ -158,11 +158,10 @@ public void removeRSGroup(String name) throws IOException { } @Override - public boolean balanceRSGroup(String groupName) throws IOException { - BalanceRSGroupRequest request = BalanceRSGroupRequest.newBuilder() - .setRSGroupName(groupName).build(); + public boolean balanceRSGroup(String groupName, BalanceRequest request) throws IOException { try { - return stub.balanceRSGroup(null, request).getBalanceRan(); + return stub.balanceRSGroup(null, + RSGroupProtobufUtil.createBalanceRSGroupRequest(groupName, request)).getBalanceRan(); } catch (ServiceException e) { throw ProtobufUtil.handleRemoteException(e); } diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java index bf26de170626..a71b3c2b686b 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java @@ -39,6 +39,7 @@ import org.apache.hadoop.hbase.PleaseHoldException; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.SnapshotDescription; import org.apache.hadoop.hbase.client.TableDescriptor; @@ -291,18 +292,21 @@ public void removeRSGroup(RpcController controller, @Override public void balanceRSGroup(RpcController controller, BalanceRSGroupRequest request, RpcCallback done) { + BalanceRequest balanceRequest = RSGroupProtobufUtil.toBalanceRequest(request); + BalanceRSGroupResponse.Builder builder = BalanceRSGroupResponse.newBuilder(); LOG.info(master.getClientIdAuditPrefix() + " balance rsgroup, group=" + request.getRSGroupName()); try { if (master.getMasterCoprocessorHost() != null) { - master.getMasterCoprocessorHost().preBalanceRSGroup(request.getRSGroupName()); + master.getMasterCoprocessorHost().preBalanceRSGroup(request.getRSGroupName(), balanceRequest); } checkPermission("balanceRSGroup"); - boolean balancerRan = groupAdminServer.balanceRSGroup(request.getRSGroupName()); + boolean balancerRan = groupAdminServer.balanceRSGroup(request.getRSGroupName(), balanceRequest); builder.setBalanceRan(balancerRan); if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().postBalanceRSGroup(request.getRSGroupName(), + balanceRequest, balancerRan); } } catch (IOException e) { diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java index 744749f41c0a..b27ed0faa7a6 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java @@ -33,6 +33,7 @@ import org.apache.hadoop.hbase.NamespaceDescriptor; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; @@ -508,28 +509,30 @@ public void removeRSGroup(String name) throws IOException { } @Override - public boolean balanceRSGroup(String groupName) throws IOException { + public boolean balanceRSGroup(String groupName, BalanceRequest request) throws IOException { ServerManager serverManager = master.getServerManager(); LoadBalancer balancer = master.getLoadBalancer(); synchronized (balancer) { // If balance not true, don't run balancer. - if (!((HMaster) master).isBalancerOn()) { + if (!((HMaster) master).isBalancerOn() && !request.isDryRun()) { return false; } if (getRSGroupInfo(groupName) == null) { throw new ConstraintException("RSGroup does not exist: "+groupName); } + // Only allow one balance run at at time. Map groupRIT = rsGroupGetRegionsInTransition(groupName); - if (groupRIT.size() > 0) { + if (groupRIT.size() > 0 && !request.isIgnoreRegionsInTransition()) { LOG.debug("Not running balancer because {} region(s) in transition: {}", groupRIT.size(), StringUtils.abbreviate( master.getAssignmentManager().getRegionStates().getRegionsInTransition().toString(), 256)); return false; } + if (serverManager.areDeadServersInProgress()) { LOG.debug("Not running balancer because processing dead regionserver(s): {}", serverManager.getDeadServers()); @@ -541,6 +544,11 @@ public boolean balanceRSGroup(String groupName) throws IOException { getRSGroupAssignmentsByTable(master.getTableStateManager(), groupName); List plans = balancer.balanceCluster(assignmentsByTable); boolean balancerRan = !plans.isEmpty(); + + if (request.isDryRun()) { + return balancerRan; + } + if (balancerRan) { LOG.info("RSGroup balance {} starting with plan count: {}", groupName, plans.size()); master.executeRegionPlansWithThrottling(plans); diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupProtobufUtil.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupProtobufUtil.java index 9f092c52f2c0..6cb441ac4fc7 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupProtobufUtil.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupProtobufUtil.java @@ -23,10 +23,12 @@ import java.util.stream.Collectors; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.net.Address; import org.apache.hadoop.hbase.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos; import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos.NameStringPair; +import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.BalanceRSGroupRequest; import org.apache.hadoop.hbase.protobuf.generated.RSGroupProtos; import org.apache.hadoop.hbase.protobuf.generated.TableProtos; import org.apache.yetus.audience.InterfaceAudience; @@ -36,6 +38,21 @@ final class RSGroupProtobufUtil { private RSGroupProtobufUtil() { } + static BalanceRSGroupRequest createBalanceRSGroupRequest(String groupName, BalanceRequest request) { + return BalanceRSGroupRequest.newBuilder() + .setRSGroupName(groupName) + .setDryRun(request.isDryRun()) + .setIgnoreRit(request.isIgnoreRegionsInTransition()) + .build(); + } + + static BalanceRequest toBalanceRequest(BalanceRSGroupRequest request) { + return BalanceRequest.newBuilder() + .setDryRun(request.hasDryRun() && request.getDryRun()) + .setIgnoreRegionsInTransition(request.hasIgnoreRit() && request.getIgnoreRit()) + .build(); + } + static RSGroupInfo toGroupInfo(RSGroupProtos.RSGroupInfo proto) { RSGroupInfo rsGroupInfo = new RSGroupInfo(proto.getName()); for(HBaseProtos.ServerName el: proto.getServersList()) { diff --git a/hbase-rsgroup/src/main/protobuf/RSGroupAdmin.proto b/hbase-rsgroup/src/main/protobuf/RSGroupAdmin.proto index 54add90eff44..a3f8bf22edc0 100644 --- a/hbase-rsgroup/src/main/protobuf/RSGroupAdmin.proto +++ b/hbase-rsgroup/src/main/protobuf/RSGroupAdmin.proto @@ -86,6 +86,8 @@ message RemoveRSGroupResponse { message BalanceRSGroupRequest { required string r_s_group_name = 1; + optional bool ignore_rit = 2; + optional bool dry_run = 3; } message BalanceRSGroupResponse { diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBalance.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBalance.java index e419ee0b8edc..d3bebe0690f7 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBalance.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBalance.java @@ -30,6 +30,7 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.Waiter; import org.apache.hadoop.hbase.Waiter.Predicate; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.TableDescriptor; @@ -80,11 +81,61 @@ public void afterMethod() throws Exception { @Test public void testGroupBalance() throws Exception { - LOG.info(name.getMethodName()); - String newGroupName = getGroupName(name.getMethodName()); + String methodName = name.getMethodName(); + + LOG.info(methodName); + String newGroupName = getGroupName(methodName); + TableName tableName = TableName.valueOf(tablePrefix + "_ns", methodName); + + ServerName first = setupBalanceTest(newGroupName, tableName); + + // balance the other group and make sure it doesn't affect the new group + admin.balancerSwitch(true, true); + rsGroupAdmin.balanceRSGroup(RSGroupInfo.DEFAULT_GROUP); + assertEquals(6, getTableServerRegionMap().get(tableName).get(first).size()); + + // disable balance, balancer will not be run and return false + admin.balancerSwitch(false, true); + assertFalse(rsGroupAdmin.balanceRSGroup(newGroupName)); + assertEquals(6, getTableServerRegionMap().get(tableName).get(first).size()); + + // enable balance + admin.balancerSwitch(true, true); + rsGroupAdmin.balanceRSGroup(newGroupName); + TEST_UTIL.waitFor(WAIT_TIMEOUT, new Waiter.Predicate() { + @Override + public boolean evaluate() throws Exception { + for (List regions : getTableServerRegionMap().get(tableName).values()) { + if (2 != regions.size()) { + return false; + } + } + return true; + } + }); + admin.balancerSwitch(false, true); + } + + @Test + public void testGroupDryRunBalance() throws Exception { + String methodName = name.getMethodName(); + + LOG.info(methodName); + String newGroupName = getGroupName(methodName); + final TableName tableName = TableName.valueOf(tablePrefix + "_ns", methodName); + + ServerName first = setupBalanceTest(newGroupName, tableName); + + // run the balancer in dry run mode. it should return true, but should not actually move any regions + admin.balancerSwitch(true, true); + assertTrue(rsGroupAdmin.balanceRSGroup(newGroupName, BalanceRequest.newBuilder().setDryRun(true).build())); + // validate imbalance still exists. + assertEquals(6, getTableServerRegionMap().get(tableName).get(first).size()); + } + + private ServerName setupBalanceTest(String newGroupName, TableName tableName) throws Exception { addGroup(newGroupName, 3); - final TableName tableName = TableName.valueOf(tablePrefix + "_ns", name.getMethodName()); admin.createNamespace(NamespaceDescriptor.create(tableName.getNamespaceAsString()) .addConfiguration(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP, newGroupName).build()); final TableDescriptor desc = TableDescriptorBuilder.newBuilder(tableName) @@ -126,31 +177,7 @@ public boolean evaluate() throws Exception { } }); - // balance the other group and make sure it doesn't affect the new group - admin.balancerSwitch(true, true); - rsGroupAdmin.balanceRSGroup(RSGroupInfo.DEFAULT_GROUP); - assertEquals(6, getTableServerRegionMap().get(tableName).get(first).size()); - - // disable balance, balancer will not be run and return false - admin.balancerSwitch(false, true); - assertFalse(rsGroupAdmin.balanceRSGroup(newGroupName)); - assertEquals(6, getTableServerRegionMap().get(tableName).get(first).size()); - - // enable balance - admin.balancerSwitch(true, true); - rsGroupAdmin.balanceRSGroup(newGroupName); - TEST_UTIL.waitFor(WAIT_TIMEOUT, new Waiter.Predicate() { - @Override - public boolean evaluate() throws Exception { - for (List regions : getTableServerRegionMap().get(tableName).values()) { - if (2 != regions.size()) { - return false; - } - } - return true; - } - }); - admin.balancerSwitch(false, true); + return first; } @Test diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java index c4ae7d448aba..e176bb7cb338 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java @@ -43,6 +43,7 @@ import org.apache.hadoop.hbase.Waiter; import org.apache.hadoop.hbase.client.AbstractTestUpdateConfiguration; import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; @@ -407,13 +408,13 @@ public void postMoveServers(final ObserverContext @Override public void preBalanceRSGroup(final ObserverContext ctx, - String groupName) throws IOException { + String groupName, BalanceRequest request) throws IOException { preBalanceRSGroupCalled = true; } @Override public void postBalanceRSGroup(final ObserverContext ctx, - String groupName, boolean balancerRan) throws IOException { + String groupName, BalanceRequest request, boolean balancerRan) throws IOException { postBalanceRSGroupCalled = true; } diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdminClient.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdminClient.java index 99d36ee7fe34..cbe1fcc30816 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdminClient.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdminClient.java @@ -25,6 +25,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.Scan; @@ -92,8 +93,8 @@ public void removeRSGroup(String name) throws IOException { } @Override - public boolean balanceRSGroup(String groupName) throws IOException { - return wrapped.balanceRSGroup(groupName); + public boolean balanceRSGroup(String groupName, BalanceRequest request) throws IOException { + return wrapped.balanceRSGroup(groupName, request); } @Override diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java index 7f09fb0e63d7..d25ea89cef61 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java @@ -28,6 +28,7 @@ import org.apache.hadoop.hbase.NamespaceDescriptor; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.client.MasterSwitchType; import org.apache.hadoop.hbase.client.Mutation; import org.apache.hadoop.hbase.client.RegionInfo; @@ -566,18 +567,20 @@ default void postRegionOffline(final ObserverContext ctx) + default void preBalance(final ObserverContext ctx, BalanceRequest request) throws IOException {} /** * Called after the balancing plan has been submitted. * @param ctx the environment to interact with the framework and master + * @param request the request used to trigger the balance * @param plans the RegionPlans which master has executed. RegionPlan serves as hint * as for the final destination for the underlying region but may not represent the * final state of assignment */ - default void postBalance(final ObserverContext ctx, List plans) + default void postBalance(final ObserverContext ctx, BalanceRequest request, List plans) throws IOException {} /** @@ -1275,7 +1278,8 @@ default void postRemoveRSGroup(final ObserverContext ctx, - String groupName) throws IOException {} + String groupName, BalanceRequest request) throws IOException { + } /** * Called after a region server group is removed @@ -1283,7 +1287,8 @@ default void preBalanceRSGroup(final ObserverContext ctx, - String groupName, boolean balancerRan) throws IOException {} + String groupName, BalanceRequest request, boolean balancerRan) throws IOException { + } /** * Called before servers are removed from rsgroup 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 index 94eba24ba121..ee2cf3053bb9 100644 --- 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 @@ -61,6 +61,7 @@ import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CellBuilderFactory; import org.apache.hadoop.hbase.CellBuilderType; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.ClusterId; import org.apache.hadoop.hbase.ClusterMetrics; import org.apache.hadoop.hbase.ClusterMetrics.Option; @@ -1772,7 +1773,7 @@ private void balanceThrottling(long nextBalanceStartTime, int maxRegionsInTransi } public boolean balance() throws IOException { - return balance(false); + return balance(BalanceRequest.defaultInstance()); } /** @@ -1798,10 +1799,12 @@ public boolean skipRegionManagementAction(final String action) { return false; } - public boolean balance(boolean force) throws IOException { - if (loadBalancerTracker == null || !loadBalancerTracker.isBalancerOn()) { + public boolean balance(BalanceRequest request) throws IOException { + if (loadBalancerTracker == null + || !(loadBalancerTracker.isBalancerOn() || request.isDryRun())) { return false; } + if (skipRegionManagementAction("balancer")) { return false; } @@ -1820,10 +1823,11 @@ 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 + - ") because " + regionsInTransition.size() + " region(s) in transition: " + toPrint + - (truncated? "(truncated list)": "")); + + if (!request.isIgnoreRegionsInTransition() || metaInTransition) { + LOG.info("Not running balancer (ignoreRIT=false" + ", metaRIT=" + metaInTransition + + ") because " + regionsInTransition.size() + " region(s) in transition: " + toPrint + + (truncated? "(truncated list)": "")); return false; } } @@ -1835,7 +1839,7 @@ public boolean balance(boolean force) throws IOException { if (this.cpHost != null) { try { - if (this.cpHost.preBalance()) { + if (this.cpHost.preBalance(request)) { LOG.debug("Coprocessor bypassing balancer request"); return false; } @@ -1857,16 +1861,22 @@ public boolean balance(boolean force) throws IOException { List plans = this.balancer.balanceCluster(assignments); + // make one last check that the cluster isn't shutting down before proceeding. if (skipRegionManagementAction("balancer")) { - // make one last check that the cluster isn't shutting down before proceeding. - return false; + // return isDryRun because in this case the dry run did run, and the user could see + // the results in the master logs. + return request.isDryRun(); } - List sucRPs = executeRegionPlansWithThrottling(plans); + // For dry run we don't actually want to execute the moves, but we do want + // to execute the coprocessor below + List sucRPs = request.isDryRun() + ? Collections.emptyList() + : executeRegionPlansWithThrottling(plans); if (this.cpHost != null) { try { - this.cpHost.postBalance(sucRPs); + this.cpHost.postBalance(request, sucRPs); } catch (IOException ioe) { // balancing already succeeded so don't change the result LOG.error("Error invoking master coprocessor postBalance()", ioe); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java index 3170bfc27da4..3108ec429222 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java @@ -31,6 +31,7 @@ import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.SharedConnection; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.MasterSwitchType; import org.apache.hadoop.hbase.client.Mutation; @@ -737,20 +738,20 @@ public void call(MasterObserver observer) throws IOException { }); } - public boolean preBalance() throws IOException { + public boolean preBalance(final BalanceRequest request) throws IOException { return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { - observer.preBalance(this); + observer.preBalance(this, request); } }); } - public void postBalance(final List plans) throws IOException { + public void postBalance(final BalanceRequest request, final List plans) throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { - observer.postBalance(this, plans); + observer.postBalance(this, request, plans); } }); } @@ -1433,22 +1434,22 @@ public void call(MasterObserver observer) throws IOException { }); } - public void preBalanceRSGroup(final String name) + public void preBalanceRSGroup(final String name, final BalanceRequest request) throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { - observer.preBalanceRSGroup(this, name); + observer.preBalanceRSGroup(this, name, request); } }); } - public void postBalanceRSGroup(final String name, final boolean balanceRan) + public void postBalanceRSGroup(final String name, final BalanceRequest request, final boolean balanceRan) throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { - observer.postBalanceRSGroup(this, name, balanceRan); + observer.postBalanceRSGroup(this, name, request, balanceRan); } }); } 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 index 85f88c640b1b..58efb65546f7 100644 --- 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 @@ -663,8 +663,9 @@ public AssignRegionResponse assignRegion(RpcController controller, public BalanceResponse balance(RpcController controller, BalanceRequest request) throws ServiceException { try { - return BalanceResponse.newBuilder().setBalancerRan(master.balance( - request.hasForce()? request.getForce(): false)).build(); + return BalanceResponse.newBuilder() + .setBalancerRan(master.balance(ProtobufUtil.toBalanceRequest(request))) + .build(); } catch (IOException ex) { throw new ServiceException(ex); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java index 0db2b32c61f9..e8092992bac1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java @@ -58,6 +58,7 @@ import org.apache.hadoop.hbase.Tag; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.Append; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; import org.apache.hadoop.hbase.client.Delete; @@ -1003,7 +1004,7 @@ public void preSetSplitOrMergeEnabled(final ObserverContext c) + public void preBalance(ObserverContext c, BalanceRequest request) throws IOException { requirePermission(c, "balance", Action.ADMIN); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java index 84c90de4234e..419b81245f13 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java @@ -824,7 +824,7 @@ public Optional getMasterObserver() { @Override public void postBalance(final ObserverContext ctx, - List plans) throws IOException { + BalanceRequest request, List plans) throws IOException { if (!plans.isEmpty()) { postBalanceCount.incrementAndGet(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java index 2cab41b6818d..300af75ab80e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java @@ -40,6 +40,7 @@ import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.MasterSwitchType; @@ -689,13 +690,14 @@ public boolean preRegionOfflineCalledOnly() { } @Override - public void preBalance(ObserverContext env) - throws IOException { + public void preBalance(ObserverContext env, + BalanceRequest request) throws IOException { preBalanceCalled = true; } @Override public void postBalance(ObserverContext env, + BalanceRequest request, List plans) throws IOException { postBalanceCalled = true; } @@ -1159,12 +1161,12 @@ public void postRemoveRSGroup(ObserverContext ctx, @Override public void preBalanceRSGroup(ObserverContext ctx, - String groupName) throws IOException { + String groupName, BalanceRequest request) throws IOException { } @Override public void postBalanceRSGroup(ObserverContext ctx, - String groupName, boolean balancerRan) throws IOException { + String groupName, BalanceRequest request, boolean balancerRan) throws IOException { } @Override 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 new file mode 100644 index 000000000000..c8585e102f74 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterDryRunBalancer.java @@ -0,0 +1,115 @@ +/** + * 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. + */ +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.client.BalanceRequest; +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.Waiter; +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(BalanceRequest.newBuilder().setDryRun(true).build())); + + // 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); + TEST_UTIL.createMultiRegionTable(tableName, FAMILYNAME, 100); + return tableName; + } + + + private HRegionServer unbalance(HMaster master, TableName tableName) throws Exception { + waitForRegionsToSettle(master); + + HRegionServer biasedServer = TEST_UTIL.getMiniHBaseCluster().getRegionServer(0); + + for (RegionInfo regionInfo : TEST_UTIL.getAdmin().getRegions(tableName)) { + master.move(regionInfo.getEncodedNameAsBytes(), + Bytes.toBytes(biasedServer.getServerName().getServerName())); + } + + waitForRegionsToSettle(master); + + 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()); + } + + private void waitForRegionsToSettle(HMaster master) { + Waiter.waitFor(TEST_UTIL.getConfiguration(), 60_000, + () -> master.getAssignmentManager().getRegionStates().getRegionsInTransitionCount() <= 0); + } +} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java index fd8591e2a75a..e54e8d6acbe7 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java @@ -64,6 +64,7 @@ import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.Append; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.Delete; @@ -776,7 +777,7 @@ public void testBalance() throws Exception { AccessTestAction action = new AccessTestAction() { @Override public Object run() throws Exception { - ACCESS_CONTROLLER.preBalance(ObserverContextImpl.createAndPrepare(CP_ENV)); + ACCESS_CONTROLLER.preBalance(ObserverContextImpl.createAndPrepare(CP_ENV), BalanceRequest.defaultInstance()); return null; } }; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java index 26507025a3fe..fb1a060eccc6 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java @@ -36,6 +36,7 @@ import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.Append; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.Delete; @@ -568,7 +569,7 @@ public Object run() throws Exception { verifyAllowed(new AccessTestAction() { @Override public Object run() throws Exception { - ACCESS_CONTROLLER.preBalance(ObserverContextImpl.createAndPrepare(CP_ENV)); + ACCESS_CONTROLLER.preBalance(ObserverContextImpl.createAndPrepare(CP_ENV), BalanceRequest.defaultInstance()); return null; } }, SUPERUSER, USER_ADMIN, USER_RW, USER_RO, USER_OWNER, USER_CREATE, USER_QUAL, USER_NONE); diff --git a/hbase-shell/src/main/ruby/hbase/admin.rb b/hbase-shell/src/main/ruby/hbase/admin.rb index f8ebe5ed95c5..c1da4bbb52b8 100644 --- a/hbase-shell/src/main/ruby/hbase/admin.rb +++ b/hbase-shell/src/main/ruby/hbase/admin.rb @@ -26,6 +26,8 @@ java_import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder java_import org.apache.hadoop.hbase.HConstants +require 'hbase/balancer_utils' + # Wrapper for org.apache.hadoop.hbase.client.HBaseAdmin module Hbase @@ -206,8 +208,9 @@ def locate_region(table_name, row_key) #---------------------------------------------------------------------------------------------- # Requests a cluster balance # Returns true if balancer ran - def balancer(force) - @admin.balancer(java.lang.Boolean.valueOf(force)) + def balancer(*args) + request = ::Hbase::BalancerUtils.create_balance_request(args) + @admin.balance(request) end #---------------------------------------------------------------------------------------------- diff --git a/hbase-shell/src/main/ruby/hbase/balancer_utils.rb b/hbase-shell/src/main/ruby/hbase/balancer_utils.rb new file mode 100644 index 000000000000..9948c0f4745c --- /dev/null +++ b/hbase-shell/src/main/ruby/hbase/balancer_utils.rb @@ -0,0 +1,57 @@ +# +# +# 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. +# + +include Java + +java_import org.apache.hadoop.hbase.client.BalanceRequest + +module Hbase + class BalancerUtils + def self.create_balance_request(args) + args = args.first if args.first.is_a?(Array) and args.size == 1 + if args.nil? or args.empty? + return BalanceRequest.defaultInstance() + elsif args.size > 2 + raise ArgumentError, "Illegal arguments #{args}. Expected between 0 and 2 arguments, but got #{args.size}." + end + + builder = BalanceRequest.newBuilder() + + index = 0 + args.each do |arg| + if !arg.is_a?(String) + raise ArgumentError, "Illegal argument in index #{index}: #{arg}. All arguments must be strings, but got #{arg.class}." + end + + case arg + when 'force', 'ignore_rit' + builder.setIgnoreRegionsInTransition(true) + when 'dry_run' + builder.setDryRun(true) + else + raise ArgumentError, "Illegal argument in index #{index}: #{arg}. Unknown option #{arg}, expected 'force', 'ignore_rit', or 'dry_run'." + end + + index += 1 + end + + return builder.build() + end + end +end diff --git a/hbase-shell/src/main/ruby/hbase/rsgroup_admin.rb b/hbase-shell/src/main/ruby/hbase/rsgroup_admin.rb index b25219ad399c..6d08dbcfab58 100644 --- a/hbase-shell/src/main/ruby/hbase/rsgroup_admin.rb +++ b/hbase-shell/src/main/ruby/hbase/rsgroup_admin.rb @@ -18,6 +18,8 @@ include Java java_import org.apache.hadoop.hbase.util.Pair +require 'hbase/balancer_utils' + # Wrapper for org.apache.hadoop.hbase.group.GroupAdminClient # Which is an API to manage region server groups @@ -61,8 +63,9 @@ def remove_rs_group(group_name) #-------------------------------------------------------------------------- # balance a group - def balance_rs_group(group_name) - @admin.balanceRSGroup(group_name) + def balance_rs_group(group_name, *args) + request = ::Hbase::BalancerUtils.create_balance_request(args) + @admin.balanceRSGroup(group_name, request) end #-------------------------------------------------------------------------- diff --git a/hbase-shell/src/main/ruby/shell/commands/balance_rsgroup.rb b/hbase-shell/src/main/ruby/shell/commands/balance_rsgroup.rb index a223e055bbfb..41eb10eb3b18 100644 --- a/hbase-shell/src/main/ruby/shell/commands/balance_rsgroup.rb +++ b/hbase-shell/src/main/ruby/shell/commands/balance_rsgroup.rb @@ -22,16 +22,25 @@ def help <<-EOF Balance a RegionServer group +Parameter can be "force" or "dry_run": + - "dry_run" will run the balancer to generate a plan, but will not actually execute that plan. + This is useful for testing out new balance configurations. See the active HMaster logs for the results of the dry_run. + - "ignore_rit" tells master whether we should force the balancer to run even if there is region in transition. + WARNING: For experts only. Forcing a balance may do more damage than repair when assignment is confused + Example: hbase> balance_rsgroup 'my_group' + hbase> balance_rsgroup 'my_group', 'ignore_rit' + hbase> balance_rsgroup 'my_group', 'dry_run' + hbase> balance_rsgroup 'my_group', 'dry_run', 'ignore_rit' EOF end - def command(group_name) + def command(group_name, *args) # Returns true if balancer was run, otherwise false. - ret = rsgroup_admin.balance_rs_group(group_name) + ret = rsgroup_admin.balance_rs_group(group_name, args) if ret puts 'Ran the balancer.' else diff --git a/hbase-shell/src/main/ruby/shell/commands/balancer.rb b/hbase-shell/src/main/ruby/shell/commands/balancer.rb index c4acdc0ecdeb..7a1f63cf3877 100644 --- a/hbase-shell/src/main/ruby/shell/commands/balancer.rb +++ b/hbase-shell/src/main/ruby/shell/commands/balancer.rb @@ -22,29 +22,25 @@ module Commands class Balancer < Command def help <<-EOF -Trigger the cluster balancer. Returns true if balancer ran and was able to -tell the region servers to unassign all the regions to balance (the re-assignment itself is async). -Otherwise false (Will not run if regions in transition). -Parameter tells master whether we should force balance even if there is region in transition. +Trigger the cluster balancer. Returns true if balancer ran, otherwise false (Will not run if regions in transition). -WARNING: For experts only. Forcing a balance may do more damage than repair -when assignment is confused +Parameter can be "force" or "dry_run": + - "dry_run" will run the balancer to generate a plan, but will not actually execute that plan. + This is useful for testing out new balance configurations. See the active HMaster logs for the results of the dry_run. + - "ignore_rit" tells master whether we should force the balancer to run even if there is region in transition. + WARNING: For experts only. Forcing a balance may do more damage than repair when assignment is confused Examples: hbase> balancer - hbase> balancer "force" + hbase> balancer "ignore_rit" + hbase> balancer "dry_run" + hbase> balancer "dry_run", "ignore_rit" EOF end - def command(force = nil) - force_balancer = 'false' - if force == 'force' - force_balancer = 'true' - elsif !force.nil? - raise ArgumentError, "Invalid argument #{force}." - end - did_balancer_run = !!admin.balancer(force_balancer) + def command(*args) + did_balancer_run = !!admin.balancer(args) formatter.row([did_balancer_run.to_s]) did_balancer_run end diff --git a/hbase-shell/src/test/ruby/hbase/admin_test.rb b/hbase-shell/src/test/ruby/hbase/admin_test.rb index 19910cac8d98..8dd69202a46a 100644 --- a/hbase-shell/src/test/ruby/hbase/admin_test.rb +++ b/hbase-shell/src/test/ruby/hbase/admin_test.rb @@ -193,6 +193,10 @@ def teardown assert(did_balancer_run == true) output = capture_stdout { command(:balancer, 'force') } assert(output.include?('true')) + + command(:balance_switch, false) + output = capture_stdout { command(:balancer, 'dry_run') } + assert(output.include?('true')) end #------------------------------------------------------------------------------- diff --git a/hbase-shell/src/test/ruby/hbase/balancer_utils_test.rb b/hbase-shell/src/test/ruby/hbase/balancer_utils_test.rb new file mode 100644 index 000000000000..9d70540750e5 --- /dev/null +++ b/hbase-shell/src/test/ruby/hbase/balancer_utils_test.rb @@ -0,0 +1,78 @@ +# +# +# 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. +# + +java_import org.apache.hadoop.hbase.client.BalanceRequest + +module Hbase + class BalancerUtilsTest < Test::Unit::TestCase + include TestHelpers + + def create_balance_request(*args) + ::Hbase::BalancerUtils.create_balance_request(args) + end + + define_test "should raise ArgumentError on unknown string argument" do + assert_raise(ArgumentError) do + request = create_balance_request('foo') + end + end + + define_test "should raise ArgumentError on non-array argument" do + assert_raise(ArgumentError) do + request = create_balance_request({foo: 'bar' }) + end + end + + define_test "should raise ArgumentError on non-string array item" do + assert_raise(ArgumentError) do + request = create_balance_request('force', true) + end + end + + define_test "should parse empty args" do + request = create_balance_request() + assert(!request.isDryRun()) + assert(!request.isIgnoreRegionsInTransition()) + end + + define_test "should parse 'force' string" do + request = create_balance_request('force') + assert(!request.isDryRun()) + assert(request.isIgnoreRegionsInTransition()) + end + + define_test "should parse 'ignore_rit' string" do + request = create_balance_request('ignore_rit') + assert(!request.isDryRun()) + assert(request.isIgnoreRegionsInTransition()) + end + + define_test "should parse 'dry_run' string" do + request = create_balance_request('dry_run') + assert(request.isDryRun()) + assert(!request.isIgnoreRegionsInTransition()) + end + + define_test "should parse multiple string args" do + request = create_balance_request('dry_run', 'ignore_rit') + assert(request.isDryRun()) + assert(request.isIgnoreRegionsInTransition()) + end + end +end diff --git a/hbase-shell/src/test/ruby/shell/rsgroup_shell_test.rb b/hbase-shell/src/test/ruby/shell/rsgroup_shell_test.rb index f7c53000c850..b305c2e58d51 100644 --- a/hbase-shell/src/test/ruby/shell/rsgroup_shell_test.rb +++ b/hbase-shell/src/test/ruby/shell/rsgroup_shell_test.rb @@ -90,6 +90,8 @@ def remove_rsgroup(group_name) # just run it to verify jruby->java api binding @hbase.rsgroup_admin.balance_rs_group(group_name) + @hbase.rsgroup_admin.balance_rs_group(group_name, 'force') + @hbase.rsgroup_admin.balance_rs_group(group_name, 'dry_run') @shell.command(:disable, table_name) @shell.command(:drop, table_name) 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 index fd551b720ffe..5ab1c1f6f276 100644 --- 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 @@ -27,6 +27,7 @@ import java.util.regex.Pattern; import org.apache.commons.lang3.NotImplementedException; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.CacheEvictionStats; import org.apache.hadoop.hbase.ClusterMetrics; import org.apache.hadoop.hbase.HConstants; @@ -786,6 +787,10 @@ public boolean balance(boolean force) { throw new NotImplementedException("balance not supported in ThriftAdmin"); } + @Override public boolean balance(BalanceRequest request) throws IOException { + throw new NotImplementedException("balance not supported in ThriftAdmin"); + } + @Override public boolean isBalancerEnabled() { throw new NotImplementedException("isBalancerEnabled not supported in ThriftAdmin"); From 279d2516d5b0e7e71cb5e5c9b3b9e864679129e2 Mon Sep 17 00:00:00 2001 From: Bryan Beaudreault Date: Tue, 10 Aug 2021 13:55:07 -0400 Subject: [PATCH 2/3] Add a new BalancerResponse object --- .../org/apache/hadoop/hbase/client/Admin.java | 9 +- .../hadoop/hbase/client/AsyncAdmin.java | 9 +- .../hadoop/hbase/client/AsyncHBaseAdmin.java | 2 +- .../hadoop/hbase/client/BalanceRequest.java | 2 +- .../hadoop/hbase/client/BalanceResponse.java | 100 ++++++++++++++++++ .../hadoop/hbase/client/HBaseAdmin.java | 57 +++++----- .../hbase/client/RawAsyncHBaseAdmin.java | 8 +- .../hbase/shaded/protobuf/ProtobufUtil.java | 17 +++ .../src/main/protobuf/Master.proto | 2 + .../hadoop/hbase/rsgroup/RSGroupAdmin.java | 5 +- .../hbase/rsgroup/RSGroupAdminClient.java | 9 +- .../hbase/rsgroup/RSGroupAdminEndpoint.java | 15 ++- .../hbase/rsgroup/RSGroupAdminServer.java | 23 ++-- .../hbase/rsgroup/RSGroupProtobufUtil.java | 18 +++- .../src/main/protobuf/RSGroupAdmin.proto | 2 + .../hbase/rsgroup/TestRSGroupsBalance.java | 11 +- .../hbase/rsgroup/TestRSGroupsBase.java | 3 +- .../hbase/rsgroup/TestRSGroupsFallback.java | 4 +- .../rsgroup/VerifyingRSGroupAdminClient.java | 3 +- .../hbase/coprocessor/MasterObserver.java | 5 +- .../apache/hadoop/hbase/master/HMaster.java | 32 +++--- .../hbase/master/MasterCoprocessorHost.java | 5 +- .../hbase/master/MasterRpcServices.java | 4 +- .../hadoop/hbase/TestRegionRebalancing.java | 27 ++++- .../TestAsyncTableGetMultiThreaded.java | 2 +- .../client/TestSeparateClientZKCluster.java | 4 +- .../hbase/coprocessor/TestMasterObserver.java | 5 +- .../master/TestMasterDryRunBalancer.java | 19 +++- .../procedure/TestProcedurePriority.java | 3 +- .../ruby/shell/commands/balance_rsgroup.rb | 11 +- .../src/main/ruby/shell/commands/balancer.rb | 11 +- hbase-shell/src/test/ruby/hbase/admin_test.rb | 6 +- .../hbase/thrift2/client/ThriftAdmin.java | 4 +- 33 files changed, 322 insertions(+), 115 deletions(-) create mode 100644 hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceResponse.java 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 index 31295dc79a7e..a3a51071b338 100644 --- 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 @@ -1252,7 +1252,8 @@ default boolean balancer() throws IOException { * @throws IOException if a remote or network exception occurs */ default boolean balance() throws IOException { - return balance(BalanceRequest.defaultInstance()); + return balance(BalanceRequest.defaultInstance()) + .isBalancerRan(); } /** @@ -1260,10 +1261,10 @@ default boolean balance() throws IOException { * balancer will run. See {@link BalanceRequest} for more details. * * @param request defines how the balancer should run - * @return true if balancer ran, false otherwise. + * @return {@link BalanceResponse} with details about the results of the invocation. * @throws IOException if a remote or network exception occurs */ - boolean balance(BalanceRequest request) throws IOException; + BalanceResponse balance(BalanceRequest request) throws IOException; /** * Invoke the balancer. Will run the balancer and if regions to move, it will @@ -1298,7 +1299,7 @@ default boolean balance(boolean force) throws IOException { BalanceRequest.newBuilder() .setIgnoreRegionsInTransition(force) .build() - ); + ).isBalancerRan(); } /** 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 index 38683cdc6dc6..85d545505e99 100644 --- 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 @@ -1257,7 +1257,8 @@ default CompletableFuture balancerSwitch(boolean on) { * {@link CompletableFuture}. */ default CompletableFuture balance() { - return balance(BalanceRequest.defaultInstance()); + return balance(BalanceRequest.defaultInstance()) + .thenApply(BalanceResponse::isBalancerRan); } /** @@ -1275,7 +1276,7 @@ default CompletableFuture balance(boolean forcible) { BalanceRequest.newBuilder() .setIgnoreRegionsInTransition(forcible) .build() - ); + ).thenApply(BalanceResponse::isBalancerRan); } /** @@ -1283,9 +1284,9 @@ default CompletableFuture balance(boolean forcible) { * balancer will run. See {@link BalanceRequest} for more details. * * @param request defines how the balancer should run - * @return true if balancer ran, false otherwise. + * @return {@link BalanceResponse} with details about the results of the invocation. */ - CompletableFuture balance(BalanceRequest request); + CompletableFuture balance(BalanceRequest request); /** * Query the current state of the balancer. 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 index adf1227fa32c..db720f3f68ed 100644 --- 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 @@ -684,7 +684,7 @@ public CompletableFuture balancerSwitch(boolean on, boolean drainRITs) } @Override - public CompletableFuture balance(BalanceRequest request) { + public CompletableFuture balance(BalanceRequest request) { return wrap(rawAdmin.balance(request)); } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceRequest.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceRequest.java index fe1b1180c0b0..d5c8f2bdcfb2 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceRequest.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceRequest.java @@ -22,7 +22,7 @@ import org.apache.yetus.audience.InterfaceStability; /** - * Encapsulates options for executing an unscheduled run of the Balancer. + * Encapsulates options for executing a run of the Balancer. */ @InterfaceAudience.Public @InterfaceStability.Evolving diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceResponse.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceResponse.java new file mode 100644 index 000000000000..6b02b21a2eeb --- /dev/null +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceResponse.java @@ -0,0 +1,100 @@ +/* + * + * 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. + */ +package org.apache.hadoop.hbase.client; + +import org.apache.yetus.audience.InterfaceAudience; +import org.apache.yetus.audience.InterfaceStability; + +/** + * Response returned from a balancer invocation + */ +@InterfaceAudience.Public +@InterfaceStability.Evolving +public final class BalanceResponse { + + @InterfaceAudience.Public + @InterfaceStability.Evolving + public final static class Builder { + private boolean balancerRan; + private int movesCalculated; + private int movesExecuted; + + private Builder() {} + + public Builder setBalancerRan(boolean balancerRan) { + this.balancerRan = balancerRan; + return this; + } + + public Builder setMovesCalculated(int movesCalculated) { + this.movesCalculated = movesCalculated; + return this; + } + + public Builder setMovesExecuted(int movesExecuted) { + this.movesExecuted = movesExecuted; + return this; + } + + public BalanceResponse build() { + return new BalanceResponse(balancerRan, movesCalculated, movesExecuted); + } + } + + public static Builder newBuilder() { + return new Builder(); + } + + private final boolean balancerRan; + private final int movesCalculated; + private final int movesExecuted; + + private BalanceResponse(boolean balancerRan, int movesCalculated, int movesExecuted) { + this.balancerRan = balancerRan; + this.movesCalculated = movesCalculated; + this.movesExecuted = movesExecuted; + } + + /** + * Determines whether the balancer ran or not. The balancer may not run for a variety of reasons, + * such as: another balance is running, there are regions in transition, the cluster is in + * maintenance mode, etc. + */ + public boolean isBalancerRan() { + return balancerRan; + } + + /** + * The number of moves calculated by the balancer if it ran. This may be zero if + * no better balance could be found. + */ + public int getMovesCalculated() { + return movesCalculated; + } + + /** + * The number of moves actually executed by the balancer if it ran. This will be + * zero if {@link #getMovesCalculated()} is zero or if {@link BalanceRequest#isDryRun()} + * was true. It may also not be equal to {@link #getMovesCalculated()} if the balancer + * was interrupted midway through executing the moves due to max run time. + */ + public int getMovesExecuted() { + return movesExecuted; + } +} 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 index d6be0bc351cd..0704d518d076 100644 --- 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 @@ -92,6 +92,25 @@ import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.ShadedAccessControlUtil; import org.apache.hadoop.hbase.security.access.UserPermission; +import org.apache.hadoop.hbase.snapshot.ClientSnapshotDescriptionUtils; +import org.apache.hadoop.hbase.snapshot.HBaseSnapshotException; +import org.apache.hadoop.hbase.snapshot.RestoreSnapshotException; +import org.apache.hadoop.hbase.snapshot.SnapshotCreationException; +import org.apache.hadoop.hbase.snapshot.UnknownSnapshotException; +import org.apache.hadoop.hbase.util.Addressing; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; +import org.apache.hadoop.hbase.util.ForeignExceptionUtil; +import org.apache.hadoop.hbase.util.Pair; +import org.apache.hadoop.ipc.RemoteException; +import org.apache.hadoop.util.StringUtils; +import org.apache.yetus.audience.InterfaceAudience; +import org.apache.yetus.audience.InterfaceStability; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.apache.hbase.thirdparty.com.google.common.base.Preconditions; +import org.apache.hbase.thirdparty.com.google.protobuf.ServiceException; +import org.apache.hbase.thirdparty.org.apache.commons.collections4.CollectionUtils; import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.shaded.protobuf.RequestConverter; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos; @@ -163,8 +182,7 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.IsProcedureDoneRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.IsProcedureDoneResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.IsRpcThrottleEnabledRequest; -import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos - .IsSnapshotCleanupEnabledRequest; +import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.IsSnapshotCleanupEnabledRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.IsSnapshotDoneRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.IsSnapshotDoneResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.ListDecommissionedRegionServersRequest; @@ -211,25 +229,6 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.ReplicationProtos.RemoveReplicationPeerResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.ReplicationProtos.UpdateReplicationPeerConfigResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.SnapshotProtos; -import org.apache.hadoop.hbase.snapshot.ClientSnapshotDescriptionUtils; -import org.apache.hadoop.hbase.snapshot.HBaseSnapshotException; -import org.apache.hadoop.hbase.snapshot.RestoreSnapshotException; -import org.apache.hadoop.hbase.snapshot.SnapshotCreationException; -import org.apache.hadoop.hbase.snapshot.UnknownSnapshotException; -import org.apache.hadoop.hbase.util.Addressing; -import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; -import org.apache.hadoop.hbase.util.ForeignExceptionUtil; -import org.apache.hadoop.hbase.util.Pair; -import org.apache.hadoop.ipc.RemoteException; -import org.apache.hadoop.util.StringUtils; -import org.apache.hbase.thirdparty.com.google.common.base.Preconditions; -import org.apache.hbase.thirdparty.com.google.protobuf.ServiceException; -import org.apache.hbase.thirdparty.org.apache.commons.collections4.CollectionUtils; -import org.apache.yetus.audience.InterfaceAudience; -import org.apache.yetus.audience.InterfaceStability; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * HBaseAdmin is no longer a client API. It is marked InterfaceAudience.Private indicating that @@ -1476,14 +1475,14 @@ protected Boolean rpcCall() throws Exception { }); } - @Override - public boolean balance(BalanceRequest request) throws IOException { - return executeCallable(new MasterCallable(getConnection(), getRpcControllerFactory()) { - @Override protected Boolean rpcCall() throws Exception { - return master.balance(getRpcController(), ProtobufUtil.toBalanceRequest(request)) - .getBalancerRan(); - } - }); + @Override public BalanceResponse balance(BalanceRequest request) throws IOException { + return executeCallable( + new MasterCallable(getConnection(), getRpcControllerFactory()) { + @Override protected BalanceResponse rpcCall() throws Exception { + MasterProtos.BalanceRequest req = ProtobufUtil.toBalanceRequest(request); + return ProtobufUtil.toBalanceResponse(master.balance(getRpcController(), req)); + } + }); } @Override 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 index 286d891ebc4f..a0e5320f855c 100644 --- 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 @@ -3209,13 +3209,13 @@ public CompletableFuture balancerSwitch(boolean on, boolean drainRITs) } @Override - public CompletableFuture balance(BalanceRequest request) { + public CompletableFuture balance(BalanceRequest request) { return this - . newMasterCaller() + . newMasterCaller() .action( - (controller, stub) -> this. call(controller, + (controller, stub) -> this. call(controller, stub, ProtobufUtil.toBalanceRequest(request), - (s, c, req, done) -> s.balance(c, req, done), (resp) -> resp.getBalancerRan())).call(); + (s, c, req, done) -> s.balance(c, req, done), (resp) -> ProtobufUtil.toBalanceResponse(resp))).call(); } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java index 7e02a0e8fb96..2297732eae04 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java @@ -68,6 +68,7 @@ import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.Append; +import org.apache.hadoop.hbase.client.BalanceResponse; import org.apache.hadoop.hbase.client.BalancerRejection; import org.apache.hadoop.hbase.client.BalancerDecision; import org.apache.hadoop.hbase.client.CheckAndMutate; @@ -3768,4 +3769,20 @@ public static BalanceRequest toBalanceRequest(MasterProtos.BalanceRequest reques .build(); } + public static MasterProtos.BalanceResponse toBalanceResponse(BalanceResponse response) { + return MasterProtos.BalanceResponse.newBuilder() + .setBalancerRan(response.isBalancerRan()) + .setMovesCalculated(response.getMovesCalculated()) + .setMovesExecuted(response.getMovesExecuted()) + .build(); + } + + public static BalanceResponse toBalanceResponse(MasterProtos.BalanceResponse response) { + return BalanceResponse.newBuilder() + .setBalancerRan(response.hasBalancerRan() && response.getBalancerRan()) + .setMovesCalculated(response.hasMovesCalculated() ? response.getMovesExecuted() : 0) + .setMovesExecuted(response.hasMovesExecuted() ? response.getMovesExecuted() : 0) + .build(); + } + } diff --git a/hbase-protocol-shaded/src/main/protobuf/Master.proto b/hbase-protocol-shaded/src/main/protobuf/Master.proto index ad5971678c53..4a6bb3959532 100644 --- a/hbase-protocol-shaded/src/main/protobuf/Master.proto +++ b/hbase-protocol-shaded/src/main/protobuf/Master.proto @@ -298,6 +298,8 @@ message BalanceRequest { message BalanceResponse { required bool balancer_ran = 1; + optional uint32 moves_calculated = 2; + optional uint32 moves_executed = 3; } message SetBalancerRunningRequest { diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdmin.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdmin.java index 6049e385199e..c1c591633da3 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdmin.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdmin.java @@ -24,6 +24,7 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.BalanceRequest; +import org.apache.hadoop.hbase.client.BalanceResponse; import org.apache.hadoop.hbase.net.Address; import org.apache.yetus.audience.InterfaceAudience; @@ -68,7 +69,7 @@ public interface RSGroupAdmin { * * @return boolean Whether balance ran or not */ - default boolean balanceRSGroup(String groupName) throws IOException { + default BalanceResponse balanceRSGroup(String groupName) throws IOException { return balanceRSGroup(groupName, BalanceRequest.defaultInstance()); } @@ -78,7 +79,7 @@ default boolean balanceRSGroup(String groupName) throws IOException { * * @return boolean Whether balance ran or not */ - boolean balanceRSGroup(String groupName, BalanceRequest request) throws IOException; + BalanceResponse balanceRSGroup(String groupName, BalanceRequest request) throws IOException; /** * Lists current set of RegionServer groups. diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminClient.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminClient.java index 44d9e90dacb9..9e5c167cbee1 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminClient.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminClient.java @@ -33,11 +33,13 @@ import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.BalanceRequest; +import org.apache.hadoop.hbase.client.BalanceResponse; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.net.Address; import org.apache.hadoop.hbase.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos; import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos.NameStringPair; +import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos; import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.AddRSGroupRequest; import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.GetRSGroupInfoOfServerRequest; import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.GetRSGroupInfoOfServerResponse; @@ -158,10 +160,11 @@ public void removeRSGroup(String name) throws IOException { } @Override - public boolean balanceRSGroup(String groupName, BalanceRequest request) throws IOException { + public BalanceResponse balanceRSGroup(String groupName, BalanceRequest request) throws IOException { try { - return stub.balanceRSGroup(null, - RSGroupProtobufUtil.createBalanceRSGroupRequest(groupName, request)).getBalanceRan(); + RSGroupAdminProtos.BalanceRSGroupRequest req = + RSGroupProtobufUtil.createBalanceRSGroupRequest(groupName, request); + return RSGroupProtobufUtil.toBalanceResponse(stub.balanceRSGroup(null, req)); } catch (ServiceException e) { throw ProtobufUtil.handleRemoteException(e); } diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java index a71b3c2b686b..5488fa81d93e 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java @@ -40,6 +40,7 @@ import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.BalanceRequest; +import org.apache.hadoop.hbase.client.BalanceResponse; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.SnapshotDescription; import org.apache.hadoop.hbase.client.TableDescriptor; @@ -294,25 +295,29 @@ public void balanceRSGroup(RpcController controller, BalanceRSGroupRequest request, RpcCallback done) { BalanceRequest balanceRequest = RSGroupProtobufUtil.toBalanceRequest(request); - BalanceRSGroupResponse.Builder builder = BalanceRSGroupResponse.newBuilder(); + BalanceRSGroupResponse.Builder builder = BalanceRSGroupResponse.newBuilder() + .setBalanceRan(false); + LOG.info(master.getClientIdAuditPrefix() + " balance rsgroup, group=" + request.getRSGroupName()); try { if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().preBalanceRSGroup(request.getRSGroupName(), balanceRequest); } + checkPermission("balanceRSGroup"); - boolean balancerRan = groupAdminServer.balanceRSGroup(request.getRSGroupName(), balanceRequest); - builder.setBalanceRan(balancerRan); + BalanceResponse response = groupAdminServer.balanceRSGroup(request.getRSGroupName(), balanceRequest); + RSGroupProtobufUtil.populateBalanceRSGroupResponse(builder, response); + if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().postBalanceRSGroup(request.getRSGroupName(), balanceRequest, - balancerRan); + response); } } catch (IOException e) { CoprocessorRpcUtils.setControllerException(controller, e); - builder.setBalanceRan(false); } + done.run(builder.build()); } diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java index b27ed0faa7a6..d25cb5e77802 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java @@ -34,6 +34,7 @@ import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.BalanceRequest; +import org.apache.hadoop.hbase.client.BalanceResponse; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; @@ -509,14 +510,16 @@ public void removeRSGroup(String name) throws IOException { } @Override - public boolean balanceRSGroup(String groupName, BalanceRequest request) throws IOException { + public BalanceResponse balanceRSGroup(String groupName, BalanceRequest request) throws IOException { ServerManager serverManager = master.getServerManager(); LoadBalancer balancer = master.getLoadBalancer(); + BalanceResponse.Builder responseBuilder = BalanceResponse.newBuilder(); + synchronized (balancer) { // If balance not true, don't run balancer. if (!((HMaster) master).isBalancerOn() && !request.isDryRun()) { - return false; + return responseBuilder.build(); } if (getRSGroupInfo(groupName) == null) { @@ -530,13 +533,13 @@ public boolean balanceRSGroup(String groupName, BalanceRequest request) throws I StringUtils.abbreviate( master.getAssignmentManager().getRegionStates().getRegionsInTransition().toString(), 256)); - return false; + return responseBuilder.build(); } if (serverManager.areDeadServersInProgress()) { LOG.debug("Not running balancer because processing dead regionserver(s): {}", serverManager.getDeadServers()); - return false; + return responseBuilder.build(); } //We balance per group instead of per table @@ -545,16 +548,16 @@ public boolean balanceRSGroup(String groupName, BalanceRequest request) throws I List plans = balancer.balanceCluster(assignmentsByTable); boolean balancerRan = !plans.isEmpty(); - if (request.isDryRun()) { - return balancerRan; - } + responseBuilder.setBalancerRan(balancerRan).setMovesCalculated(plans.size()); - if (balancerRan) { + if (balancerRan && !request.isDryRun()) { LOG.info("RSGroup balance {} starting with plan count: {}", groupName, plans.size()); - master.executeRegionPlansWithThrottling(plans); + List executed = master.executeRegionPlansWithThrottling(plans); + responseBuilder.setMovesExecuted(executed.size()); LOG.info("RSGroup balance " + groupName + " completed"); } - return balancerRan; + + return responseBuilder.build(); } } diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupProtobufUtil.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupProtobufUtil.java index 6cb441ac4fc7..42472d3cd37d 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupProtobufUtil.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupProtobufUtil.java @@ -21,14 +21,15 @@ import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; - import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.BalanceRequest; +import org.apache.hadoop.hbase.client.BalanceResponse; import org.apache.hadoop.hbase.net.Address; import org.apache.hadoop.hbase.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos; import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos.NameStringPair; import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.BalanceRSGroupRequest; +import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.BalanceRSGroupResponse; import org.apache.hadoop.hbase.protobuf.generated.RSGroupProtos; import org.apache.hadoop.hbase.protobuf.generated.TableProtos; import org.apache.yetus.audience.InterfaceAudience; @@ -38,6 +39,21 @@ final class RSGroupProtobufUtil { private RSGroupProtobufUtil() { } + static void populateBalanceRSGroupResponse(BalanceRSGroupResponse.Builder responseBuilder, BalanceResponse response) { + responseBuilder + .setBalanceRan(response.isBalancerRan()) + .setMovesCalculated(response.getMovesCalculated()) + .setMovesExecuted(response.getMovesExecuted()); + } + + static BalanceResponse toBalanceResponse(BalanceRSGroupResponse response) { + return BalanceResponse.newBuilder() + .setBalancerRan(response.getBalanceRan()) + .setMovesExecuted(response.hasMovesExecuted() ? response.getMovesExecuted() : 0) + .setMovesCalculated(response.hasMovesCalculated() ? response.getMovesCalculated() : 0) + .build(); + } + static BalanceRSGroupRequest createBalanceRSGroupRequest(String groupName, BalanceRequest request) { return BalanceRSGroupRequest.newBuilder() .setRSGroupName(groupName) diff --git a/hbase-rsgroup/src/main/protobuf/RSGroupAdmin.proto b/hbase-rsgroup/src/main/protobuf/RSGroupAdmin.proto index a3f8bf22edc0..2a4f2d80269f 100644 --- a/hbase-rsgroup/src/main/protobuf/RSGroupAdmin.proto +++ b/hbase-rsgroup/src/main/protobuf/RSGroupAdmin.proto @@ -92,6 +92,8 @@ message BalanceRSGroupRequest { message BalanceRSGroupResponse { required bool balanceRan = 1; + optional uint32 moves_calculated = 2; + optional uint32 moves_executed = 3; } message ListRSGroupInfosRequest { diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBalance.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBalance.java index d3bebe0690f7..2e502b6a85cc 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBalance.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBalance.java @@ -31,6 +31,7 @@ import org.apache.hadoop.hbase.Waiter; import org.apache.hadoop.hbase.Waiter.Predicate; import org.apache.hadoop.hbase.client.BalanceRequest; +import org.apache.hadoop.hbase.client.BalanceResponse; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.TableDescriptor; @@ -96,7 +97,7 @@ public void testGroupBalance() throws Exception { // disable balance, balancer will not be run and return false admin.balancerSwitch(false, true); - assertFalse(rsGroupAdmin.balanceRSGroup(newGroupName)); + assertFalse(rsGroupAdmin.balanceRSGroup(newGroupName).isBalancerRan()); assertEquals(6, getTableServerRegionMap().get(tableName).get(first).size()); // enable balance @@ -128,7 +129,11 @@ public void testGroupDryRunBalance() throws Exception { // run the balancer in dry run mode. it should return true, but should not actually move any regions admin.balancerSwitch(true, true); - assertTrue(rsGroupAdmin.balanceRSGroup(newGroupName, BalanceRequest.newBuilder().setDryRun(true).build())); + BalanceResponse response = rsGroupAdmin.balanceRSGroup(newGroupName, + BalanceRequest.newBuilder().setDryRun(true).build()); + assertTrue(response.isBalancerRan()); + assertTrue(response.getMovesCalculated() > 0); + assertEquals(0, response.getMovesExecuted()); // validate imbalance still exists. assertEquals(6, getTableServerRegionMap().get(tableName).get(first).size()); } @@ -194,7 +199,7 @@ public void testMisplacedRegions() throws Exception { RSGroupInfo.getName()); admin.balancerSwitch(true, true); - assertTrue(rsGroupAdmin.balanceRSGroup(RSGroupInfo.getName())); + assertTrue(rsGroupAdmin.balanceRSGroup(RSGroupInfo.getName()).isBalancerRan()); admin.balancerSwitch(false, true); assertTrue(observer.preBalanceRSGroupCalled); assertTrue(observer.postBalanceRSGroupCalled); diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java index e176bb7cb338..d658ba4a618e 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java @@ -44,6 +44,7 @@ import org.apache.hadoop.hbase.client.AbstractTestUpdateConfiguration; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.BalanceRequest; +import org.apache.hadoop.hbase.client.BalanceResponse; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; @@ -414,7 +415,7 @@ public void preBalanceRSGroup(final ObserverContext ctx, - String groupName, BalanceRequest request, boolean balancerRan) throws IOException { + String groupName, BalanceRequest request, BalanceResponse response) throws IOException { postBalanceRSGroupCalled = true; } diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsFallback.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsFallback.java index 15220c4b5b42..bd45e8ccd52f 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsFallback.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsFallback.java @@ -107,14 +107,14 @@ public void testFallback() throws Exception { Address startRSAddress = t.getRegionServer().getServerName().getAddress(); TEST_UTIL.waitFor(3000, () -> rsGroupAdmin.getRSGroupInfo(RSGroupInfo.DEFAULT_GROUP) .containsServer(startRSAddress)); - assertTrue(master.balance()); + assertTrue(master.balance().isBalancerRan()); assertRegionsInGroup(tableName, RSGroupInfo.DEFAULT_GROUP); // add a new server to test group, regions move back t = TEST_UTIL.getMiniHBaseCluster().startRegionServerAndWait(60000); rsGroupAdmin.moveServers( Collections.singleton(t.getRegionServer().getServerName().getAddress()), groupName); - assertTrue(master.balance()); + assertTrue(master.balance().isBalancerRan()); assertRegionsInGroup(tableName, groupName); TEST_UTIL.deleteTable(tableName); diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdminClient.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdminClient.java index cbe1fcc30816..7b8472d9fd70 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdminClient.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdminClient.java @@ -26,6 +26,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.BalanceRequest; +import org.apache.hadoop.hbase.client.BalanceResponse; import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.Scan; @@ -93,7 +94,7 @@ public void removeRSGroup(String name) throws IOException { } @Override - public boolean balanceRSGroup(String groupName, BalanceRequest request) throws IOException { + public BalanceResponse balanceRSGroup(String groupName, BalanceRequest request) throws IOException { return wrapped.balanceRSGroup(groupName, request); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java index d25ea89cef61..ce70647e9254 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java @@ -29,6 +29,7 @@ import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.BalanceRequest; +import org.apache.hadoop.hbase.client.BalanceResponse; import org.apache.hadoop.hbase.client.MasterSwitchType; import org.apache.hadoop.hbase.client.Mutation; import org.apache.hadoop.hbase.client.RegionInfo; @@ -1285,9 +1286,11 @@ default void preBalanceRSGroup(final ObserverContext ctx, - String groupName, BalanceRequest request, boolean balancerRan) throws IOException { + String groupName, BalanceRequest request, BalanceResponse response) throws IOException { } /** 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 index ee2cf3053bb9..8eb1163bfa96 100644 --- 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 @@ -84,6 +84,7 @@ import org.apache.hadoop.hbase.TableNotDisabledException; import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.UnknownRegionException; +import org.apache.hadoop.hbase.client.BalanceResponse; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor; import org.apache.hadoop.hbase.client.CompactionState; import org.apache.hadoop.hbase.client.MasterSwitchType; @@ -1772,7 +1773,7 @@ private void balanceThrottling(long nextBalanceStartTime, int maxRegionsInTransi if (interrupted) Thread.currentThread().interrupt(); } - public boolean balance() throws IOException { + public BalanceResponse balance() throws IOException { return balance(BalanceRequest.defaultInstance()); } @@ -1799,14 +1800,16 @@ public boolean skipRegionManagementAction(final String action) { return false; } - public boolean balance(BalanceRequest request) throws IOException { + public BalanceResponse balance(BalanceRequest request) throws IOException { + BalanceResponse.Builder responseBuilder = BalanceResponse.newBuilder(); + if (loadBalancerTracker == null || !(loadBalancerTracker.isBalancerOn() || request.isDryRun())) { - return false; + return responseBuilder.build(); } if (skipRegionManagementAction("balancer")) { - return false; + return responseBuilder.build(); } synchronized (this.balancer) { @@ -1828,24 +1831,24 @@ public boolean balance(BalanceRequest request) throws IOException { LOG.info("Not running balancer (ignoreRIT=false" + ", metaRIT=" + metaInTransition + ") because " + regionsInTransition.size() + " region(s) in transition: " + toPrint + (truncated? "(truncated list)": "")); - return false; + return responseBuilder.build(); } } if (this.serverManager.areDeadServersInProgress()) { LOG.info("Not running balancer because processing dead regionserver(s): " + this.serverManager.getDeadServers()); - return false; + return responseBuilder.build(); } if (this.cpHost != null) { try { if (this.cpHost.preBalance(request)) { LOG.debug("Coprocessor bypassing balancer request"); - return false; + return responseBuilder.build(); } } catch (IOException ioe) { LOG.error("Error invoking master coprocessor preBalance()", ioe); - return false; + return responseBuilder.build(); } } @@ -1861,11 +1864,11 @@ public boolean balance(BalanceRequest request) throws IOException { List plans = this.balancer.balanceCluster(assignments); - // make one last check that the cluster isn't shutting down before proceeding. + responseBuilder.setBalancerRan(true).setMovesCalculated(plans == null ? 0 : plans.size()); + if (skipRegionManagementAction("balancer")) { - // return isDryRun because in this case the dry run did run, and the user could see - // the results in the master logs. - return request.isDryRun(); + // make one last check that the cluster isn't shutting down before proceeding. + return responseBuilder.build(); } // For dry run we don't actually want to execute the moves, but we do want @@ -1882,10 +1885,13 @@ public boolean balance(BalanceRequest request) throws IOException { LOG.error("Error invoking master coprocessor postBalance()", ioe); } } + + responseBuilder.setMovesExecuted(sucRPs.size()); } + // If LoadBalancer did not generate any plans, it means the cluster is already balanced. // Return true indicating a success. - return true; + return responseBuilder.build(); } /** diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java index 3108ec429222..a1f2dce361c7 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java @@ -32,6 +32,7 @@ import org.apache.hadoop.hbase.SharedConnection; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.BalanceRequest; +import org.apache.hadoop.hbase.client.BalanceResponse; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.MasterSwitchType; import org.apache.hadoop.hbase.client.Mutation; @@ -1444,12 +1445,12 @@ public void call(MasterObserver observer) throws IOException { }); } - public void postBalanceRSGroup(final String name, final BalanceRequest request, final boolean balanceRan) + public void postBalanceRSGroup(final String name, final BalanceRequest request, final BalanceResponse response) throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { - observer.postBalanceRSGroup(this, name, request, balanceRan); + observer.postBalanceRSGroup(this, name, request, response); } }); } 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 index 58efb65546f7..01378a29861a 100644 --- 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 @@ -663,9 +663,7 @@ public AssignRegionResponse assignRegion(RpcController controller, public BalanceResponse balance(RpcController controller, BalanceRequest request) throws ServiceException { try { - return BalanceResponse.newBuilder() - .setBalancerRan(master.balance(ProtobufUtil.toBalanceRequest(request))) - .build(); + return ProtobufUtil.toBalanceResponse(master.balance(ProtobufUtil.toBalanceRequest(request))); } catch (IOException ex) { throw new ServiceException(ex); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java index ed37713d631a..30492c20826d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.IOException; @@ -27,6 +28,7 @@ import java.util.Collection; import java.util.List; import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.client.BalanceResponse; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.RegionInfo; @@ -128,14 +130,21 @@ public void testRebalanceOnRegionServerNumberChange() assertRegionsAreBalanced(); // On a balanced cluster, calling balance() should return true - assert(UTIL.getHBaseCluster().getMaster().balance() == true); + BalanceResponse response = UTIL.getHBaseCluster().getMaster().balance(); + assertTrue(response.isBalancerRan()); + assertEquals(0, response.getMovesCalculated()); + assertEquals(0, response.getMovesExecuted()); // if we add a server, then the balance() call should return true // add a region server - total of 3 LOG.info("Started third server=" + UTIL.getHBaseCluster().startRegionServer().getRegionServer().getServerName()); waitForAllRegionsAssigned(); - assert(UTIL.getHBaseCluster().getMaster().balance() == true); + + response = UTIL.getHBaseCluster().getMaster().balance(); + assertTrue(response.isBalancerRan()); + assertTrue(response.getMovesCalculated() > 0); + assertEquals(response.getMovesCalculated(), response.getMovesExecuted()); assertRegionsAreBalanced(); // kill a region server - total of 2 @@ -152,14 +161,24 @@ public void testRebalanceOnRegionServerNumberChange() UTIL.getHBaseCluster().startRegionServer().getRegionServer().getServerName()); waitOnCrashProcessing(); waitForAllRegionsAssigned(); - assert(UTIL.getHBaseCluster().getMaster().balance() == true); + + response = UTIL.getHBaseCluster().getMaster().balance(); + assertTrue(response.isBalancerRan()); + assertTrue(response.getMovesCalculated() > 0); + assertEquals(response.getMovesCalculated(), response.getMovesExecuted()); + assertRegionsAreBalanced(); for (int i = 0; i < 6; i++){ LOG.info("Adding " + (i + 5) + "th region server"); UTIL.getHBaseCluster().startRegionServer(); } waitForAllRegionsAssigned(); - assert(UTIL.getHBaseCluster().getMaster().balance() == true); + + response = UTIL.getHBaseCluster().getMaster().balance(); + assertTrue(response.isBalancerRan()); + assertTrue(response.getMovesCalculated() > 0); + assertEquals(response.getMovesCalculated(), response.getMovesExecuted()); + assertRegionsAreBalanced(); regionLocator.close(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableGetMultiThreaded.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableGetMultiThreaded.java index 23ab43de0746..e6f4b1a6e6f2 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableGetMultiThreaded.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableGetMultiThreaded.java @@ -198,7 +198,7 @@ public String explainFailure() throws Exception { } Thread.sleep(5000); LOG.info("====== Balancing cluster ======"); - admin.balance(true); + admin.balance(BalanceRequest.newBuilder().setIgnoreRegionsInTransition(true).build()); LOG.info("====== Balance cluster done ======"); Thread.sleep(5000); ServerName metaServer = TEST_UTIL.getHBaseCluster().getServerHoldingMeta(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSeparateClientZKCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSeparateClientZKCluster.java index e01c84e2a19f..52b69db25534 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSeparateClientZKCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSeparateClientZKCluster.java @@ -144,7 +144,7 @@ public void testMasterSwitch() throws Exception { } LOG.info("Got master {}", cluster.getMaster().getServerName()); // confirm client access still works - assertTrue(admin.balance(false)); + assertTrue(admin.balance(BalanceRequest.defaultInstance()).isBalancerRan()); } } @@ -278,4 +278,4 @@ public void testChangeMetaReplicaCount() throws Exception { TEST_UTIL.waitFor(30000, () -> locator.getAllRegionLocations().size() == 1); } } -} \ No newline at end of file +} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java index 300af75ab80e..71a7643c1f17 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java @@ -41,6 +41,7 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.BalanceRequest; +import org.apache.hadoop.hbase.client.BalanceResponse; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.MasterSwitchType; @@ -1166,7 +1167,7 @@ public void preBalanceRSGroup(ObserverContext ctx, @Override public void postBalanceRSGroup(ObserverContext ctx, - String groupName, BalanceRequest request, boolean balancerRan) throws IOException { + String groupName, BalanceRequest request, BalanceResponse response) throws IOException { } @Override @@ -1618,7 +1619,7 @@ public void testRegionTransitionOperations() throws Exception { UTIL.waitUntilNoRegionsInTransition(); // now trigger a balance master.balanceSwitch(true); - boolean balanceRun = master.balance(); + master.balance(); assertTrue("Coprocessor should be called on region rebalancing", cp.wasBalanceCalled()); } finally { 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 index c8585e102f74..730075394fa6 100644 --- 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 @@ -21,12 +21,14 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import java.io.IOException; +import org.apache.commons.lang3.Validate; import org.apache.hadoop.hbase.client.BalanceRequest; 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.Waiter; +import org.apache.hadoop.hbase.client.BalanceResponse; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.regionserver.HRegionServer; import org.apache.hadoop.hbase.testclassification.MasterTests; @@ -56,7 +58,9 @@ public void shutdown() throws Exception { public void testDryRunBalancer() throws Exception { TEST_UTIL.startMiniCluster(2); - TableName tableName = createTable("testDryRunBalancer"); + int numRegions = 100; + int regionsPerRs = numRegions / 2; + TableName tableName = createTable("testDryRunBalancer", numRegions); HMaster master = Mockito.spy(TEST_UTIL.getHBaseCluster().getMaster()); // dry run should be possible with balancer disabled @@ -66,7 +70,14 @@ public void testDryRunBalancer() throws Exception { HRegionServer biasedServer = unbalance(master, tableName); - assertTrue(master.balance(BalanceRequest.newBuilder().setDryRun(true).build())); + BalanceResponse response = master.balance(BalanceRequest.newBuilder().setDryRun(true).build()); + assertTrue(response.isBalancerRan()); + // we don't know for sure that it will be exactly half the regions + assertTrue( + response.getMovesCalculated() >= (regionsPerRs - 1) + && response.getMovesCalculated() <= (regionsPerRs + 1)); + // but we expect no moves executed due to dry run + assertEquals(0, response.getMovesExecuted()); // sanity check that we truly don't try to execute any plans Mockito.verify(master, Mockito.never()).executeRegionPlansWithThrottling(Mockito.anyList()); @@ -77,9 +88,9 @@ public void testDryRunBalancer() throws Exception { TEST_UTIL.deleteTable(tableName); } - private TableName createTable(String table) throws IOException { + private TableName createTable(String table, int numRegions) throws IOException { TableName tableName = TableName.valueOf(table); - TEST_UTIL.createMultiRegionTable(tableName, FAMILYNAME, 100); + TEST_UTIL.createMultiRegionTable(tableName, FAMILYNAME, numRegions); return tableName; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestProcedurePriority.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestProcedurePriority.java index fa4c4a5283c0..95211a3ffda5 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestProcedurePriority.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestProcedurePriority.java @@ -28,6 +28,7 @@ import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.Waiter.ExplainingPredicate; +import org.apache.hadoop.hbase.client.BalanceRequest; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; import org.apache.hadoop.hbase.client.Durability; import org.apache.hadoop.hbase.client.Get; @@ -120,7 +121,7 @@ public static void setUp() throws Exception { for (Future future : futures) { future.get(1, TimeUnit.MINUTES); } - UTIL.getAdmin().balance(true); + UTIL.getAdmin().balance(BalanceRequest.newBuilder().setIgnoreRegionsInTransition(true).build()); UTIL.waitUntilNoRegionsInTransition(); } diff --git a/hbase-shell/src/main/ruby/shell/commands/balance_rsgroup.rb b/hbase-shell/src/main/ruby/shell/commands/balance_rsgroup.rb index 41eb10eb3b18..aff9fa7ce942 100644 --- a/hbase-shell/src/main/ruby/shell/commands/balance_rsgroup.rb +++ b/hbase-shell/src/main/ruby/shell/commands/balance_rsgroup.rb @@ -40,13 +40,14 @@ def help def command(group_name, *args) # Returns true if balancer was run, otherwise false. - ret = rsgroup_admin.balance_rs_group(group_name, args) - if ret - puts 'Ran the balancer.' + resp = rsgroup_admin.balance_rs_group(group_name, args) + if resp.isBalancerRan + formatter.row(["Balancer ran"]) + formatter.row(["Moves calculated: #{resp.getMovesCalculated}, moves executed: #{resp.getMovesExecuted}"]) else - puts "Couldn't run the balancer." + formatter.row(["Balancer did not run. See logs for details."]) end - ret + resp.isBalancerRan end end end diff --git a/hbase-shell/src/main/ruby/shell/commands/balancer.rb b/hbase-shell/src/main/ruby/shell/commands/balancer.rb index 7a1f63cf3877..d5304e0007f6 100644 --- a/hbase-shell/src/main/ruby/shell/commands/balancer.rb +++ b/hbase-shell/src/main/ruby/shell/commands/balancer.rb @@ -40,9 +40,14 @@ def help end def command(*args) - did_balancer_run = !!admin.balancer(args) - formatter.row([did_balancer_run.to_s]) - did_balancer_run + resp = admin.balancer(args) + if resp.isBalancerRan + formatter.row(["Balancer ran"]) + formatter.row(["Moves calculated: #{resp.getMovesCalculated}, moves executed: #{resp.getMovesExecuted}"]) + else + formatter.row(["Balancer did not run. See logs for details."]) + end + resp.isBalancerRan end end end diff --git a/hbase-shell/src/test/ruby/hbase/admin_test.rb b/hbase-shell/src/test/ruby/hbase/admin_test.rb index 8dd69202a46a..26793a121926 100644 --- a/hbase-shell/src/test/ruby/hbase/admin_test.rb +++ b/hbase-shell/src/test/ruby/hbase/admin_test.rb @@ -192,11 +192,13 @@ def teardown did_balancer_run = command(:balancer) assert(did_balancer_run == true) output = capture_stdout { command(:balancer, 'force') } - assert(output.include?('true')) + assert(output.include?('Balancer ran')) command(:balance_switch, false) + output = capture_stdout { command(:balancer) } + assert(output.include?('Balancer did not run')) output = capture_stdout { command(:balancer, 'dry_run') } - assert(output.include?('true')) + assert(output.include?('Balancer ran')) 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 index 5ab1c1f6f276..0aa9862c031a 100644 --- 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 @@ -41,6 +41,7 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.client.BalanceResponse; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor; import org.apache.hadoop.hbase.client.CompactType; import org.apache.hadoop.hbase.client.CompactionState; @@ -787,7 +788,8 @@ public boolean balance(boolean force) { throw new NotImplementedException("balance not supported in ThriftAdmin"); } - @Override public boolean balance(BalanceRequest request) throws IOException { + @Override + public BalanceResponse balance(BalanceRequest request) throws IOException { throw new NotImplementedException("balance not supported in ThriftAdmin"); } From e5340e6b8a718d9e2a0ca102c4c9d116d1dc51c3 Mon Sep 17 00:00:00 2001 From: Bryan Beaudreault Date: Fri, 27 Aug 2021 13:36:38 -0400 Subject: [PATCH 3/3] Remove InterfaceStability and improve docs --- .../hadoop/hbase/client/BalanceRequest.java | 24 ++++++++-- .../hadoop/hbase/client/BalanceResponse.java | 44 +++++++++++++++---- 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceRequest.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceRequest.java index d5c8f2bdcfb2..e464410c8df5 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceRequest.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceRequest.java @@ -19,18 +19,18 @@ package org.apache.hadoop.hbase.client; import org.apache.yetus.audience.InterfaceAudience; -import org.apache.yetus.audience.InterfaceStability; /** * Encapsulates options for executing a run of the Balancer. */ @InterfaceAudience.Public -@InterfaceStability.Evolving public final class BalanceRequest { private static final BalanceRequest DEFAULT = BalanceRequest.newBuilder().build(); + /** + * Builder for constructing a {@link BalanceRequest} + */ @InterfaceAudience.Public - @InterfaceStability.Evolving public final static class Builder { private boolean dryRun = false; private boolean ignoreRegionsInTransition = false; @@ -65,15 +65,25 @@ public Builder setIgnoreRegionsInTransition(boolean ignoreRegionsInTransition) { return this; } + /** + * Build the {@link BalanceRequest} + */ public BalanceRequest build() { return new BalanceRequest(dryRun, ignoreRegionsInTransition); } } + /** + * Create a builder to construct a custom {@link BalanceRequest}. + */ public static Builder newBuilder() { return new Builder(); } + /** + * Get a BalanceRequest for a default run of the balancer. The default mode executes + * any moves calculated and will not run if regions are already in transition. + */ public static BalanceRequest defaultInstance() { return DEFAULT; } @@ -86,10 +96,18 @@ private BalanceRequest(boolean dryRun, boolean ignoreRegionsInTransition) { this.ignoreRegionsInTransition = ignoreRegionsInTransition; } + /** + * Returns true if the balancer should run in dry run mode, otherwise false. In + * dry run mode, moves will be calculated but not executed. + */ public boolean isDryRun() { return dryRun; } + /** + * Returns true if the balancer should execute even if regions are in transition, otherwise + * false. This is an advanced usage feature, as it can cause more issues than it fixes. + */ public boolean isIgnoreRegionsInTransition() { return ignoreRegionsInTransition; } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceResponse.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceResponse.java index 6b02b21a2eeb..0d8e84b9a0d2 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceResponse.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceResponse.java @@ -19,17 +19,17 @@ package org.apache.hadoop.hbase.client; import org.apache.yetus.audience.InterfaceAudience; -import org.apache.yetus.audience.InterfaceStability; /** * Response returned from a balancer invocation */ @InterfaceAudience.Public -@InterfaceStability.Evolving public final class BalanceResponse { + /** + * Builds a {@link BalanceResponse} for returning results of a balance invocation to callers + */ @InterfaceAudience.Public - @InterfaceStability.Evolving public final static class Builder { private boolean balancerRan; private int movesCalculated; @@ -37,26 +37,52 @@ public final static class Builder { private Builder() {} + /** + * Set true if the balancer ran, otherwise false. The balancer may not run in some + * circumstances, such as if a balance is already running or there are regions already + * in transition. + * + * @param balancerRan true if balancer ran, false otherwise + */ public Builder setBalancerRan(boolean balancerRan) { this.balancerRan = balancerRan; return this; } + /** + * Set how many moves were calculated by the balancer. This will be zero if the cluster is + * already balanced. + * + * @param movesCalculated moves calculated by the balance run + */ public Builder setMovesCalculated(int movesCalculated) { this.movesCalculated = movesCalculated; return this; } + /** + * Set how many of the calculated moves were actually executed by the balancer. This should be + * zero if the balancer is run with {@link BalanceRequest#isDryRun()}. It may also not equal + * movesCalculated if the balancer ran out of time while executing the moves. + * + * @param movesExecuted moves executed by the balance run + */ public Builder setMovesExecuted(int movesExecuted) { this.movesExecuted = movesExecuted; return this; } + /** + * Build the {@link BalanceResponse} + */ public BalanceResponse build() { return new BalanceResponse(balancerRan, movesCalculated, movesExecuted); } } + /** + * Creates a new {@link BalanceResponse.Builder} + */ public static Builder newBuilder() { return new Builder(); } @@ -72,17 +98,17 @@ private BalanceResponse(boolean balancerRan, int movesCalculated, int movesExecu } /** - * Determines whether the balancer ran or not. The balancer may not run for a variety of reasons, - * such as: another balance is running, there are regions in transition, the cluster is in - * maintenance mode, etc. + * Returns true if the balancer ran, otherwise false. The balancer may not run for a + * variety of reasons, such as: another balance is running, there are regions in + * transition, the cluster is in maintenance mode, etc. */ public boolean isBalancerRan() { return balancerRan; } /** - * The number of moves calculated by the balancer if it ran. This may be zero if - * no better balance could be found. + * The number of moves calculated by the balancer if {@link #isBalancerRan()} is true. This will + * be zero if no better balance could be found. */ public int getMovesCalculated() { return movesCalculated; @@ -92,7 +118,7 @@ public int getMovesCalculated() { * The number of moves actually executed by the balancer if it ran. This will be * zero if {@link #getMovesCalculated()} is zero or if {@link BalanceRequest#isDryRun()} * was true. It may also not be equal to {@link #getMovesCalculated()} if the balancer - * was interrupted midway through executing the moves due to max run time. + * was interrupted midway through executing the moves due to max run time. */ public int getMovesExecuted() { return movesExecuted;