diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/RegionsStateInvalidException.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/RegionsStateInvalidException.java new file mode 100644 index 0000000000..bbb3c28e0e --- /dev/null +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/RegionsStateInvalidException.java @@ -0,0 +1,38 @@ +/** + * + * 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; + +import java.io.IOException; + +import org.apache.yetus.audience.InterfaceAudience; +import org.apache.yetus.audience.InterfaceStability; + +/** + * This exception is thrown by the master when a region server reports invalid + * region state that master currently doesn't handle. That can mean there's + * double assignment going on, so we kill the RS to narrow the data loss window. + */ +@SuppressWarnings("serial") +@InterfaceAudience.Private +@InterfaceStability.Stable +public class RegionsStateInvalidException extends IOException { + public RegionsStateInvalidException(String message) { + super(message); + } +} diff --git a/hbase-protocol-shaded/src/main/protobuf/Admin.proto b/hbase-protocol-shaded/src/main/protobuf/Admin.proto index c622d589c6..131342bdd5 100644 --- a/hbase-protocol-shaded/src/main/protobuf/Admin.proto +++ b/hbase-protocol-shaded/src/main/protobuf/Admin.proto @@ -274,6 +274,7 @@ message ExecuteProceduresRequest { repeated OpenRegionRequest open_region = 1; repeated CloseRegionRequest close_region = 2; repeated RemoteProcedureRequest proc = 3; + optional uint64 master_rpc_deadline = 4; } message ExecuteProceduresResponse { 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 c12f806f8d..fb724cb847 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 @@ -1073,6 +1073,9 @@ public class AssignmentManager { // This is possible as a region server has just closed a region but the region server // report is generated before the closing, but arrive after the closing. Make sure there // is some elapsed time so less false alarms. + // TODO: we should deal with this situation because it is very likely to mean double + // assignment is happening. We used to kill the server, However it was removed + // in HBASE-21421 due to various other races resulting in RS deaths. if (!regionNode.getRegionLocation().equals(serverName) && diff > 1000) { LOG.warn("{} reported OPEN on server={} but state has otherwise", regionNode, serverName); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java index 2d022b7bad..967ead7be8 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java @@ -23,6 +23,7 @@ import static org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureP import edu.umd.cs.findbugs.annotations.Nullable; import java.io.IOException; import org.apache.hadoop.hbase.HBaseIOException; +import org.apache.hadoop.hbase.RegionsStateInvalidException; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.RegionInfo; @@ -392,9 +393,13 @@ public class TransitRegionStateProcedure // from '<' to '<='. So here we still need to check whether the serverName // matches, to determine whether this is a retry when the openSeqNum is not changed. if (!regionNode.getRegionLocation().equals(serverName)) { - LOG.warn("Received report {} transition from {} for {}, pid={} but the region is not on it," + - " should be a retry, ignore", TransitionCode.OPENED, serverName, regionNode, getProcId()); - return; + LOG.warn("Received report {} transition from {} for {}, pid={} but the region is not on it," + + " killing RS", TransitionCode.OPENED, serverName, regionNode, getProcId()); + // We may be killing an innocent RS due to some network race condition (to fix that, we'd + // need HBASE-21864). However, that is relatively harmless compared to HBASE-21862. + // Play it safe and assume we could have a double-assignment situation. + // Note that we don't do it in regular RS report, because races there are much more frequent. + throw new RegionsStateInvalidException("Potentially double-assigning " + regionNode); } regionNode.setOpenSeqNum(openSeqNum); env.getAssignmentManager().regionOpened(regionNode); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RSProcedureDispatcher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RSProcedureDispatcher.java index 638f9d3461..a5101c133c 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RSProcedureDispatcher.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RSProcedureDispatcher.java @@ -23,6 +23,7 @@ import java.util.Set; import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; import org.apache.hadoop.hbase.DoNotRetryIOException; +import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.exceptions.ClientExceptionsUtil; @@ -67,6 +68,12 @@ public class RSProcedureDispatcher "hbase.regionserver.rpc.startup.waittime"; private static final int DEFAULT_RS_RPC_STARTUP_WAIT_TIME = 60000; + // -1 means don't set the deadline + public static final String MASTER_RPC_DEADLINE_SAFETY_MS = + "hbase.master.rpc.deadline.safety.ms"; + private static final int DEFAULT_MASTER_RPC_DEADLINE_SAFETY_MS = 0; + + private static final int RS_VERSION_WITH_EXEC_PROCS = 0x0200000; // 2.0 protected final MasterServices master; @@ -308,7 +315,13 @@ public class RSProcedureDispatcher LOG.trace("Building request with operations count=" + remoteProcedures.size()); } splitAndResolveOperation(getServerName(), remoteProcedures, this); - + int safetyMs = procedureEnv.getMasterConfiguration().getInt( + MASTER_RPC_DEADLINE_SAFETY_MS, DEFAULT_MASTER_RPC_DEADLINE_SAFETY_MS); + if (safetyMs >= 0) { + long timeoutMs = safetyMs + procedureEnv.getMasterConfiguration().getLong( + HConstants.HBASE_RPC_TIMEOUT_KEY, HConstants.DEFAULT_HBASE_RPC_TIMEOUT); + request.setMasterRpcDeadline(EnvironmentEdgeManager.currentTime() + timeoutMs); + } try { sendRequest(getServerName(), request.build()); } catch (IOException e) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java index 34a6c13924..e8fd6abbc7 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java @@ -75,6 +75,7 @@ import org.apache.hadoop.hbase.HealthCheckChore; import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.NotServingRegionException; import org.apache.hadoop.hbase.PleaseHoldException; +import org.apache.hadoop.hbase.RegionsStateInvalidException; import org.apache.hadoop.hbase.ScheduledChore; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.Stoppable; @@ -1230,7 +1231,7 @@ public class HRegionServer extends HasThread implements rss.regionServerReport(null, request.build()); } catch (ServiceException se) { IOException ioe = ProtobufUtil.getRemoteException(se); - if (ioe instanceof YouAreDeadException) { + if (ioe instanceof YouAreDeadException || ioe instanceof RegionsStateInvalidException) { // This will be caught and handled as a fatal error in run() throw ioe; } @@ -2325,6 +2326,11 @@ public class HRegionServer extends HasThread implements return true; } catch (ServiceException se) { IOException ioe = ProtobufUtil.getRemoteException(se); + if (ioe instanceof RegionsStateInvalidException) { + abort("Incorrect transition", ioe); + return false; + } + boolean pause = ioe instanceof ServerNotRunningYetException || ioe instanceof PleaseHoldException; if (pause) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java index 592f99c431..bf9a4d9e57 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java @@ -3776,6 +3776,19 @@ public class RSRpcServices implements HBaseRPCErrorHandler, checkOpen(); throwOnWrongStartCode(request); regionServer.getRegionServerCoprocessorHost().preExecuteProcedures(); + long rpcDeadline = request.hasMasterRpcDeadline() ? request.getMasterRpcDeadline() : 0; + long now = EnvironmentEdgeManager.currentTime(); + if (rpcDeadline > 0 && rpcDeadline < now) { + // Master probably already gave up on this request. Executing procedures in such cases + // can cause bugs in master due to some assumptions about failures. Fail the call. + String msg = "Dropping the executeProcedures request after the timeout; deadline is " + + rpcDeadline + "; current time is " + now; + LOG.warn(msg); + throw new ServiceException(msg); + } + // In theory it's still possible to reach the deadline before responding, + // but it should be fast and we can't un-submit part of the procedures right now. + if (request.getOpenRegionCount() > 0) { // Avoid reading from the TableDescritor every time(usually it will read from the file // system)