From 8de6b1a3aa3a2ee6356277116e8b1e0ac4d1a64b Mon Sep 17 00:00:00 2001 From: Michael Stack Date: Mon, 12 Mar 2018 16:08:41 -0700 Subject: [PATCH] HBASE-20178 [AMv2] Throw exception if hostile environment Fail-fast by throwing exception out of Procedure constructor if move but table is disabled, etc. M hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMove.java Test stolen from HBASE-20131 M hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableState.java Add convenience isEnabled/isDisabled M hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java Promote assert state to throw exception. M hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java Add isClusterUp M hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java Move constructor now throws exception M hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java M hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java M hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java M hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DisableTableProcedure.java M hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java M hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java M hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TruncateTableProcedure.java Do environment check at construction and fail-fast if hostile. M hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineTableProcedure.java Add checkTableOnline utility method. --- .../org/apache/hadoop/hbase/client/TableState.java | 14 +++ .../hadoop/hbase/zookeeper/ReadOnlyZKClient.java | 2 +- .../org/apache/hadoop/hbase/ipc/CallRunner.java | 3 +- .../org/apache/hadoop/hbase/master/HMaster.java | 6 +- .../apache/hadoop/hbase/master/MasterServices.java | 5 + .../hbase/master/assignment/AssignProcedure.java | 3 +- .../hbase/master/assignment/AssignmentManager.java | 14 ++- .../assignment/MergeTableRegionsProcedure.java | 7 +- .../master/assignment/MoveRegionProcedure.java | 12 +- .../assignment/SplitTableRegionProcedure.java | 3 + .../AbstractStateMachineTableProcedure.java | 29 +++++ .../master/procedure/DeleteNamespaceProcedure.java | 6 +- .../master/procedure/DisableTableProcedure.java | 8 +- .../master/procedure/ModifyTableProcedure.java | 8 +- .../master/procedure/RestoreSnapshotProcedure.java | 8 +- .../master/procedure/TruncateTableProcedure.java | 8 +- .../hadoop/hbase/regionserver/HRegionServer.java | 3 +- .../hbase/regionserver/RegionServerServices.java | 5 + .../hadoop/hbase/MockRegionServerServices.java | 5 + .../hbase/master/MockNoopMasterServices.java | 5 + .../hadoop/hbase/master/MockRegionServer.java | 5 + .../hadoop/hbase/regionserver/TestRegionMove.java | 128 +++++++++++++++++++++ 22 files changed, 257 insertions(+), 30 deletions(-) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMove.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableState.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableState.java index d2334a4266..1df07cc1a5 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableState.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableState.java @@ -39,6 +39,20 @@ public class TableState { ENABLING; /** + * @return True if table is enabled: i.e. {@link #ENABLED} or {@link #ENABLING}. + */ + public boolean isEnabled() { + return isInStates(ENABLED, ENABLING); + } + + /** + * @return True if table is disabled: i.e. NOT {@link #ENABLED} and NOT {@link #ENABLING}. + */ + public boolean isDisabled() { + return !isEnabled(); + } + + /** * Covert from PB version of State * * @param state convert from diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java index e41f04acbf..d2f4763332 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java @@ -313,7 +313,7 @@ public final class ReadOnlyZKClient implements Closeable { } if (task == null) { if (pendingRequests == 0) { - LOG.debug("{} to {} inactive for {}ms; closing (Will reconnect when new requests)", + LOG.trace("{} to {} inactive for {}ms; closing (Will reconnect when new requests)", getId(), connectString, keepAliveTimeMs); closeZk(); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java index e18518ebaa..e4763a5c5d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java @@ -138,7 +138,8 @@ public class CallRunner { RpcServer.LOG.trace(call.toShortString(), e); } } else { - RpcServer.LOG.debug(call.toShortString(), e); + // Don't dump full exception.. just String version + RpcServer.LOG.debug(call.toShortString() + ", exception=" + e); } errorThrowable = e; error = StringUtils.stringifyException(e); 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 cd967ed4d8..1a4e6b65eb 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 @@ -1681,7 +1681,6 @@ public class HMaster extends HRegionServer implements MasterServices { // Now we can do the move RegionPlan rp = new RegionPlan(hri, regionState.getServerName(), dest); assert rp.getDestination() != null: rp.toString() + " " + dest; - assert rp.getSource() != null: rp.toString(); try { checkInitialized(); @@ -2398,8 +2397,9 @@ public class HMaster extends HRegionServer implements MasterServices { throw new IOException("Can't modify catalog tables"); } checkTableExists(tableName); - if (!getTableStateManager().isTableState(tableName, TableState.State.DISABLED)) { - throw new TableNotDisabledException(tableName); + TableState.State state = getTableStateManager().getTableState(tableName); + if (!state.equals(TableState.State.DISABLED)) { + throw new TableNotDisabledException(tableName + " is not disabled, state=" + state); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java index 0e552d6930..52046c5149 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java @@ -501,4 +501,9 @@ public interface MasterServices extends Server { boolean recoverMeta() throws IOException; String getClientIdAuditPrefix(); + + /** + * @return True if cluster is up; false if cluster is not up (we are shutting down). + */ + boolean isClusterUp(); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java index fd3155398f..28ad0f092e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java @@ -26,7 +26,6 @@ import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.RetriesExhaustedException; -import org.apache.hadoop.hbase.client.TableState; import org.apache.hadoop.hbase.exceptions.UnexpectedStateException; import org.apache.hadoop.hbase.master.RegionState.State; import org.apache.hadoop.hbase.master.TableStateManager; @@ -161,7 +160,7 @@ public class AssignProcedure extends RegionTransitionProcedure { // Don't assign if table is in disabling or disabled state. TableStateManager tsm = env.getMasterServices().getTableStateManager(); TableName tn = regionNode.getRegionInfo().getTable(); - if (tsm.isTableState(tn, TableState.State.DISABLING, TableState.State.DISABLED)) { + if (!tsm.getTableState(tn).isDisabled()) { LOG.info("Table " + tn + " state=" + tsm.getTableState(tn) + ", skipping " + this); return false; } 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 a48ed75708..106c712c14 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 @@ -556,7 +556,7 @@ public class AssignmentManager implements ServerListener { ProcedureSyncWait.submitAndWaitProcedure(master.getMasterProcedureExecutor(), proc); } - public Future moveAsync(final RegionPlan regionPlan) { + public Future moveAsync(final RegionPlan regionPlan) throws HBaseIOException { MoveRegionProcedure proc = createMoveRegionProcedure(regionPlan); return ProcedureSyncWait.submitProcedure(master.getMasterProcedureExecutor(), proc); } @@ -678,7 +678,7 @@ public class AssignmentManager implements ServerListener { return procedures.toArray(ASSIGN_PROCEDURE_ARRAY_TYPE); } - // Needed for the following method so it can type the created Array we return + // Needed for the following method so it can type the created Array we retur n private static final UnassignProcedure [] UNASSIGN_PROCEDURE_ARRAY_TYPE = new UnassignProcedure[0]; @@ -695,7 +695,8 @@ public class AssignmentManager implements ServerListener { return procs.toArray(UNASSIGN_PROCEDURE_ARRAY_TYPE); } - public MoveRegionProcedure[] createReopenProcedures(final Collection regionInfo) { + public MoveRegionProcedure[] createReopenProcedures(final Collection regionInfo) + throws IOException { final MoveRegionProcedure[] procs = new MoveRegionProcedure[regionInfo.size()]; int index = 0; for (RegionInfo hri: regionInfo) { @@ -744,7 +745,8 @@ public class AssignmentManager implements ServerListener { return proc; } - public MoveRegionProcedure createMoveRegionProcedure(final RegionPlan plan) { + public MoveRegionProcedure createMoveRegionProcedure(final RegionPlan plan) + throws HBaseIOException { if (plan.getRegionInfo().getTable().isSystemTable()) { List exclude = getExcludedServersForSystemTable(); if (plan.getDestination() != null && exclude.contains(plan.getDestination())) { @@ -861,8 +863,8 @@ public class AssignmentManager implements ServerListener { final ServerStateNode serverNode = regionStates.getOrCreateServer(serverName); if (!reportTransition(regionNode, serverNode, state, seqId)) { - LOG.warn(String.format( - "No procedure for %s. server=%s to transition to %s", regionNode, serverName, state)); + // Don't log if shutting down cluster; during shutdown. + LOG.warn("No matchin procedure found for {} to transition to {}", regionNode, state); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java index 052ba7fbd3..ec59d15bfd 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java @@ -73,6 +73,8 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos.M * The procedure to Merge a region in a table. * This procedure takes an exclusive table lock since it is working over multiple regions. * It holds the lock for the life of the procedure. + *

Throws exception on construction if determines context hostile to merge (cluster going + * down or master is shutting down or table is disabled).

*/ @InterfaceAudience.Private public class MergeTableRegionsProcedure @@ -96,14 +98,15 @@ public class MergeTableRegionsProcedure public MergeTableRegionsProcedure(final MasterProcedureEnv env, final RegionInfo regionToMergeA, final RegionInfo regionToMergeB, - final boolean forcible) throws MergeRegionException { + final boolean forcible) throws IOException { this(env, new RegionInfo[] {regionToMergeA, regionToMergeB}, forcible); } public MergeTableRegionsProcedure(final MasterProcedureEnv env, final RegionInfo[] regionsToMerge, final boolean forcible) - throws MergeRegionException { + throws IOException { super(env); + checkTableOnline(env); // Check daughter regions and make sure that we have valid daughter regions // before doing the real work. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java index a29bfee2cb..733b9eb265 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java @@ -21,6 +21,7 @@ package org.apache.hadoop.hbase.master.assignment; import java.io.IOException; +import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.RegionInfo; @@ -40,6 +41,9 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos.M * It first runs an unassign subprocedure followed * by an assign subprocedure. It takes a lock on the region being moved. * It holds the lock for the life of the procedure. + * + *

Throws exception on construction if determines context hostile to move (cluster going + * down or master is shutting down or table is disabled).

*/ @InterfaceAudience.Private public class MoveRegionProcedure extends AbstractStateMachineRegionProcedure { @@ -51,9 +55,15 @@ public class MoveRegionProcedure extends AbstractStateMachineRegionProcedureThrows exception on construction if determines context hostile to spllt (cluster going + * down or master is shutting down or table is disabled).

*/ @InterfaceAudience.Private public class SplitTableRegionProcedure @@ -104,6 +106,7 @@ public class SplitTableRegionProcedure public SplitTableRegionProcedure(final MasterProcedureEnv env, final RegionInfo regionToSplit, final byte[] splitRow) throws IOException { super(env, regionToSplit); + checkTableOnline(env); this.bestSplitRow = splitRow; checkSplittable(env, regionToSplit, bestSplitRow); final TableName table = regionToSplit.getTable(); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineTableProcedure.java index 833b659a5b..23894986de 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineTableProcedure.java @@ -20,11 +20,14 @@ package org.apache.hadoop.hbase.master.procedure; import java.io.IOException; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.TableNotEnabledException; import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.master.MasterFileSystem; +import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.procedure2.StateMachineProcedure; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.util.FSUtils; @@ -124,4 +127,30 @@ public abstract class AbstractStateMachineTableProcedure Path tableDir = FSUtils.getTableDir(mfs.getRootDir(), getTableName()); return new Path(tableDir, ServerRegionReplicaUtil.getRegionInfoForFs(region).getEncodedName()); } + + /** + * Check table is online and modifiable and that cluster and master are up. + * Run in constructor so can pass exception to caller. + */ + protected void checkTableOnline(MasterProcedureEnv env) throws HBaseIOException { + MasterServices master = env.getMasterServices(); + if (!master.isClusterUp()) { + throw new HBaseIOException("Cluster not up!"); + } + if (master.isStopping() || master.isStopped()) { + throw new HBaseIOException("Master stopping=" + master.isStopping() + + ", stopped=" + master.isStopped()); + } + try { + checkTableModifiable(env); + if (master.getTableStateManager().getTableState(getTableName()).isEnabled()) { + throw new TableNotEnabledException(getTableName()); + } + } catch (IOException ioe) { + if (ioe instanceof HBaseIOException) { + throw (HBaseIOException)ioe; + } + throw new HBaseIOException(ioe); + } + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteNamespaceProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteNamespaceProcedure.java index 5f7959e410..8369a19564 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteNamespaceProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteNamespaceProcedure.java @@ -71,11 +71,7 @@ public class DeleteNamespaceProcedure @Override protected Flow executeFromState(final MasterProcedureEnv env, final DeleteNamespaceState state) throws InterruptedException { - if (isTraceEnabled()) { - LOG.trace(this + " execute state=" + state); - } - LOG.info(this + " execute state=" + state); - + LOG.info(this.toString()); try { switch (state) { case DELETE_NAMESPACE_PREPARE: diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DisableTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DisableTableProcedure.java index e748c6ce7f..7e657e1322 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DisableTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DisableTableProcedure.java @@ -20,6 +20,7 @@ package org.apache.hadoop.hbase.master.procedure; import java.io.IOException; +import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.TableNotEnabledException; @@ -57,7 +58,8 @@ public class DisableTableProcedure * @param skipTableStateCheck whether to check table state */ public DisableTableProcedure(final MasterProcedureEnv env, final TableName tableName, - final boolean skipTableStateCheck) { + final boolean skipTableStateCheck) + throws HBaseIOException { this(env, tableName, skipTableStateCheck, null); } @@ -68,9 +70,11 @@ public class DisableTableProcedure * @param skipTableStateCheck whether to check table state */ public DisableTableProcedure(final MasterProcedureEnv env, final TableName tableName, - final boolean skipTableStateCheck, final ProcedurePrepareLatch syncLatch) { + final boolean skipTableStateCheck, final ProcedurePrepareLatch syncLatch) + throws HBaseIOException { super(env, syncLatch); this.tableName = tableName; + checkTableOnline(env); this.skipTableStateCheck = skipTableStateCheck; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java index f0be1e0819..bf6233c602 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java @@ -24,6 +24,7 @@ import java.util.List; import java.util.Set; import org.apache.hadoop.hbase.DoNotRetryIOException; +import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.TableName; @@ -63,15 +64,18 @@ public class ModifyTableProcedure initilize(); } - public ModifyTableProcedure(final MasterProcedureEnv env, final TableDescriptor htd) { + public ModifyTableProcedure(final MasterProcedureEnv env, final TableDescriptor htd) + throws HBaseIOException { this(env, htd, null); } public ModifyTableProcedure(final MasterProcedureEnv env, final TableDescriptor htd, - final ProcedurePrepareLatch latch) { + final ProcedurePrepareLatch latch) + throws HBaseIOException { super(env, latch); initilize(); this.modifiedTableDescriptor = htd; + checkTableOnline(env); } private void initilize() { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java index 9aa5171786..b1216af9e3 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java @@ -28,6 +28,7 @@ import java.util.Map; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.DoNotRetryIOException; +import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.TableNotFoundException; @@ -81,7 +82,8 @@ public class RestoreSnapshotProcedure } public RestoreSnapshotProcedure(final MasterProcedureEnv env, - final TableDescriptor tableDescriptor, final SnapshotDescription snapshot) { + final TableDescriptor tableDescriptor, final SnapshotDescription snapshot) + throws HBaseIOException { this(env, tableDescriptor, snapshot, false); } /** @@ -95,10 +97,12 @@ public class RestoreSnapshotProcedure final MasterProcedureEnv env, final TableDescriptor tableDescriptor, final SnapshotDescription snapshot, - final boolean restoreAcl) { + final boolean restoreAcl) + throws HBaseIOException { super(env); // This is the new schema we are going to write out as this modification. this.modifiedTableDescriptor = tableDescriptor; + checkTableOnline(env); // Snapshot information this.snapshot = snapshot; this.restoreAcl = restoreAcl; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TruncateTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TruncateTableProcedure.java index 80cc5a8047..bae93a21bf 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TruncateTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TruncateTableProcedure.java @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.TableNotDisabledException; import org.apache.hadoop.hbase.TableNotFoundException; @@ -56,14 +57,17 @@ public class TruncateTableProcedure } public TruncateTableProcedure(final MasterProcedureEnv env, final TableName tableName, - boolean preserveSplits) { + boolean preserveSplits) + throws HBaseIOException { this(env, tableName, preserveSplits, null); } public TruncateTableProcedure(final MasterProcedureEnv env, final TableName tableName, - boolean preserveSplits, ProcedurePrepareLatch latch) { + boolean preserveSplits, ProcedurePrepareLatch latch) + throws HBaseIOException { super(env, latch); this.tableName = tableName; + checkTableOnline(env); this.preserveSplits = preserveSplits; } 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 81febc0ea9..e03757ffbd 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 @@ -910,7 +910,8 @@ public class HRegionServer extends HasThread implements /** * @return True if the cluster is up. */ - private boolean isClusterUp() { + @Override + public boolean isClusterUp() { return this.masterless || (this.clusterStatusTracker != null && this.clusterStatusTracker.isClusterUp()); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java index 60bd215933..023efd94b9 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java @@ -234,4 +234,9 @@ public interface RegionServerServices extends Server, MutableOnlineRegions, Favo * See HBASE-17712 for more details. */ void unassign(byte[] regionName) throws IOException; + + /** + * @return True if cluster is up; false if cluster is not up (we are shutting down). + */ + boolean isClusterUp(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java index f1e020f581..879b592f17 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java @@ -334,4 +334,9 @@ public class MockRegionServerServices implements RegionServerServices { public Connection createConnection(Configuration conf) throws IOException { return null; } + + @Override + public boolean isClusterUp() { + return true; + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java index 4e66676303..cc2f43dd44 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java @@ -478,4 +478,9 @@ public class MockNoopMasterServices implements MasterServices { public ReplicationPeerManager getReplicationPeerManager() { return null; } + + @Override + public boolean isClusterUp() { + return true; + } } \ No newline at end of file diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java index cabd769698..d366b67fe5 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java @@ -678,4 +678,9 @@ ClientProtos.ClientService.BlockingInterface, RegionServerServices { public Connection createConnection(Configuration conf) throws IOException { return null; } + + @Override + public boolean isClusterUp() { + return true; + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMove.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMove.java new file mode 100644 index 0000000000..8940beaac4 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMove.java @@ -0,0 +1,128 @@ +/** + * 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.regionserver; + +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.util.List; +import java.util.stream.Collectors; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.TableNotEnabledException; +import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.testclassification.LargeTests; +import org.apache.hadoop.hbase.util.Bytes; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.rules.ExpectedException; +import org.junit.rules.TestName; + +/** + * Test move fails when table disabled + */ +@Category({LargeTests.class}) +public class TestRegionMove { + + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestRegionMove.class); + + @Rule + public ExpectedException thrown = ExpectedException.none(); + + @Rule + public TestName name = new TestName(); + private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); + public static Configuration CONF ; + protected static final String F1 = "f1"; + + // Test names + protected TableName tableName; + protected String method; + + @BeforeClass + public static void startCluster() throws Exception { + TEST_UTIL.startMiniCluster(2); + } + + @AfterClass + public static void stopCluster() throws Exception { + TEST_UTIL.shutdownMiniCluster(); + } + + @Before + public void setup() throws IOException { + CONF = TEST_UTIL.getConfiguration(); + method = name.getMethodName(); + tableName = TableName.valueOf(method); + } + + @Test + public void testDisableAndMove() throws Exception { + Admin admin = TEST_UTIL.getAdmin(); + + // Create a table with more than one region + Table t = TEST_UTIL.createMultiRegionTable(tableName, Bytes.toBytes(F1), 10); + TEST_UTIL.waitUntilAllRegionsAssigned(tableName); + + // Write an update to each region + for (RegionInfo regionInfo : admin.getRegions(tableName)) { + byte[] startKey = regionInfo.getStartKey(); + // The startKey of the first region is "empty", which would throw an error if we try to + // Put that. + byte[] rowKey = org.apache.hbase.thirdparty.com.google.common.primitives.Bytes.concat( + startKey, Bytes.toBytes("1")); + Put p = new Put(rowKey); + p.addColumn(Bytes.toBytes(F1), Bytes.toBytes("q1"), Bytes.toBytes("value")); + t.put(p); + } + + // Get a Region which is on the first RS + HRegionServer rs1 = TEST_UTIL.getRSForFirstRegionInTable(tableName); + HRegionServer rs2 = TEST_UTIL.getOtherRegionServer(rs1); + List regionsOnRS1ForTable = admin.getRegions(rs1.getServerName()).stream() + .filter((regionInfo) -> regionInfo.getTable().equals(tableName)) + .collect(Collectors.toList()); + assertTrue( + "Expected to find at least one region for " + tableName + " on " + rs1.getServerName() + + ", but found none", !regionsOnRS1ForTable.isEmpty()); + final RegionInfo regionToMove = regionsOnRS1ForTable.get(0); + + // Disable the table + admin.disableTable(tableName); + + // We except a DNRIOE when we try to move a region which isn't open. + thrown.expect(TableNotEnabledException.class); + thrown.expectMessage(t.getName().toString()); + + // Move the region to the other RS -- should fail + admin.move(regionToMove.getEncodedNameAsBytes(), Bytes.toBytes(rs2.getServerName().toString())); + } +} -- 2.11.0 (Apple Git-81)