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..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 @@ -1251,7 +1251,20 @@ 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()) + .isBalancerRan(); + } + + /** + * 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 {@link BalanceResponse} with details about the results of the invocation. + * @throws IOException if a remote or network exception occurs + */ + BalanceResponse balance(BalanceRequest request) throws IOException; /** * Invoke the balancer. Will run the balancer and if regions to move, it will @@ -1262,7 +1275,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 +1290,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() + ).isBalancerRan(); + } /** * 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..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(false); + return balance(BalanceRequest.defaultInstance()) + .thenApply(BalanceResponse::isBalancerRan); } /** @@ -1267,8 +1268,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() + ).thenApply(BalanceResponse::isBalancerRan); + } + + /** + * 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 {@link BalanceResponse} with details about the results of the invocation. */ - 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..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,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..e464410c8df5 --- /dev/null +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceRequest.java @@ -0,0 +1,114 @@ +/* + * + * 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; + +/** + * Encapsulates options for executing a run of the Balancer. + */ +@InterfaceAudience.Public +public final class BalanceRequest { + private static final BalanceRequest DEFAULT = BalanceRequest.newBuilder().build(); + + /** + * Builder for constructing a {@link BalanceRequest} + */ + @InterfaceAudience.Public + 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; + } + + /** + * 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; + } + + private final boolean dryRun; + private final boolean ignoreRegionsInTransition; + + private BalanceRequest(boolean dryRun, boolean ignoreRegionsInTransition) { + this.dryRun = dryRun; + 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 new file mode 100644 index 000000000000..0d8e84b9a0d2 --- /dev/null +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceResponse.java @@ -0,0 +1,126 @@ +/* + * + * 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; + +/** + * Response returned from a balancer invocation + */ +@InterfaceAudience.Public +public final class BalanceResponse { + + /** + * Builds a {@link BalanceResponse} for returning results of a balance invocation to callers + */ + @InterfaceAudience.Public + public final static class Builder { + private boolean balancerRan; + private int movesCalculated; + private int movesExecuted; + + 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(); + } + + 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; + } + + /** + * 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 {@link #isBalancerRan()} is true. This will + * 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 b7ed2d03ad97..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,26 +1475,14 @@ protected Boolean rpcCall() throws Exception { }); } - @Override - public boolean balance() 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 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 57d4bac5a3bc..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 @@ -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() + . newMasterCaller() .action( - (controller, stub) -> this. call(controller, - stub, RequestConverter.buildBalanceRequest(forcible), - (s, c, req, done) -> s.balance(c, req, done), (resp) -> resp.getBalancerRan())).call(); + (controller, stub) -> this. call(controller, + stub, ProtobufUtil.toBalanceRequest(request), + (s, c, req, done) -> s.balance(c, req, done), (resp) -> ProtobufUtil.toBalanceResponse(resp))).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..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 @@ -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; @@ -67,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; @@ -3753,4 +3755,34 @@ 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(); + } + + 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-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..4a6bb3959532 100644 --- a/hbase-protocol-shaded/src/main/protobuf/Master.proto +++ b/hbase-protocol-shaded/src/main/protobuf/Master.proto @@ -292,11 +292,14 @@ message IsInMaintenanceModeResponse { } message BalanceRequest { - optional bool force = 1; + optional bool ignore_rit = 1; + optional bool dry_run = 2; } 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 e698f9ac6f25..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 @@ -23,6 +23,8 @@ import java.util.Set; 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; @@ -67,7 +69,17 @@ public interface RSGroupAdmin { * * @return boolean Whether balance ran or not */ - boolean balanceRSGroup(String groupName) throws IOException; + default BalanceResponse 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 + */ + 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 08357f64bc9b..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 @@ -32,13 +32,15 @@ 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.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.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 +160,11 @@ public void removeRSGroup(String name) throws IOException { } @Override - public boolean balanceRSGroup(String groupName) throws IOException { - BalanceRSGroupRequest request = BalanceRSGroupRequest.newBuilder() - .setRSGroupName(groupName).build(); + public BalanceResponse balanceRSGroup(String groupName, BalanceRequest request) throws IOException { try { - return stub.balanceRSGroup(null, 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 bf26de170626..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 @@ -39,6 +39,8 @@ 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.BalanceResponse; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.SnapshotDescription; import org.apache.hadoop.hbase.client.TableDescriptor; @@ -291,24 +293,31 @@ public void removeRSGroup(RpcController controller, @Override public void balanceRSGroup(RpcController controller, BalanceRSGroupRequest request, RpcCallback done) { - BalanceRSGroupResponse.Builder builder = BalanceRSGroupResponse.newBuilder(); + BalanceRequest balanceRequest = RSGroupProtobufUtil.toBalanceRequest(request); + + 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()); + master.getMasterCoprocessorHost().preBalanceRSGroup(request.getRSGroupName(), balanceRequest); } + checkPermission("balanceRSGroup"); - boolean balancerRan = groupAdminServer.balanceRSGroup(request.getRSGroupName()); - builder.setBalanceRan(balancerRan); + BalanceResponse response = groupAdminServer.balanceRSGroup(request.getRSGroupName(), balanceRequest); + RSGroupProtobufUtil.populateBalanceRSGroupResponse(builder, response); + if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().postBalanceRSGroup(request.getRSGroupName(), - balancerRan); + balanceRequest, + 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 744749f41c0a..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 @@ -33,6 +33,8 @@ 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.BalanceResponse; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; @@ -508,32 +510,36 @@ public void removeRSGroup(String name) throws IOException { } @Override - public boolean balanceRSGroup(String groupName) 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()) { - return false; + if (!((HMaster) master).isBalancerOn() && !request.isDryRun()) { + return responseBuilder.build(); } 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; + 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 @@ -541,12 +547,17 @@ public boolean balanceRSGroup(String groupName) throws IOException { getRSGroupAssignmentsByTable(master.getTableStateManager(), groupName); List plans = balancer.balanceCluster(assignmentsByTable); boolean balancerRan = !plans.isEmpty(); - if (balancerRan) { + + responseBuilder.setBalancerRan(balancerRan).setMovesCalculated(plans.size()); + + 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 9f092c52f2c0..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,12 +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; @@ -36,6 +39,36 @@ 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) + .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..2a4f2d80269f 100644 --- a/hbase-rsgroup/src/main/protobuf/RSGroupAdmin.proto +++ b/hbase-rsgroup/src/main/protobuf/RSGroupAdmin.proto @@ -86,10 +86,14 @@ message RemoveRSGroupResponse { message BalanceRSGroupRequest { required string r_s_group_name = 1; + optional bool ignore_rit = 2; + optional bool dry_run = 3; } 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 e419ee0b8edc..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 @@ -30,6 +30,8 @@ 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.BalanceResponse; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.TableDescriptor; @@ -80,11 +82,65 @@ 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).isBalancerRan()); + 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); + 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()); + } + + 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 +182,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 @@ -167,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 c4ae7d448aba..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 @@ -43,6 +43,8 @@ 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.BalanceResponse; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; @@ -407,13 +409,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, 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 99d36ee7fe34..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 @@ -25,6 +25,8 @@ 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; @@ -92,8 +94,8 @@ public void removeRSGroup(String name) throws IOException { } @Override - public boolean balanceRSGroup(String groupName) throws IOException { - return wrapped.balanceRSGroup(groupName); + public BalanceResponse 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..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 @@ -28,6 +28,8 @@ 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.BalanceResponse; import org.apache.hadoop.hbase.client.MasterSwitchType; import org.apache.hadoop.hbase.client.Mutation; import org.apache.hadoop.hbase.client.RegionInfo; @@ -566,18 +568,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,15 +1279,19 @@ default void postRemoveRSGroup(final ObserverContext ctx, - String groupName) throws IOException {} + String groupName, BalanceRequest request) throws IOException { + } /** * Called after a region server group is removed * @param ctx the environment to interact with the framework and master * @param groupName group name + * @param request the request sent to the balancer + * @param response the response returned by the balancer */ default void postBalanceRSGroup(final ObserverContext ctx, - String groupName, boolean balancerRan) throws IOException {} + String groupName, BalanceRequest request, BalanceResponse response) 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..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 @@ -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; @@ -83,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; @@ -1771,8 +1773,8 @@ private void balanceThrottling(long nextBalanceStartTime, int maxRegionsInTransi if (interrupted) Thread.currentThread().interrupt(); } - public boolean balance() throws IOException { - return balance(false); + public BalanceResponse balance() throws IOException { + return balance(BalanceRequest.defaultInstance()); } /** @@ -1798,12 +1800,16 @@ public boolean skipRegionManagementAction(final String action) { return false; } - public boolean balance(boolean force) throws IOException { - if (loadBalancerTracker == null || !loadBalancerTracker.isBalancerOn()) { - return false; + public BalanceResponse balance(BalanceRequest request) throws IOException { + BalanceResponse.Builder responseBuilder = BalanceResponse.newBuilder(); + + if (loadBalancerTracker == null + || !(loadBalancerTracker.isBalancerOn() || request.isDryRun())) { + return responseBuilder.build(); } + if (skipRegionManagementAction("balancer")) { - return false; + return responseBuilder.build(); } synchronized (this.balancer) { @@ -1820,28 +1826,29 @@ 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)": "")); - return false; + + 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 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()) { + 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(); } } @@ -1857,25 +1864,34 @@ public boolean balance(boolean force) throws IOException { List plans = this.balancer.balanceCluster(assignments); + responseBuilder.setBalancerRan(true).setMovesCalculated(plans == null ? 0 : plans.size()); + if (skipRegionManagementAction("balancer")) { // make one last check that the cluster isn't shutting down before proceeding. - return false; + return responseBuilder.build(); } - 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); } } + + 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 3170bfc27da4..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 @@ -31,6 +31,8 @@ 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.BalanceResponse; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.MasterSwitchType; import org.apache.hadoop.hbase.client.Mutation; @@ -737,20 +739,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 +1435,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 BalanceResponse response) 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, 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 85f88c640b1b..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,8 +663,7 @@ 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 ProtobufUtil.toBalanceResponse(master.balance(ProtobufUtil.toBalanceRequest(request))); } 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/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/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/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 2cab41b6818d..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 @@ -40,6 +40,8 @@ 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.BalanceResponse; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.MasterSwitchType; @@ -689,13 +691,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 +1162,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, BalanceResponse response) throws IOException { } @Override @@ -1616,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 new file mode 100644 index 000000000000..730075394fa6 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterDryRunBalancer.java @@ -0,0 +1,126 @@ +/** + * 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.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; +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); + + 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 + // 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); + + 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()); + + // should still be unbalanced post dry run + assertServerContainsAllRegions(biasedServer.getServerName(), tableName); + + TEST_UTIL.deleteTable(tableName); + } + + private TableName createTable(String table, int numRegions) throws IOException { + TableName tableName = TableName.valueOf(table); + TEST_UTIL.createMultiRegionTable(tableName, FAMILYNAME, numRegions); + 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/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-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..aff9fa7ce942 100644 --- a/hbase-shell/src/main/ruby/shell/commands/balance_rsgroup.rb +++ b/hbase-shell/src/main/ruby/shell/commands/balance_rsgroup.rb @@ -22,22 +22,32 @@ 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) - 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 c4acdc0ecdeb..d5304e0007f6 100644 --- a/hbase-shell/src/main/ruby/shell/commands/balancer.rb +++ b/hbase-shell/src/main/ruby/shell/commands/balancer.rb @@ -22,31 +22,32 @@ 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}." + def command(*args) + 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 - did_balancer_run = !!admin.balancer(force_balancer) - formatter.row([did_balancer_run.to_s]) - did_balancer_run + 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 19910cac8d98..26793a121926 100644 --- a/hbase-shell/src/test/ruby/hbase/admin_test.rb +++ b/hbase-shell/src/test/ruby/hbase/admin_test.rb @@ -192,7 +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?('Balancer ran')) 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..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 @@ -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; @@ -40,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; @@ -786,6 +788,11 @@ public boolean balance(boolean force) { throw new NotImplementedException("balance not supported in ThriftAdmin"); } + @Override + public BalanceResponse 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");