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 a3a51071b33..fb616128d99 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,20 +1251,7 @@ public interface Admin extends Abortable, Closeable { * @return true if balancer ran, false otherwise. * @throws IOException if a remote or network exception occurs */ - 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; + boolean balance() throws IOException; /** * Invoke the balancer. Will run the balancer and if regions to move, it will @@ -1275,7 +1262,7 @@ public interface Admin extends Abortable, Closeable { * @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(BalanceRequest)} instead. + * Use {@link #balance(boolean)} instead. */ @Deprecated default boolean balancer(boolean force) throws IOException { @@ -1290,17 +1277,8 @@ public interface Admin extends Abortable, Closeable { * @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. */ - @Deprecated - default boolean balance(boolean force) throws IOException { - return balance( - BalanceRequest.newBuilder() - .setIgnoreRegionsInTransition(force) - .build() - ).isBalancerRan(); - } + boolean balance(boolean force) throws IOException; /** * 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 85d545505e9..6b8fda79f07 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,8 +1257,7 @@ public interface AsyncAdmin { * {@link CompletableFuture}. */ default CompletableFuture balance() { - return balance(BalanceRequest.defaultInstance()) - .thenApply(BalanceResponse::isBalancerRan); + return balance(false); } /** @@ -1268,25 +1267,8 @@ public interface AsyncAdmin { * @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(BalanceRequest request); + CompletableFuture balance(boolean forcible); /** * 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 db720f3f68e..c7b9897f501 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 @@ class AsyncHBaseAdmin implements AsyncAdmin { } @Override - public CompletableFuture balance(BalanceRequest request) { - return wrap(rawAdmin.balance(request)); + public CompletableFuture balance(boolean forcible) { + return wrap(rawAdmin.balance(forcible)); } @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 deleted file mode 100644 index e464410c8df..00000000000 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceRequest.java +++ /dev/null @@ -1,114 +0,0 @@ -/* - * - * 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 deleted file mode 100644 index 0d8e84b9a0d..00000000000 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceResponse.java +++ /dev/null @@ -1,126 +0,0 @@ -/* - * - * 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 0704d518d07..b7ed2d03ad9 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,25 +92,6 @@ import org.apache.hadoop.hbase.security.access.GetUserPermissionsRequest; 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; @@ -182,7 +163,8 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.IsInMainte 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; @@ -229,6 +211,25 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.ReplicationProtos.GetRe 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 @@ -1475,14 +1476,26 @@ public class HBaseAdmin implements Admin { }); } - @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 + 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 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 a0e5320f855..57d4bac5a3b 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,13 +126,14 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos; 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; @@ -3209,16 +3210,15 @@ class RawAsyncHBaseAdmin implements AsyncAdmin { } @Override - public CompletableFuture balance(BalanceRequest request) { + public CompletableFuture balance(boolean forcible) { return this - . newMasterCaller() + . newMasterCaller() .action( - (controller, stub) -> this. call(controller, - stub, ProtobufUtil.toBalanceRequest(request), - (s, c, req, done) -> s.balance(c, req, done), (resp) -> ProtobufUtil.toBalanceResponse(resp))).call(); + (controller, stub) -> this. call(controller, + stub, RequestConverter.buildBalanceRequest(forcible), + (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 2297732eae0..c2544f6d008 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,7 +46,6 @@ import java.util.stream.Collectors; 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; @@ -68,7 +67,6 @@ import org.apache.hadoop.hbase.NamespaceDescriptor; 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; @@ -3755,34 +3753,4 @@ public final class ProtobufUtil { .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 5f2181e3ccd..bdae13bbfce 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,6 +106,7 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.RegionSpeci 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; @@ -1575,6 +1576,15 @@ public final class RequestConverter { 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 4a6bb395953..045feb4a416 100644 --- a/hbase-protocol-shaded/src/main/protobuf/Master.proto +++ b/hbase-protocol-shaded/src/main/protobuf/Master.proto @@ -292,14 +292,11 @@ message IsInMaintenanceModeResponse { } message BalanceRequest { - optional bool ignore_rit = 1; - optional bool dry_run = 2; + optional bool force = 1; } 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 c1c591633da..e698f9ac6f2 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,8 +23,6 @@ import java.util.Map; 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; @@ -69,17 +67,7 @@ public interface RSGroupAdmin { * * @return boolean Whether balance ran or not */ - 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; + boolean balanceRSGroup(String groupName) 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 9e5c167cbee..08357f64bc9 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,15 +32,13 @@ import org.apache.hadoop.hbase.ServerName; 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; @@ -160,11 +158,11 @@ public class RSGroupAdminClient implements RSGroupAdmin { } @Override - public BalanceResponse balanceRSGroup(String groupName, BalanceRequest request) throws IOException { + public boolean balanceRSGroup(String groupName) throws IOException { + BalanceRSGroupRequest request = BalanceRSGroupRequest.newBuilder() + .setRSGroupName(groupName).build(); try { - RSGroupAdminProtos.BalanceRSGroupRequest req = - RSGroupProtobufUtil.createBalanceRSGroupRequest(groupName, request); - return RSGroupProtobufUtil.toBalanceResponse(stub.balanceRSGroup(null, req)); + return stub.balanceRSGroup(null, 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 5488fa81d93..bf26de17062 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,8 +39,6 @@ import org.apache.hadoop.hbase.NamespaceDescriptor; 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; @@ -293,31 +291,24 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { @Override public void balanceRSGroup(RpcController controller, BalanceRSGroupRequest request, RpcCallback done) { - BalanceRequest balanceRequest = RSGroupProtobufUtil.toBalanceRequest(request); - - BalanceRSGroupResponse.Builder builder = BalanceRSGroupResponse.newBuilder() - .setBalanceRan(false); - + BalanceRSGroupResponse.Builder builder = BalanceRSGroupResponse.newBuilder(); LOG.info(master.getClientIdAuditPrefix() + " balance rsgroup, group=" + request.getRSGroupName()); try { if (master.getMasterCoprocessorHost() != null) { - master.getMasterCoprocessorHost().preBalanceRSGroup(request.getRSGroupName(), balanceRequest); + master.getMasterCoprocessorHost().preBalanceRSGroup(request.getRSGroupName()); } - checkPermission("balanceRSGroup"); - BalanceResponse response = groupAdminServer.balanceRSGroup(request.getRSGroupName(), balanceRequest); - RSGroupProtobufUtil.populateBalanceRSGroupResponse(builder, response); - + boolean balancerRan = groupAdminServer.balanceRSGroup(request.getRSGroupName()); + builder.setBalanceRan(balancerRan); if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().postBalanceRSGroup(request.getRSGroupName(), - balanceRequest, - response); + balancerRan); } } 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 d25cb5e7780..744749f41c0 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,8 +33,6 @@ import org.apache.hadoop.hbase.HConstants; 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; @@ -510,36 +508,32 @@ public class RSGroupAdminServer implements RSGroupAdmin { } @Override - public BalanceResponse balanceRSGroup(String groupName, BalanceRequest request) throws IOException { + public boolean balanceRSGroup(String groupName) 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 responseBuilder.build(); + if (!((HMaster) master).isBalancerOn()) { + 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 && !request.isIgnoreRegionsInTransition()) { + if (groupRIT.size() > 0) { LOG.debug("Not running balancer because {} region(s) in transition: {}", groupRIT.size(), StringUtils.abbreviate( master.getAssignmentManager().getRegionStates().getRegionsInTransition().toString(), 256)); - return responseBuilder.build(); + return false; } - if (serverManager.areDeadServersInProgress()) { LOG.debug("Not running balancer because processing dead regionserver(s): {}", serverManager.getDeadServers()); - return responseBuilder.build(); + return false; } //We balance per group instead of per table @@ -547,17 +541,12 @@ public class RSGroupAdminServer implements RSGroupAdmin { getRSGroupAssignmentsByTable(master.getTableStateManager(), groupName); List plans = balancer.balanceCluster(assignmentsByTable); boolean balancerRan = !plans.isEmpty(); - - responseBuilder.setBalancerRan(balancerRan).setMovesCalculated(plans.size()); - - if (balancerRan && !request.isDryRun()) { + if (balancerRan) { LOG.info("RSGroup balance {} starting with plan count: {}", groupName, plans.size()); - List executed = master.executeRegionPlansWithThrottling(plans); - responseBuilder.setMovesExecuted(executed.size()); + master.executeRegionPlansWithThrottling(plans); LOG.info("RSGroup balance " + groupName + " completed"); } - - return responseBuilder.build(); + return balancerRan; } } 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 42472d3cd37..9f092c52f2c 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,15 +21,12 @@ package org.apache.hadoop.hbase.rsgroup; 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; @@ -39,36 +36,6 @@ 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 2a4f2d80269..54add90eff4 100644 --- a/hbase-rsgroup/src/main/protobuf/RSGroupAdmin.proto +++ b/hbase-rsgroup/src/main/protobuf/RSGroupAdmin.proto @@ -86,14 +86,10 @@ 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 2e502b6a85c..e419ee0b8ed 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,8 +30,6 @@ import org.apache.hadoop.hbase.ServerName; 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; @@ -82,65 +80,11 @@ public class TestRSGroupsBalance extends TestRSGroupsBase { @Test public void testGroupBalance() throws Exception { - 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 { + LOG.info(name.getMethodName()); + String newGroupName = getGroupName(name.getMethodName()); 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) @@ -182,7 +126,31 @@ public class TestRSGroupsBalance extends TestRSGroupsBase { } }); - return first; + // 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 @@ -199,7 +167,7 @@ public class TestRSGroupsBalance extends TestRSGroupsBase { RSGroupInfo.getName()); admin.balancerSwitch(true, true); - assertTrue(rsGroupAdmin.balanceRSGroup(RSGroupInfo.getName()).isBalancerRan()); + assertTrue(rsGroupAdmin.balanceRSGroup(RSGroupInfo.getName())); 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 d658ba4a618..c4ae7d448ab 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,8 +43,6 @@ import org.apache.hadoop.hbase.TableName; 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; @@ -409,13 +407,13 @@ public abstract class TestRSGroupsBase extends AbstractTestUpdateConfiguration { @Override public void preBalanceRSGroup(final ObserverContext ctx, - String groupName, BalanceRequest request) throws IOException { + String groupName) throws IOException { preBalanceRSGroupCalled = true; } @Override public void postBalanceRSGroup(final ObserverContext ctx, - String groupName, BalanceRequest request, BalanceResponse response) throws IOException { + String groupName, boolean balancerRan) 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 bd45e8ccd52..15220c4b5b4 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 class TestRSGroupsFallback extends TestRSGroupsBase { Address startRSAddress = t.getRegionServer().getServerName().getAddress(); TEST_UTIL.waitFor(3000, () -> rsGroupAdmin.getRSGroupInfo(RSGroupInfo.DEFAULT_GROUP) .containsServer(startRSAddress)); - assertTrue(master.balance().isBalancerRan()); + assertTrue(master.balance()); 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().isBalancerRan()); + assertTrue(master.balance()); 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 7b8472d9fd7..99d36ee7fe3 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,8 +25,6 @@ import java.util.Set; 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; @@ -94,8 +92,8 @@ public class VerifyingRSGroupAdminClient implements RSGroupAdmin { } @Override - public BalanceResponse balanceRSGroup(String groupName, BalanceRequest request) throws IOException { - return wrapped.balanceRSGroup(groupName, request); + public boolean balanceRSGroup(String groupName) throws IOException { + return wrapped.balanceRSGroup(groupName); } @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 ce70647e925..7f09fb0e63d 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,8 +28,6 @@ import org.apache.hadoop.hbase.MetaMutationAnnotation; 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; @@ -568,20 +566,18 @@ public interface MasterObserver { * Called prior to requesting rebalancing of the cluster regions, though after * the initial checks for regions in transition and the balance switch flag. * @param ctx the environment to interact with the framework and master - * @param request the request used to trigger the balancer */ - default void preBalance(final ObserverContext ctx, BalanceRequest request) + default void preBalance(final ObserverContext ctx) 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, BalanceRequest request, List plans) + default void postBalance(final ObserverContext ctx, List plans) throws IOException {} /** @@ -1279,19 +1275,15 @@ public interface MasterObserver { * @param groupName group name */ default void preBalanceRSGroup(final ObserverContext ctx, - String groupName, BalanceRequest request) throws IOException { - } + String groupName) 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, BalanceRequest request, BalanceResponse response) throws IOException { - } + String groupName, 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 866e3fd5005..350e68aa15b 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,7 +61,6 @@ import org.apache.hadoop.fs.Path; 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; @@ -84,7 +83,6 @@ import org.apache.hadoop.hbase.TableName; 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,8 +1770,8 @@ public class HMaster extends HRegionServer implements MasterServices { if (interrupted) Thread.currentThread().interrupt(); } - public BalanceResponse balance() throws IOException { - return balance(BalanceRequest.defaultInstance()); + public boolean balance() throws IOException { + return balance(false); } /** @@ -1799,16 +1797,12 @@ public class HMaster extends HRegionServer implements MasterServices { return false; } - public BalanceResponse balance(BalanceRequest request) throws IOException { - BalanceResponse.Builder responseBuilder = BalanceResponse.newBuilder(); - - if (loadBalancerTracker == null - || !(loadBalancerTracker.isBalancerOn() || request.isDryRun())) { - return responseBuilder.build(); + public boolean balance(boolean force) throws IOException { + if (loadBalancerTracker == null || !loadBalancerTracker.isBalancerOn()) { + return false; } - if (skipRegionManagementAction("balancer")) { - return responseBuilder.build(); + return false; } synchronized (this.balancer) { @@ -1825,29 +1819,28 @@ public class HMaster extends HRegionServer implements MasterServices { toPrint = regionsInTransition.subList(0, max); truncated = true; } - - 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 (!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 (this.serverManager.areDeadServersInProgress()) { LOG.info("Not running balancer because processing dead regionserver(s): " + this.serverManager.getDeadServers()); - return responseBuilder.build(); + return false; } if (this.cpHost != null) { try { - if (this.cpHost.preBalance(request)) { + if (this.cpHost.preBalance()) { LOG.debug("Coprocessor bypassing balancer request"); - return responseBuilder.build(); + return false; } } catch (IOException ioe) { LOG.error("Error invoking master coprocessor preBalance()", ioe); - return responseBuilder.build(); + return false; } } @@ -1863,34 +1856,25 @@ public class HMaster extends HRegionServer implements MasterServices { 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 responseBuilder.build(); + return false; } - // 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); + List sucRPs = executeRegionPlansWithThrottling(plans); if (this.cpHost != null) { try { - this.cpHost.postBalance(request, sucRPs); + this.cpHost.postBalance(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 responseBuilder.build(); + return true; } /** 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 a1f2dce361c..3170bfc27da 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,8 +31,6 @@ import org.apache.hadoop.hbase.NamespaceDescriptor; 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; @@ -739,20 +737,20 @@ public class MasterCoprocessorHost }); } - public boolean preBalance(final BalanceRequest request) throws IOException { + public boolean preBalance() throws IOException { return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { - observer.preBalance(this, request); + observer.preBalance(this); } }); } - public void postBalance(final BalanceRequest request, final List plans) throws IOException { + public void postBalance(final List plans) throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { - observer.postBalance(this, request, plans); + observer.postBalance(this, plans); } }); } @@ -1435,22 +1433,22 @@ public class MasterCoprocessorHost }); } - public void preBalanceRSGroup(final String name, final BalanceRequest request) + public void preBalanceRSGroup(final String name) throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { - observer.preBalanceRSGroup(this, name, request); + observer.preBalanceRSGroup(this, name); } }); } - public void postBalanceRSGroup(final String name, final BalanceRequest request, final BalanceResponse response) + public void postBalanceRSGroup(final String name, final boolean balanceRan) throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { - observer.postBalanceRSGroup(this, name, request, response); + observer.postBalanceRSGroup(this, name, 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 01378a29861..85f88c640b1 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,7 +663,8 @@ public class MasterRpcServices extends RSRpcServices implements public BalanceResponse balance(RpcController controller, BalanceRequest request) throws ServiceException { try { - return ProtobufUtil.toBalanceResponse(master.balance(ProtobufUtil.toBalanceRequest(request))); + return BalanceResponse.newBuilder().setBalancerRan(master.balance( + request.hasForce()? request.getForce(): false)).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 e8092992bac..0db2b32c61f 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,7 +58,6 @@ import org.apache.hadoop.hbase.TableName; 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; @@ -1004,7 +1003,7 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, } @Override - public void preBalance(ObserverContext c, BalanceRequest request) + public void preBalance(ObserverContext c) 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 30492c20826..ed37713d631 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,7 +18,6 @@ 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; @@ -28,7 +27,6 @@ import java.util.Arrays; 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; @@ -130,21 +128,14 @@ public class TestRegionRebalancing { assertRegionsAreBalanced(); // On a balanced cluster, calling balance() should return true - BalanceResponse response = UTIL.getHBaseCluster().getMaster().balance(); - assertTrue(response.isBalancerRan()); - assertEquals(0, response.getMovesCalculated()); - assertEquals(0, response.getMovesExecuted()); + assert(UTIL.getHBaseCluster().getMaster().balance() == true); // 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(); - - response = UTIL.getHBaseCluster().getMaster().balance(); - assertTrue(response.isBalancerRan()); - assertTrue(response.getMovesCalculated() > 0); - assertEquals(response.getMovesCalculated(), response.getMovesExecuted()); + assert(UTIL.getHBaseCluster().getMaster().balance() == true); assertRegionsAreBalanced(); // kill a region server - total of 2 @@ -161,24 +152,14 @@ public class TestRegionRebalancing { UTIL.getHBaseCluster().startRegionServer().getRegionServer().getServerName()); waitOnCrashProcessing(); waitForAllRegionsAssigned(); - - response = UTIL.getHBaseCluster().getMaster().balance(); - assertTrue(response.isBalancerRan()); - assertTrue(response.getMovesCalculated() > 0); - assertEquals(response.getMovesCalculated(), response.getMovesExecuted()); - + assert(UTIL.getHBaseCluster().getMaster().balance() == true); assertRegionsAreBalanced(); for (int i = 0; i < 6; i++){ LOG.info("Adding " + (i + 5) + "th region server"); UTIL.getHBaseCluster().startRegionServer(); } waitForAllRegionsAssigned(); - - response = UTIL.getHBaseCluster().getMaster().balance(); - assertTrue(response.isBalancerRan()); - assertTrue(response.getMovesCalculated() > 0); - assertEquals(response.getMovesCalculated(), response.getMovesExecuted()); - + assert(UTIL.getHBaseCluster().getMaster().balance() == true); 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 e6f4b1a6e6f..23ab43de074 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 class TestAsyncTableGetMultiThreaded { } Thread.sleep(5000); LOG.info("====== Balancing cluster ======"); - admin.balance(BalanceRequest.newBuilder().setIgnoreRegionsInTransition(true).build()); + admin.balance(true); 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 419b81245f1..84c90de4234 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 class TestMultiParallel { @Override public void postBalance(final ObserverContext ctx, - BalanceRequest request, List plans) throws IOException { + 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 52b69db2553..e01c84e2a19 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 class TestSeparateClientZKCluster { } LOG.info("Got master {}", cluster.getMaster().getServerName()); // confirm client access still works - assertTrue(admin.balance(BalanceRequest.defaultInstance()).isBalancerRan()); + assertTrue(admin.balance(false)); } } @@ -278,4 +278,4 @@ public class TestSeparateClientZKCluster { 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 71a7643c1f1..2cab41b6818 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,8 +40,6 @@ import org.apache.hadoop.hbase.NamespaceDescriptor; 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; @@ -691,14 +689,13 @@ public class TestMasterObserver { } @Override - public void preBalance(ObserverContext env, - BalanceRequest request) throws IOException { + public void preBalance(ObserverContext env) + throws IOException { preBalanceCalled = true; } @Override public void postBalance(ObserverContext env, - BalanceRequest request, List plans) throws IOException { postBalanceCalled = true; } @@ -1162,12 +1159,12 @@ public class TestMasterObserver { @Override public void preBalanceRSGroup(ObserverContext ctx, - String groupName, BalanceRequest request) throws IOException { + String groupName) throws IOException { } @Override public void postBalanceRSGroup(ObserverContext ctx, - String groupName, BalanceRequest request, BalanceResponse response) throws IOException { + String groupName, boolean balancerRan) throws IOException { } @Override @@ -1619,7 +1616,7 @@ public class TestMasterObserver { UTIL.waitUntilNoRegionsInTransition(); // now trigger a balance master.balanceSwitch(true); - master.balance(); + boolean balanceRun = 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 deleted file mode 100644 index 730075394fa..00000000000 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterDryRunBalancer.java +++ /dev/null @@ -1,126 +0,0 @@ -/** - * 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 95211a3ffda..fa4c4a5283c 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,7 +28,6 @@ import org.apache.hadoop.hbase.HBaseClassTestRule; 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; @@ -121,7 +120,7 @@ public class TestProcedurePriority { for (Future future : futures) { future.get(1, TimeUnit.MINUTES); } - UTIL.getAdmin().balance(BalanceRequest.newBuilder().setIgnoreRegionsInTransition(true).build()); + UTIL.getAdmin().balance(true); 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 e54e8d6acbe..fd8591e2a75 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,7 +64,6 @@ 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.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; @@ -777,7 +776,7 @@ public class TestAccessController extends SecureTestUtil { AccessTestAction action = new AccessTestAction() { @Override public Object run() throws Exception { - ACCESS_CONTROLLER.preBalance(ObserverContextImpl.createAndPrepare(CP_ENV), BalanceRequest.defaultInstance()); + ACCESS_CONTROLLER.preBalance(ObserverContextImpl.createAndPrepare(CP_ENV)); 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 fb1a060eccc..26507025a3f 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,7 +36,6 @@ import org.apache.hadoop.hbase.TableNameTestRule; 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; @@ -569,7 +568,7 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { verifyAllowed(new AccessTestAction() { @Override public Object run() throws Exception { - ACCESS_CONTROLLER.preBalance(ObserverContextImpl.createAndPrepare(CP_ENV), BalanceRequest.defaultInstance()); + ACCESS_CONTROLLER.preBalance(ObserverContextImpl.createAndPrepare(CP_ENV)); 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 c1da4bbb52b..f8ebe5ed95c 100644 --- a/hbase-shell/src/main/ruby/hbase/admin.rb +++ b/hbase-shell/src/main/ruby/hbase/admin.rb @@ -26,8 +26,6 @@ java_import org.apache.hadoop.hbase.TableName 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 @@ -208,9 +206,8 @@ module Hbase #---------------------------------------------------------------------------------------------- # Requests a cluster balance # Returns true if balancer ran - def balancer(*args) - request = ::Hbase::BalancerUtils.create_balance_request(args) - @admin.balance(request) + def balancer(force) + @admin.balancer(java.lang.Boolean.valueOf(force)) end #---------------------------------------------------------------------------------------------- diff --git a/hbase-shell/src/main/ruby/hbase/balancer_utils.rb b/hbase-shell/src/main/ruby/hbase/balancer_utils.rb deleted file mode 100644 index 9948c0f4745..00000000000 --- a/hbase-shell/src/main/ruby/hbase/balancer_utils.rb +++ /dev/null @@ -1,57 +0,0 @@ -# -# -# 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 6d08dbcfab5..b25219ad399 100644 --- a/hbase-shell/src/main/ruby/hbase/rsgroup_admin.rb +++ b/hbase-shell/src/main/ruby/hbase/rsgroup_admin.rb @@ -18,8 +18,6 @@ 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 @@ -63,9 +61,8 @@ module Hbase #-------------------------------------------------------------------------- # balance a group - def balance_rs_group(group_name, *args) - request = ::Hbase::BalancerUtils.create_balance_request(args) - @admin.balanceRSGroup(group_name, request) + def balance_rs_group(group_name) + @admin.balanceRSGroup(group_name) 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 aff9fa7ce94..a223e055bbf 100644 --- a/hbase-shell/src/main/ruby/shell/commands/balance_rsgroup.rb +++ b/hbase-shell/src/main/ruby/shell/commands/balance_rsgroup.rb @@ -22,32 +22,22 @@ module Shell <<-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, *args) + def command(group_name) # Returns true if balancer was run, otherwise false. - 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}"]) + ret = rsgroup_admin.balance_rs_group(group_name) + if ret + puts 'Ran the balancer.' else - formatter.row(["Balancer did not run. See logs for details."]) + puts "Couldn't run the balancer." end - resp.isBalancerRan + ret 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 d5304e0007f..c4acdc0ecde 100644 --- a/hbase-shell/src/main/ruby/shell/commands/balancer.rb +++ b/hbase-shell/src/main/ruby/shell/commands/balancer.rb @@ -22,32 +22,31 @@ module Shell class Balancer < Command def help <<-EOF -Trigger the cluster balancer. Returns true if balancer ran, otherwise false (Will not run if regions in transition). +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. -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 +WARNING: For experts only. Forcing a balance may do more damage than repair +when assignment is confused Examples: hbase> balancer - hbase> balancer "ignore_rit" - hbase> balancer "dry_run" - hbase> balancer "dry_run", "ignore_rit" + hbase> balancer "force" EOF end - 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."]) + def command(force = nil) + force_balancer = 'false' + if force == 'force' + force_balancer = 'true' + elsif !force.nil? + raise ArgumentError, "Invalid argument #{force}." end - resp.isBalancerRan + did_balancer_run = !!admin.balancer(force_balancer) + formatter.row([did_balancer_run.to_s]) + did_balancer_run 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 26793a12192..19910cac8d9 100644 --- a/hbase-shell/src/test/ruby/hbase/admin_test.rb +++ b/hbase-shell/src/test/ruby/hbase/admin_test.rb @@ -192,13 +192,7 @@ module Hbase did_balancer_run = command(:balancer) assert(did_balancer_run == true) output = capture_stdout { command(:balancer, 'force') } - 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')) + 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 deleted file mode 100644 index 9d70540750e..00000000000 --- a/hbase-shell/src/test/ruby/hbase/balancer_utils_test.rb +++ /dev/null @@ -1,78 +0,0 @@ -# -# -# 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 b305c2e58d5..f7c53000c85 100644 --- a/hbase-shell/src/test/ruby/shell/rsgroup_shell_test.rb +++ b/hbase-shell/src/test/ruby/shell/rsgroup_shell_test.rb @@ -90,8 +90,6 @@ module Hbase # 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 0aa9862c031..fd551b720ff 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,7 +27,6 @@ import java.util.concurrent.Future; 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; @@ -41,7 +40,6 @@ import org.apache.hadoop.hbase.TableExistsException; 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; @@ -788,11 +786,6 @@ public class ThriftAdmin implements Admin { 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");