From dd6974fda8c7feba2f7031a1d3ccf7e1f0bff8f0 Mon Sep 17 00:00:00 2001 From: bsglz <18031031@qq.com> Date: Wed, 16 Sep 2020 11:05:03 +0800 Subject: [PATCH] HBASE-24875 Remove the force param for unassign since it dose not take effect any more (#2254) Modified compared to main branch to deprecate obviated MasterObserver interface methods instead of remove them. Signed-off-by: Sean Busbey (cherry picked from commit c5ca191921cad9100fb47aa57348571b3ddc06ad) Conflicts: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AdminOverAsyncAdmin.java hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdmin.java hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java --- .../org/apache/hadoop/hbase/client/Admin.java | 16 ++++++++-- .../hadoop/hbase/client/AsyncAdmin.java | 13 +++++++- .../hadoop/hbase/client/AsyncHBaseAdmin.java | 4 +-- .../hadoop/hbase/client/HBaseAdmin.java | 4 +-- .../hbase/client/RawAsyncHBaseAdmin.java | 4 +-- .../shaded/protobuf/RequestConverter.java | 4 +-- .../src/main/protobuf/Master.proto | 1 + .../src/main/protobuf/MasterProcedure.proto | 1 + .../hbase/coprocessor/MasterObserver.java | 30 +++++++++++++++++-- .../hbase/master/MasterCoprocessorHost.java | 9 +++--- .../hbase/master/MasterRpcServices.java | 7 ++--- .../master/assignment/UnassignProcedure.java | 6 ---- .../security/access/AccessController.java | 4 +-- .../hbase/coprocessor/TestMasterObserver.java | 4 +-- .../security/access/TestAccessController.java | 2 +- .../access/TestWithDisabledAuthorization.java | 3 +- hbase-shell/src/main/ruby/hbase/admin.rb | 5 ++-- .../src/main/ruby/shell/commands/unassign.rb | 10 ++----- .../hbase/thrift2/client/ThriftAdmin.java | 3 +- 19 files changed, 83 insertions(+), 47 deletions(-) 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 e59c6cb94d5..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 @@ -1167,6 +1167,13 @@ public interface Admin extends Abortable, Closeable { */ void assign(byte[] regionName) throws IOException; + /** + * Unassign a Region. + * @param regionName Region name to assign. + * @throws IOException if a remote or network exception occurs + */ + void unassign(byte[] regionName) throws IOException; + /** * Unassign a region from current hosting regionserver. Region will then be assigned to a * regionserver chosen at random. Region could be reassigned back to the same server. Use {@link @@ -1176,9 +1183,14 @@ public interface Admin extends Abortable, Closeable { * @param force If true, force unassign (Will remove region from regions-in-transition too if * present. If results in double assignment use hbck -fix to resolve. To be used by experts). * @throws IOException if a remote or network exception occurs + * @deprecated since 2.4.0 and will be removed in 4.0.0. Use {@link #unassign(byte[])} + * instead. + * @see HBASE-24875 */ - void unassign(byte[] regionName, boolean force) - throws IOException; + @Deprecated + default void unassign(byte[] regionName, boolean force) throws IOException { + unassign(regionName); + } /** * Offline specified region from master's in-memory state. It will not attempt to reassign the 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 27d15533d75..5e68ea8c1e2 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 @@ -588,6 +588,11 @@ public interface AsyncAdmin { */ CompletableFuture assign(byte[] regionName); + /** + * @param regionName Encoded or full name of region to unassign. + */ + CompletableFuture unassign(byte[] regionName); + /** * Unassign a region from current hosting regionserver. Region will then be assigned to a * regionserver chosen at random. Region could be reassigned back to the same server. Use @@ -597,8 +602,14 @@ public interface AsyncAdmin { * @param forcible If true, force unassign (Will remove region from regions-in-transition too if * present. If results in double assignment use hbck -fix to resolve. To be used by * experts). + * @deprecated since 2.4.0 and will be removed in 4.0.0. Use {@link #unassign(byte[])} + * instead. + * @see HBASE-24875 */ - CompletableFuture unassign(byte[] regionName, boolean forcible); + @Deprecated + default CompletableFuture unassign(byte[] regionName, boolean forcible) { + return unassign(regionName); + } /** * Offline specified region from master's in-memory state. It will not attempt to reassign the 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 7c51ed0a74b..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 @@ -368,8 +368,8 @@ class AsyncHBaseAdmin implements AsyncAdmin { } @Override - public CompletableFuture unassign(byte[] regionName, boolean forcible) { - return wrap(rawAdmin.unassign(regionName, forcible)); + public CompletableFuture unassign(byte[] regionName) { + return wrap(rawAdmin.unassign(regionName)); } @Override 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 b652143d50f..b643d9422bb 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 @@ -1438,14 +1438,14 @@ public class HBaseAdmin implements Admin { } @Override - public void unassign(final byte [] regionName, final boolean force) throws IOException { + public void unassign(final byte [] regionName) throws IOException { final byte[] toBeUnassigned = getRegionName(regionName); executeCallable(new MasterCallable(getConnection(), getRpcControllerFactory()) { @Override protected Void rpcCall() throws Exception { setPriority(regionName); UnassignRegionRequest request = - RequestConverter.buildUnassignRegionRequest(toBeUnassigned, force); + RequestConverter.buildUnassignRegionRequest(toBeUnassigned); master.unassignRegion(getRpcController(), request); return null; } 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 7f939141bec..9c9918e054d 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 @@ -1531,7 +1531,7 @@ class RawAsyncHBaseAdmin implements AsyncAdmin { } @Override - public CompletableFuture unassign(byte[] regionName, boolean forcible) { + public CompletableFuture unassign(byte[] regionName) { CompletableFuture future = new CompletableFuture<>(); addListener(getRegionInfo(regionName), (regionInfo, err) -> { if (err != null) { @@ -1542,7 +1542,7 @@ class RawAsyncHBaseAdmin implements AsyncAdmin { this. newMasterCaller().priority(regionInfo.getTable()) .action(((controller, stub) -> this . call(controller, stub, - RequestConverter.buildUnassignRegionRequest(regionInfo.getRegionName(), forcible), + RequestConverter.buildUnassignRegionRequest(regionInfo.getRegionName()), (s, c, req, done) -> s.unassignRegion(c, req, done), resp -> null))) .call(), (ret, err2) -> { 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 825e914ed32..f435f56ed9f 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 @@ -1327,14 +1327,12 @@ public final class RequestConverter { * Creates a protocol buffer UnassignRegionRequest * * @param regionName - * @param force * @return an UnassignRegionRequest */ public static UnassignRegionRequest buildUnassignRegionRequest( - final byte [] regionName, final boolean force) { + final byte [] regionName) { UnassignRegionRequest.Builder builder = UnassignRegionRequest.newBuilder(); builder.setRegion(buildRegionSpecifier(RegionSpecifierType.REGION_NAME,regionName)); - builder.setForce(force); return builder.build(); } diff --git a/hbase-protocol-shaded/src/main/protobuf/Master.proto b/hbase-protocol-shaded/src/main/protobuf/Master.proto index 66b175c3085..ac1e8db08f9 100644 --- a/hbase-protocol-shaded/src/main/protobuf/Master.proto +++ b/hbase-protocol-shaded/src/main/protobuf/Master.proto @@ -109,6 +109,7 @@ message AssignRegionResponse { message UnassignRegionRequest { required RegionSpecifier region = 1; + // This parameter is ignored optional bool force = 2 [default = false]; } diff --git a/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto b/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto index 1cc93878ce1..c852bf326d0 100644 --- a/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto +++ b/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto @@ -348,6 +348,7 @@ message UnassignRegionStateData { // This is the server currently hosting the Region, the // server we will send the unassign rpc too. optional ServerName hosting_server = 5; + // This parameter is ignored optional bool force = 4 [default = false]; optional bool remove_after_unassigning = 6 [default = false]; // Current attempt index used for expotential backoff when stuck 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 4ea80de1121..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 @@ -507,26 +507,52 @@ public interface MasterObserver { * @param ctx the environment to interact with the framework and master * @param regionInfo * @param force whether to force unassignment or not + * @deprecated in 2.4.0. replaced by preUnassign(ctx, regionInfo). removed in hbase 3. + * until then safe to either leave implementation here or move it + * to the new method. default impl of that method calls this one. */ default void preUnassign(final ObserverContext ctx, final RegionInfo regionInfo, final boolean force) throws IOException {} + /** + * Called prior to unassigning a given region. + * @param ctx the environment to interact with the framework and master + * @param regionInfo + */ + default void preUnassign(final ObserverContext ctx, + final RegionInfo regionInfo) throws IOException { + preUnassign(ctx, regionInfo, false); + } + /** * Called after the region unassignment has been requested. * @param ctx the environment to interact with the framework and master * @param regionInfo * @param force whether to force unassignment or not + * @deprecated in 2.4.0. replaced by postUnassign(ctx, regionInfo). removed in hbase 3. + * until then safe to either leave implementation here or move it + * to the new method. default impl of that method calls this one. */ default void postUnassign(final ObserverContext ctx, final RegionInfo regionInfo, final boolean force) throws IOException {} + /** + * Called after the region unassignment has been requested. + * @param ctx the environment to interact with the framework and master + * @param regionInfo + */ + default void postUnassign(final ObserverContext ctx, + final RegionInfo regionInfo) throws IOException { + postUnassign(ctx, regionInfo, false); + } + /** * Called prior to marking a given region as offline. * @param ctx the environment to interact with the framework and master * @param regionInfo */ default void preRegionOffline(final ObserverContext ctx, - final RegionInfo regionInfo) throws IOException {} + final RegionInfo regionInfo) throws IOException {} /** * Called after the region has been marked offline. @@ -534,7 +560,7 @@ public interface MasterObserver { * @param regionInfo */ default void postRegionOffline(final ObserverContext ctx, - final RegionInfo regionInfo) throws IOException {} + final RegionInfo regionInfo) throws IOException {} /** * Called prior to requesting rebalancing of the cluster regions, though after 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 2a29c72844b..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 @@ -681,21 +681,20 @@ public class MasterCoprocessorHost }); } - public void preUnassign(final RegionInfo regionInfo, final boolean force) - throws IOException { + public void preUnassign(final RegionInfo regionInfo) throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { - observer.preUnassign(this, regionInfo, force); + observer.preUnassign(this, regionInfo); } }); } - public void postUnassign(final RegionInfo regionInfo, final boolean force) throws IOException { + public void postUnassign(final RegionInfo regionInfo) throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { - observer.postUnassign(this, regionInfo, force); + observer.postUnassign(this, regionInfo); } }); } 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 3a8719f0beb..76a2516d785 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 @@ -1679,7 +1679,6 @@ public class MasterRpcServices extends RSRpcServices implements try { final byte [] regionName = req.getRegion().getValue().toByteArray(); RegionSpecifierType type = req.getRegion().getType(); - final boolean force = req.getForce(); UnassignRegionResponse urr = UnassignRegionResponse.newBuilder().build(); master.checkInitialized(); @@ -1699,13 +1698,13 @@ public class MasterRpcServices extends RSRpcServices implements RegionInfo hri = pair.getFirst(); if (master.cpHost != null) { - master.cpHost.preUnassign(hri, force); + master.cpHost.preUnassign(hri); } LOG.debug(master.getClientIdAuditPrefix() + " unassign " + hri.getRegionNameAsString() - + " in current location if it is online and reassign.force=" + force); + + " in current location if it is online"); master.getAssignmentManager().unassign(hri); if (master.cpHost != null) { - master.cpHost.postUnassign(hri, force); + master.cpHost.postUnassign(hri); } return urr; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/UnassignProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/UnassignProcedure.java index 5b9eaddf350..b9f880d364a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/UnassignProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/UnassignProcedure.java @@ -48,8 +48,6 @@ public class UnassignProcedure extends RegionTransitionProcedure { protected volatile ServerName destinationServer; - private boolean force; - private boolean removeAfterUnassigning; public UnassignProcedure() { @@ -80,9 +78,6 @@ public class UnassignProcedure extends RegionTransitionProcedure { if (this.destinationServer != null) { state.setDestinationServer(ProtobufUtil.toServerName(destinationServer)); } - if (force) { - state.setForce(true); - } if (removeAfterUnassigning) { state.setRemoveAfterUnassigning(true); } @@ -98,7 +93,6 @@ public class UnassignProcedure extends RegionTransitionProcedure { setTransitionState(state.getTransitionState()); setRegionInfo(ProtobufUtil.toRegionInfo(state.getRegionInfo())); this.hostingServer = ProtobufUtil.toServerName(state.getHostingServer()); - force = state.getForce(); if (state.hasDestinationServer()) { this.destinationServer = ProtobufUtil.toServerName(state.getDestinationServer()); } 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 1f53e2784bf..0d5ac23fc12 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 @@ -983,8 +983,8 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, } @Override - public void preUnassign(ObserverContext c, RegionInfo regionInfo, - boolean force) throws IOException { + public void preUnassign(ObserverContext c, RegionInfo regionInfo) + throws IOException { requirePermission(c, "unassign", regionInfo.getTable(), null, null, Action.ADMIN); } 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 813fb290831..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 @@ -650,13 +650,13 @@ public class TestMasterObserver { @Override public void preUnassign(ObserverContext env, - final RegionInfo regionInfo, final boolean force) throws IOException { + final RegionInfo regionInfo) throws IOException { preUnassignCalled = true; } @Override public void postUnassign(ObserverContext env, - final RegionInfo regionInfo, final boolean force) throws IOException { + final RegionInfo regionInfo) throws IOException { postUnassignCalled = true; } 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 1d6af1afaeb..40823a0c762 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 @@ -668,7 +668,7 @@ public class TestAccessController extends SecureTestUtil { AccessTestAction action = new AccessTestAction() { @Override public Object run() throws Exception { - ACCESS_CONTROLLER.preUnassign(ObserverContextImpl.createAndPrepare(CP_ENV), hri, false); + ACCESS_CONTROLLER.preUnassign(ObserverContextImpl.createAndPrepare(CP_ENV), hri); 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 eb7dcc644fd..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 @@ -559,8 +559,7 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { @Override public Object run() throws Exception { HRegionInfo region = new HRegionInfo(testTable.getTableName()); - ACCESS_CONTROLLER.preUnassign(ObserverContextImpl.createAndPrepare(CP_ENV), region, - true); + ACCESS_CONTROLLER.preUnassign(ObserverContextImpl.createAndPrepare(CP_ENV), region); 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 5e9b7a95c45..cc5f3637700 100644 --- a/hbase-shell/src/main/ruby/hbase/admin.rb +++ b/hbase-shell/src/main/ruby/hbase/admin.rb @@ -558,8 +558,9 @@ module Hbase #---------------------------------------------------------------------------------------------- # Unassign a region - def unassign(region_name, force) - @admin.unassign(region_name.to_java_bytes, java.lang.Boolean.valueOf(force)) + # the force parameter is deprecated, if it is specified, will be ignored. + def unassign(region_name, force = nil) + @admin.unassign(region_name.to_java_bytes) end #---------------------------------------------------------------------------------------------- diff --git a/hbase-shell/src/main/ruby/shell/commands/unassign.rb b/hbase-shell/src/main/ruby/shell/commands/unassign.rb index 0e6aa6141a1..8bd74e82746 100644 --- a/hbase-shell/src/main/ruby/shell/commands/unassign.rb +++ b/hbase-shell/src/main/ruby/shell/commands/unassign.rb @@ -23,23 +23,19 @@ module Shell def help <<-EOF Unassign a region. It could be executed only when region in expected state(OPEN). -Pass 'true' to force the unassignment ('force' will clear all in-memory state in -master before the reassign. If results in double assignment use hbck -fix to resolve. -To be used by experts). In addition, you can use "unassigns" command available on HBCK2 tool to skip the state check. (For more info on HBCK2: https://github.com/apache/hbase-operator-tools/blob/master/hbase-hbck2/README.md) Use with caution. For experts only. Examples: hbase> unassign 'REGIONNAME' - hbase> unassign 'REGIONNAME', true hbase> unassign 'ENCODED_REGIONNAME' - hbase> unassign 'ENCODED_REGIONNAME', true EOF end - def command(region_name, force = 'false') - admin.unassign(region_name, force) + # the force parameter is deprecated, if it is specified, will be ignored. + def command(region_name, force = nil) + admin.unassign(region_name) end end end diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java index d4bc5698259..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 @@ -761,9 +761,8 @@ public class ThriftAdmin implements Admin { } @Override - public void unassign(byte[] regionName, boolean force) { + public void unassign(byte[] regionName) { throw new NotImplementedException("unassign not supported in ThriftAdmin"); - } @Override