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 <busbey@apache.org>
(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

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 <code>true</code>, 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 <a href="https://issues.apache.org/jira/browse/HBASE-24875">HBASE-24875</a>
    */
-  void unassign(byte[] regionName, boolean force)
+  @Deprecated
-      throws IOException;
+  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

hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java

@@ -588,6 +588,11 @@ public interface AsyncAdmin {
    */
   CompletableFuture<Void> assign(byte[] regionName);
 
+  /**
+   * @param regionName Encoded or full name of region to unassign.
+   */
+  CompletableFuture<Void> 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 <a href="https://issues.apache.org/jira/browse/HBASE-24875">HBASE-24875</a>
    */
-  CompletableFuture<Void> unassign(byte[] regionName, boolean forcible);
+  @Deprecated
+  default CompletableFuture<Void> unassign(byte[] regionName, boolean forcible) {
+    return unassign(regionName);
+  }
 
   /**
    * Offline specified region from master's in-memory state. It will not attempt to reassign the

hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java

@@ -368,8 +368,8 @@ class AsyncHBaseAdmin implements AsyncAdmin {
   }
 
   @Override
-  public CompletableFuture<Void> unassign(byte[] regionName, boolean forcible) {
+  public CompletableFuture<Void> unassign(byte[] regionName) {
-    return wrap(rawAdmin.unassign(regionName, forcible));
+    return wrap(rawAdmin.unassign(regionName));
   }
 
   @Override

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<Void>(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;
       }

hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java

@@ -1531,7 +1531,7 @@ class RawAsyncHBaseAdmin implements AsyncAdmin {
   }
 
   @Override
-  public CompletableFuture<Void> unassign(byte[] regionName, boolean forcible) {
+  public CompletableFuture<Void> unassign(byte[] regionName) {
     CompletableFuture<Void> future = new CompletableFuture<>();
     addListener(getRegionInfo(regionName), (regionInfo, err) -> {
       if (err != null) {
@@ -1542,7 +1542,7 @@ class RawAsyncHBaseAdmin implements AsyncAdmin {
         this.<Void> newMasterCaller().priority(regionInfo.getTable())
           .action(((controller, stub) -> this
             .<UnassignRegionRequest, UnassignRegionResponse, Void> 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) -> {

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();
   }
 

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];
 }
 

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

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<MasterCoprocessorEnvironment> 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<MasterCoprocessorEnvironment> 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<MasterCoprocessorEnvironment> 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<MasterCoprocessorEnvironment> 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<MasterCoprocessorEnvironment> 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<MasterCoprocessorEnvironment> ctx,
-      final RegionInfo regionInfo) throws IOException {}
+    final RegionInfo regionInfo) throws IOException {}
 
   /**
    * Called prior to requesting rebalancing of the cluster regions, though after

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)
+  public void preUnassign(final RegionInfo regionInfo) throws IOException {
-      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);
       }
     });
   }

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;

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());
     }

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<MasterCoprocessorEnvironment> c, RegionInfo regionInfo,
+  public void preUnassign(ObserverContext<MasterCoprocessorEnvironment> c, RegionInfo regionInfo)
-      boolean force) throws IOException {
+      throws IOException {
     requirePermission(c, "unassign",
         regionInfo.getTable(), null, null, Action.ADMIN);
   }

hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java

@@ -650,13 +650,13 @@ public class TestMasterObserver {
 
     @Override
     public void preUnassign(ObserverContext<MasterCoprocessorEnvironment> env,
-        final RegionInfo regionInfo, final boolean force) throws IOException {
+        final RegionInfo regionInfo) throws IOException {
       preUnassignCalled = true;
     }
 
     @Override
     public void postUnassign(ObserverContext<MasterCoprocessorEnvironment> env,
-        final RegionInfo regionInfo, final boolean force) throws IOException {
+        final RegionInfo regionInfo) throws IOException {
       postUnassignCalled = true;
     }
 

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;
       }
     };

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,
+        ACCESS_CONTROLLER.preUnassign(ObserverContextImpl.createAndPrepare(CP_ENV), region);
-          true);
         return null;
       }
     }, SUPERUSER, USER_ADMIN, USER_RW, USER_RO, USER_OWNER, USER_CREATE, USER_QUAL, USER_NONE);

hbase-shell/src/main/ruby/hbase/admin.rb

@@ -558,8 +558,9 @@ module Hbase
 
     #----------------------------------------------------------------------------------------------
     # Unassign a region
-    def unassign(region_name, force)
+    # the force parameter is deprecated, if it is specified, will be ignored.
-      @admin.unassign(region_name.to_java_bytes, java.lang.Boolean.valueOf(force))
+    def unassign(region_name, force = nil)
+      @admin.unassign(region_name.to_java_bytes)
     end
 
     #----------------------------------------------------------------------------------------------

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')
+      # the force parameter is deprecated, if it is specified, will be ignored.
-        admin.unassign(region_name, force)
+      def command(region_name, force = nil)
+        admin.unassign(region_name)
       end
     end
   end

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