diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseHbck.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseHbck.java index 8df0504b2a9a..83b53ccba3c3 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseHbck.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseHbck.java @@ -135,10 +135,11 @@ public Map setRegionStateInMeta( } @Override - public List assigns(List encodedRegionNames, boolean override) throws IOException { + public List assigns(List encodedRegionNames, boolean override, boolean force) + throws IOException { try { AssignsResponse response = this.hbck.assigns(rpcControllerFactory.newController(), - RequestConverter.toAssignRegionsRequest(encodedRegionNames, override)); + RequestConverter.toAssignRegionsRequest(encodedRegionNames, override, force)); return response.getPidList(); } catch (ServiceException se) { LOG.debug(toCommaDelimitedString(encodedRegionNames), se); @@ -147,11 +148,11 @@ public List assigns(List encodedRegionNames, boolean override) thr } @Override - public List unassigns(List encodedRegionNames, boolean override) + public List unassigns(List encodedRegionNames, boolean override, boolean force) throws IOException { try { UnassignsResponse response = this.hbck.unassigns(rpcControllerFactory.newController(), - RequestConverter.toUnassignRegionsRequest(encodedRegionNames, override)); + RequestConverter.toUnassignRegionsRequest(encodedRegionNames, override, force)); return response.getPidList(); } catch (ServiceException se) { LOG.debug(toCommaDelimitedString(encodedRegionNames), se); diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Hbck.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Hbck.java index b5ba25058838..6baa876f9387 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Hbck.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Hbck.java @@ -62,19 +62,26 @@ public interface Hbck extends Abortable, Closeable { * good if many Regions to online -- and it will schedule the assigns even in the case where * Master is initializing (as long as the ProcedureExecutor is up). Does NOT call Coprocessor * hooks. - * @param override You need to add the override for case where a region has previously - * been bypassed. When a Procedure has been bypassed, a Procedure will - * have completed but no other Procedure will be able to make progress - * on the target entity (intentionally). This override flag will - * override this fencing mechanism. + * @param override You need to add override for unset of the procedure from + * RegionStateNode without byPassing preTransitCheck + * @param force You need to add force for case where a region has previously been + * bypassed. When a Procedure has been bypassed, a Procedure will have + * completed but no other Procedure will be able to make progress on the + * target entity (intentionally). Skips preTransitCheck only when + * selected along with override option * @param encodedRegionNames Region encoded names; e.g. 1588230740 is the hard-coded encoding for * hbase:meta region and de00010733901a05f5a2a3a382e27dd4 is an example * of what a random user-space encoded Region name looks like. */ - List assigns(List encodedRegionNames, boolean override) throws IOException; + List assigns(List encodedRegionNames, boolean override, boolean force) + throws IOException; + + default List assigns(List encodedRegionNames, boolean override) throws IOException { + return assigns(encodedRegionNames, override, true); + } default List assigns(List encodedRegionNames) throws IOException { - return assigns(encodedRegionNames, false); + return assigns(encodedRegionNames, false, false); } /** @@ -82,19 +89,27 @@ default List assigns(List encodedRegionNames) throws IOException { * at a time -- good if many Regions to offline -- and it will schedule the assigns even in the * case where Master is initializing (as long as the ProcedureExecutor is up). Does NOT call * Coprocessor hooks. - * @param override You need to add the override for case where a region has previously - * been bypassed. When a Procedure has been bypassed, a Procedure will - * have completed but no other Procedure will be able to make progress - * on the target entity (intentionally). This override flag will - * override this fencing mechanism. + * @param override You need to add override for unset of the procedure from + * RegionStateNode without byPassing preTransitCheck + * @param force You need to add force for case where a region has previously been + * bypassed. When a Procedure has been bypassed, a Procedure will have + * completed but no other Procedure will be able to make progress on the + * target entity (intentionally). Skips preTransitCheck only when + * selected along with override option * @param encodedRegionNames Region encoded names; e.g. 1588230740 is the hard-coded encoding for * hbase:meta region and de00010733901a05f5a2a3a382e27dd4 is an example * of what a random user-space encoded Region name looks like. */ - List unassigns(List encodedRegionNames, boolean override) throws IOException; + List unassigns(List encodedRegionNames, boolean override, boolean force) + throws IOException; + + default List unassigns(List encodedRegionNames, boolean override) + throws IOException { + return unassigns(encodedRegionNames, override, true); + } default List unassigns(List encodedRegionNames) throws IOException { - return unassigns(encodedRegionNames, false); + return unassigns(encodedRegionNames, false, true); } /** 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 377b46494633..ce12aaea0d24 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 @@ -1590,17 +1590,17 @@ private static List toProtoServerNames(List // HBCK2 public static MasterProtos.AssignsRequest toAssignRegionsRequest(List encodedRegionNames, - boolean override) { + boolean override, boolean force) { MasterProtos.AssignsRequest.Builder b = MasterProtos.AssignsRequest.newBuilder(); return b.addAllRegion(toEncodedRegionNameRegionSpecifiers(encodedRegionNames)) - .setOverride(override).build(); + .setOverride(override).setForce(force).build(); } public static MasterProtos.UnassignsRequest - toUnassignRegionsRequest(List encodedRegionNames, boolean override) { + toUnassignRegionsRequest(List encodedRegionNames, boolean override, boolean force) { MasterProtos.UnassignsRequest.Builder b = MasterProtos.UnassignsRequest.newBuilder(); return b.addAllRegion(toEncodedRegionNameRegionSpecifiers(encodedRegionNames)) - .setOverride(override).build(); + .setOverride(override).setForce(force).build(); } public static MasterProtos.ScheduleServerCrashProcedureRequest diff --git a/hbase-protocol-shaded/src/main/protobuf/server/master/Master.proto b/hbase-protocol-shaded/src/main/protobuf/server/master/Master.proto index b1e750f4d920..a8adaa27453f 100644 --- a/hbase-protocol-shaded/src/main/protobuf/server/master/Master.proto +++ b/hbase-protocol-shaded/src/main/protobuf/server/master/Master.proto @@ -1302,6 +1302,7 @@ message SetRegionStateInMetaResponse { message AssignsRequest { repeated RegionSpecifier region = 1; optional bool override = 2 [default = false]; + optional bool force = 3 [default = false]; } /** Like Admin's AssignRegionResponse except it can @@ -1317,6 +1318,7 @@ message AssignsResponse { message UnassignsRequest { repeated RegionSpecifier region = 1; optional bool override = 2 [default = false]; + optional bool force= 3 [default = false]; } /** Like Admin's UnassignRegionResponse except it can 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 6d330d6eb791..1da8e03d179e 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 @@ -2714,6 +2714,7 @@ public MasterProtos.AssignsResponse assigns(RpcController controller, MasterProtos.AssignsResponse.Builder responseBuilder = MasterProtos.AssignsResponse.newBuilder(); final boolean override = request.getOverride(); + final boolean force = request.getForce(); LOG.info("{} assigns, override={}", server.getClientIdAuditPrefix(), override); for (HBaseProtos.RegionSpecifier rs : request.getRegionList()) { final RegionInfo info = getRegionInfo(rs); @@ -2721,7 +2722,7 @@ public MasterProtos.AssignsResponse assigns(RpcController controller, LOG.info("Unknown region {}", rs); continue; } - responseBuilder.addPid(Optional.ofNullable(am.createOneAssignProcedure(info, override)) + responseBuilder.addPid(Optional.ofNullable(am.createOneAssignProcedure(info, override, force)) .map(pe::submitProcedure).orElse(Procedure.NO_PROC_ID)); } return responseBuilder.build(); @@ -2741,6 +2742,7 @@ public MasterProtos.UnassignsResponse unassigns(RpcController controller, MasterProtos.UnassignsResponse.Builder responseBuilder = MasterProtos.UnassignsResponse.newBuilder(); final boolean override = request.getOverride(); + final boolean force = request.getForce(); LOG.info("{} unassigns, override={}", server.getClientIdAuditPrefix(), override); for (HBaseProtos.RegionSpecifier rs : request.getRegionList()) { final RegionInfo info = getRegionInfo(rs); @@ -2748,8 +2750,9 @@ public MasterProtos.UnassignsResponse unassigns(RpcController controller, LOG.info("Unknown region {}", rs); continue; } - responseBuilder.addPid(Optional.ofNullable(am.createOneUnassignProcedure(info, override)) - .map(pe::submitProcedure).orElse(Procedure.NO_PROC_ID)); + responseBuilder + .addPid(Optional.ofNullable(am.createOneUnassignProcedure(info, override, force)) + .map(pe::submitProcedure).orElse(Procedure.NO_PROC_ID)); } return responseBuilder.build(); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java index 49cf29ee61d9..9cee9f87ce2f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java @@ -765,11 +765,14 @@ private void preTransitCheck(RegionStateNode regionNode, RegionState.State[] exp * @param override If false, check RegionState is appropriate for assign; if not throw exception. */ private TransitRegionStateProcedure createAssignProcedure(RegionInfo regionInfo, ServerName sn, - boolean override) throws IOException { + boolean override, boolean force) throws IOException { RegionStateNode regionNode = regionStates.getOrCreateRegionStateNode(regionInfo); regionNode.lock(); try { if (override) { + if (!force) { + preTransitCheck(regionNode, STATES_EXPECTED_ON_ASSIGN); + } if (regionNode.getProcedure() != null) { regionNode.unsetProcedure(regionNode.getProcedure()); } @@ -787,7 +790,7 @@ private TransitRegionStateProcedure createAssignProcedure(RegionInfo regionInfo, /** * Create an assign TransitRegionStateProcedure. Does NO checking of RegionState. Presumes * appriopriate state ripe for assign. - * @see #createAssignProcedure(RegionInfo, ServerName, boolean) + * @see #createAssignProcedure(RegionInfo, ServerName, boolean, boolean) */ private TransitRegionStateProcedure createAssignProcedure(RegionStateNode regionNode, ServerName targetServer) { @@ -801,7 +804,7 @@ private TransitRegionStateProcedure createAssignProcedure(RegionStateNode region } public long assign(RegionInfo regionInfo, ServerName sn) throws IOException { - TransitRegionStateProcedure proc = createAssignProcedure(regionInfo, sn, false); + TransitRegionStateProcedure proc = createAssignProcedure(regionInfo, sn, false, false); ProcedureSyncWait.submitAndWaitProcedure(master.getMasterProcedureExecutor(), proc); return proc.getProcId(); } @@ -817,7 +820,7 @@ public long assign(RegionInfo regionInfo) throws IOException { */ public Future assignAsync(RegionInfo regionInfo, ServerName sn) throws IOException { return ProcedureSyncWait.submitProcedure(master.getMasterProcedureExecutor(), - createAssignProcedure(regionInfo, sn, false)); + createAssignProcedure(regionInfo, sn, false, false)); } /** @@ -961,10 +964,11 @@ static int compare(TransitRegionStateProcedure left, TransitRegionStateProcedure * method is called from HBCK2. * @return an assign or null */ - public TransitRegionStateProcedure createOneAssignProcedure(RegionInfo ri, boolean override) { + public TransitRegionStateProcedure createOneAssignProcedure(RegionInfo ri, boolean override, + boolean force) { TransitRegionStateProcedure trsp = null; try { - trsp = createAssignProcedure(ri, null, override); + trsp = createAssignProcedure(ri, null, override, force); } catch (IOException ioe) { LOG.info( "Failed {} assign, override={}" @@ -978,12 +982,16 @@ public TransitRegionStateProcedure createOneAssignProcedure(RegionInfo ri, boole * Create one TransitRegionStateProcedure to unassign a region. This method is called from HBCK2. * @return an unassign or null */ - public TransitRegionStateProcedure createOneUnassignProcedure(RegionInfo ri, boolean override) { + public TransitRegionStateProcedure createOneUnassignProcedure(RegionInfo ri, boolean override, + boolean force) { RegionStateNode regionNode = regionStates.getOrCreateRegionStateNode(ri); TransitRegionStateProcedure trsp = null; regionNode.lock(); try { if (override) { + if (!force) { + preTransitCheck(regionNode, STATES_EXPECTED_ON_UNASSIGN_OR_MOVE); + } if (regionNode.getProcedure() != null) { regionNode.unsetProcedure(regionNode.getProcedure()); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TruncateRegionProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TruncateRegionProcedure.java index 9730391baf22..83722d6c1dca 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TruncateRegionProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TruncateRegionProcedure.java @@ -210,10 +210,10 @@ public TableOperationType getTableOperationType() { private TransitRegionStateProcedure createUnAssignProcedures(MasterProcedureEnv env) throws IOException { - return env.getAssignmentManager().createOneUnassignProcedure(getRegion(), true); + return env.getAssignmentManager().createOneUnassignProcedure(getRegion(), true, true); } private TransitRegionStateProcedure createAssignProcedures(MasterProcedureEnv env) { - return env.getAssignmentManager().createOneAssignProcedure(getRegion(), true); + return env.getAssignmentManager().createOneAssignProcedure(getRegion(), true, true); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHbck.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHbck.java index 406b25fed4e0..360641e64b7d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHbck.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHbck.java @@ -250,11 +250,20 @@ public void testAssigns() throws Exception { for (long pid : pids) { assertEquals(Procedure.NO_PROC_ID, pid); } - // If we pass override, then we should be able to unassign EVEN THOUGH Regions already + // Rerun the unassign with override. Should fail for all Regions since they already + // unassigned; failed + // unassign will manifest as all pids being -1 (ever since HBASE-24885). + pids = hbck.unassigns( + regions.stream().map(RegionInfo::getEncodedName).collect(Collectors.toList()), true, false); + waitOnPids(pids); + for (long pid : pids) { + assertEquals(Procedure.NO_PROC_ID, pid); + } + // If we pass force, then we should be able to unassign EVEN THOUGH Regions already // unassigned.... makes for a mess but operator might want to do this at an extreme when // doing fixup of broke cluster. pids = hbck.unassigns( - regions.stream().map(RegionInfo::getEncodedName).collect(Collectors.toList()), true); + regions.stream().map(RegionInfo::getEncodedName).collect(Collectors.toList()), true, true); waitOnPids(pids); for (long pid : pids) { assertNotEquals(Procedure.NO_PROC_ID, pid); @@ -283,6 +292,12 @@ public void testAssigns() throws Exception { LOG.info("RS: {}", rs.toString()); assertTrue(rs.toString(), rs.isOpened()); } + // Rerun the assign with override. Should fail for all Regions since they already assigned + pids = hbck.assigns( + regions.stream().map(RegionInfo::getEncodedName).collect(Collectors.toList()), true, false); + for (long pid : pids) { + assertEquals(Procedure.NO_PROC_ID, pid); + } // What happens if crappy region list passed? pids = hbck.assigns( Arrays.stream(new String[] { "a", "some rubbish name" }).collect(Collectors.toList())); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionBypass.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionBypass.java index 61520873240c..8295da82f49c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionBypass.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionBypass.java @@ -125,7 +125,7 @@ public void testBypass() throws IOException, InterruptedException { .getMasterProcedureExecutor().getActiveProcIds().isEmpty()); // Now assign with the override flag. for (RegionInfo ri : regions) { - TEST_UTIL.getHbck().assigns(Arrays. asList(ri.getEncodedName()), true); + TEST_UTIL.getHbck().assigns(Arrays. asList(ri.getEncodedName()), true, true); } TEST_UTIL.waitFor(60000, () -> TEST_UTIL.getHBaseCluster().getMaster() .getMasterProcedureExecutor().getActiveProcIds().isEmpty());