From 102cb913355b0e7c7476e37823d1d0e8f5fa8f02 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 or if master is going down, etc. Also fixed bug around table state where we presumed ENABLED though no entry in hbase:meta (we were using this mechanism for whether a table existed or not). 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 preflightCheck utility method. M hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java Removed setting time setting table state; broke when using other than default environment edge masked by presumption that no state meant active. --- .../org/apache/hadoop/hbase/MetaTableAccessor.java | 7 +- .../org/apache/hadoop/hbase/client/TableState.java | 43 +++++-- .../hadoop/hbase/zookeeper/ReadOnlyZKClient.java | 2 +- .../org/apache/hadoop/hbase/ipc/CallRunner.java | 3 +- .../org/apache/hadoop/hbase/master/HMaster.java | 6 +- .../hadoop/hbase/master/MasterRpcServices.java | 5 +- .../apache/hadoop/hbase/master/MasterServices.java | 5 + .../hbase/master/MirroringTableStateManager.java | 2 +- .../hadoop/hbase/master/TableNamespaceManager.java | 4 +- .../hadoop/hbase/master/TableStateManager.java | 21 ++-- .../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 | 49 ++++++++ .../master/procedure/DeleteNamespaceProcedure.java | 6 +- .../master/procedure/DisableTableProcedure.java | 17 +-- .../master/procedure/EnableTableProcedure.java | 9 +- .../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 + .../resources/hbase-webapps/master/rsgroup.jsp | 9 +- .../hadoop/hbase/MockRegionServerServices.java | 5 + .../hadoop/hbase/master/AbstractTestDLS.java | 1 + .../hbase/master/MockNoopMasterServices.java | 5 + .../hadoop/hbase/master/MockRegionServer.java | 5 + .../master/assignment/MockMasterServices.java | 12 ++ .../procedure/MasterProcedureTestingUtility.java | 4 +- .../procedure/TestDisableTableProcedure.java | 46 +++++--- .../hadoop/hbase/regionserver/TestRegionMove.java | 128 +++++++++++++++++++++ 33 files changed, 374 insertions(+), 91 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/MetaTableAccessor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java index e779054741..3d3edd62fe 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java @@ -1073,10 +1073,7 @@ public class MetaTableAccessor { } Table metaHTable = getMetaHTable(conn); Get get = new Get(tableName.getName()).addColumn(getTableFamily(), getTableStateColumn()); - long time = EnvironmentEdgeManager.currentTime(); - get.setTimeRange(0, time); - Result result = - metaHTable.get(get); + Result result = metaHTable.get(get); return getTableState(result); } @@ -1643,7 +1640,7 @@ public class MetaTableAccessor { private static void updateTableState(Connection connection, TableState state) throws IOException { Put put = makePutFromTableState(state, EnvironmentEdgeManager.currentTime()); putToMetaTable(connection, put); - LOG.info("Updated table {} state to {} in META", state.getTableName(), state.getState()); + LOG.info("Updated {} in hbase:meta", state); } /** 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..8a93d07042 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 @@ -97,6 +97,38 @@ public class TableState { private final State state; /** + * @return True if table is {@link State#ENABLED}. + */ + public boolean isEnabled() { + return isInStates(State.ENABLED); + } + + /** + * @return True if {@link State#ENABLED} or {@link State#ENABLING} + */ + public boolean isEnabledOrEnabling() { + return isInStates(State.ENABLED, State.ENABLING); + } + + /** + * @return True if table is disabled. + */ + public boolean isDisabled() { + return isInStates(State.DISABLED); + } + + /** + * @return True if {@link State#DISABLED} or {@link State#DISABLED} + */ + public boolean isDisabledOrDisabling() { + return isInStates(State.DISABLED, State.DISABLING); + } + + public boolean equals(TableState.State state) { + return getState().equals(state); + } + + /** * Create instance of TableState. * @param tableName name of the table * @param state table state @@ -177,14 +209,14 @@ public class TableState { /** * Static version of state checker - * @param state desired * @param target equals to any of * @return true if satisfies */ - public static boolean isInStates(State state, State... target) { + public boolean isInStates(State... target) { for (State tableState : target) { - if (state.equals(tableState)) + if (this.state.equals(tableState)) { return true; + } } return false; } @@ -212,9 +244,6 @@ public class TableState { @Override public String toString() { - return "TableState{" + - ", tableName=" + tableName + - ", state=" + state + - '}'; + return "tableName=" + tableName + ", state=" + state; } } 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..1812d8c6f9 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 ts = getTableStateManager().getTableState(tableName); + if (!ts.isDisabled()) { + throw new TableNotDisabledException("Not DISABLE tableState=" + ts); } } 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 8f92041f6b..e328e0ffd9 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 @@ -1038,10 +1038,9 @@ public class MasterRpcServices extends RSRpcServices try { master.checkServiceStarted(); TableName tableName = ProtobufUtil.toTableName(request.getTableName()); - TableState.State state = master.getTableStateManager() - .getTableState(tableName); + TableState ts = master.getTableStateManager().getTableState(tableName); GetTableStateResponse.Builder builder = GetTableStateResponse.newBuilder(); - builder.setTableState(new TableState(tableName, state).convert()); + builder.setTableState(ts.convert()); return builder.build(); } catch (IOException e) { throw new ServiceException(e); 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/MirroringTableStateManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MirroringTableStateManager.java index 752c9412af..417e11d980 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MirroringTableStateManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MirroringTableStateManager.java @@ -77,7 +77,7 @@ public class MirroringTableStateManager extends TableStateManager { } private void updateZooKeeper(TableState tableState) throws IOException { - if (tableState == null || tableState.getState() == null) { + if (tableState == null) { return; } String znode = ZNodePaths.joinZNode(this.master.getZooKeeper().getZNodePaths().tableZNode, diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableNamespaceManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableNamespaceManager.java index f334d32ae4..f63a7b0102 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableNamespaceManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableNamespaceManager.java @@ -315,12 +315,12 @@ public class TableNamespaceManager implements Stoppable { return false; } - private TableState.State getTableState() throws IOException { + private TableState getTableState() throws IOException { return masterServices.getTableStateManager().getTableState(TableName.NAMESPACE_TABLE_NAME); } private boolean isTableEnabled() throws IOException { - return getTableState().equals(TableState.State.ENABLED); + return getTableState().isEnabled(); } private boolean isTableAssigned() { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableStateManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableStateManager.java index affb684f74..8a33de69f0 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableStateManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableStateManager.java @@ -93,7 +93,7 @@ public class TableStateManager { * @return null if succeed or table state if failed * @throws IOException */ - public TableState.State setTableStateIfInStates(TableName tableName, + public TableState setTableStateIfInStates(TableName tableName, TableState.State newState, TableState.State... states) throws IOException { @@ -107,12 +107,11 @@ public class TableStateManager { updateMetaState(tableName, newState); return null; } else { - return currentState.getState(); + return currentState; } } finally { lock.writeLock().unlock(); } - } /** @@ -141,8 +140,8 @@ public class TableStateManager { public boolean isTableState(TableName tableName, TableState.State... states) { try { - TableState.State tableState = getTableState(tableName); - return TableState.isInStates(tableState, states); + TableState tableState = getTableState(tableName); + return tableState.isInStates(states); } catch (IOException e) { LOG.error("Unable to get table " + tableName + " state", e); return false; @@ -188,12 +187,12 @@ public class TableStateManager { } @NonNull - public TableState.State getTableState(TableName tableName) throws IOException { + public TableState getTableState(TableName tableName) throws IOException { TableState currentState = readMetaState(tableName); if (currentState == null) { throw new TableStateNotFoundException(tableName); } - return currentState.getState(); + return currentState; } protected void updateMetaState(TableName tableName, TableState.State newState) @@ -243,7 +242,7 @@ public class TableStateManager { continue; } TableState tableState = states.get(table); - if (tableState == null || tableState.getState() == null) { + if (tableState == null) { LOG.warn(table + " has no table state in hbase:meta, assuming ENABLED"); MetaTableAccessor.updateTableState(connection, TableName.valueOf(table), TableState.State.ENABLED); @@ -283,13 +282,13 @@ public class TableStateManager { entry.getKey()); continue; } - TableState.State state = null; + TableState ts = null; try { - state = getTableState(entry.getKey()); + ts = getTableState(entry.getKey()); } catch (TableStateNotFoundException e) { // This can happen; table exists but no TableState. } - if (state == null) { + if (ts == null) { TableState.State zkstate = entry.getValue(); // Only migrate if it is an enable or disabled table. If in-between -- ENABLING or // DISABLING then we have a problem; we are starting up an hbase-2 on a cluster with 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..0ece343fa3 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).isDisabledOrDisabling()) { 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 33a8d2174b..754731b02b 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..b94c0d872c 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,13 +98,13 @@ 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); // Check daughter regions and make sure that we have valid daughter regions @@ -116,6 +118,7 @@ public class MergeTableRegionsProcedure // Since HBASE-7721, we don't need fix up daughters any more. so here do nothing this.regionsToMerge = regionsToMerge; this.mergedRegion = createMergedRegionInfo(regionsToMerge); + preflightChecks(env, true); this.forcible = forcible; } 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..065987f013 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); + preflightChecks(env, true); 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..3182d45f25 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,17 @@ 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.TableNotDisabledException; +import org.apache.hadoop.hbase.TableNotEnabledException; import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.client.TableState; import org.apache.hadoop.hbase.master.MasterFileSystem; +import org.apache.hadoop.hbase.master.MasterServices; +import org.apache.hadoop.hbase.master.TableStateManager; import org.apache.hadoop.hbase.procedure2.StateMachineProcedure; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.util.FSUtils; @@ -124,4 +130,47 @@ public abstract class AbstractStateMachineTableProcedure Path tableDir = FSUtils.getTableDir(mfs.getRootDir(), getTableName()); return new Path(tableDir, ServerRegionReplicaUtil.getRegionInfoForFs(region).getEncodedName()); } + + /** + * Check that cluster is up and master is running. Check table is modifiable. + * If enabled, check table is enabled else check it is disabled. + * Call in Procedure constructor so can pass any exception to caller. + * @param enabled If true, check table is enabled and throw exception if not. If false, do the + * inverse. If null, do no table checks. + */ + protected void preflightChecks(MasterProcedureEnv env, Boolean enabled) 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()); + } + if (enabled == null) { + // Don't do any table checks. + return; + } + try { + // Checks table exists and is modifiable. + checkTableModifiable(env); + TableName tn = getTableName(); + TableStateManager tsm = master.getTableStateManager(); + TableState ts = tsm.getTableState(tn); + if (enabled) { + if (!ts.isEnabledOrEnabling()) { + throw new TableNotEnabledException(tn); + } + } else { + if (!ts.isDisabledOrDisabling()) { + throw new TableNotDisabledException(tn); + } + } + } 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..413e3ae3b4 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; + preflightChecks(env, true); this.skipTableStateCheck = skipTableStateCheck; } @@ -230,11 +234,10 @@ public class DisableTableProcedure // was implemented. With table lock, there is no need to set the state here (it will // set the state later on). A quick state check should be enough for us to move forward. TableStateManager tsm = env.getMasterServices().getTableStateManager(); - TableState.State state = tsm.getTableState(tableName); - if (!state.equals(TableState.State.ENABLED)){ - LOG.info("Table " + tableName + " isn't enabled;is "+state.name()+"; skipping disable"); - setFailure("master-disable-table", new TableNotEnabledException( - tableName+" state is "+state.name())); + TableState ts = tsm.getTableState(tableName); + if (!ts.isEnabled()) { + LOG.info("Not ENABLED tableState=" + ts + "; skipping disable"); + setFailure("master-disable-table", new TableNotEnabledException(ts.toString())); canTableBeDisabled = false; } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.java index c501e5396a..c46070cd58 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.java @@ -333,11 +333,10 @@ public class EnableTableProcedure // was implemented. With table lock, there is no need to set the state here (it will // set the state later on). A quick state check should be enough for us to move forward. TableStateManager tsm = env.getMasterServices().getTableStateManager(); - TableState.State state = tsm.getTableState(tableName); - if(!state.equals(TableState.State.DISABLED)){ - LOG.info("Table " + tableName + " isn't disabled;is "+state.name()+"; skipping enable"); - setFailure("master-enable-table", new TableNotDisabledException( - this.tableName+" state is "+state.name())); + TableState ts = tsm.getTableState(tableName); + if(!ts.isDisabled()){ + LOG.info("Not DISABLED tableState=" + ts + "; skipping enable"); + setFailure("master-enable-table", new TableNotDisabledException(ts.toString())); canTableBeEnabled = false; } } 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..1f1ba3cfee 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; + preflightChecks(env, null/*No table checks; if changing peers, table can be online*/); } 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..08a6dcc51d 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; + preflightChecks(env, false); // 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..4b2c21fbe5 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; + preflightChecks(env, false); 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/main/resources/hbase-webapps/master/rsgroup.jsp b/hbase-server/src/main/resources/hbase-webapps/master/rsgroup.jsp index 43753a5b1a..d866008f42 100644 --- a/hbase-server/src/main/resources/hbase-webapps/master/rsgroup.jsp +++ b/hbase-server/src/main/resources/hbase-webapps/master/rsgroup.jsp @@ -428,13 +428,12 @@ <%= tableName.getNamespaceAsString() %> <%= tableName.getQualifierAsString() %> - <% TableState.State tableState = master.getTableStateManager().getTableState(tableName); - if(TableState.isInStates(tableState, - TableState.State.DISABLED, TableState.State.DISABLING)) { + <% TableState tableState = master.getTableStateManager().getTableState(tableName); + if(tableState.isDisabledOrDisabling()) { %> - <%= tableState.name() %> + <%= tableState.getState().name() %> <% } else { %> - <%= tableState.name() %> + <%= tableState.getState().name() %> <% } %> <% Map> tableRegions = master.getAssignmentManager().getRegionStates().getRegionByStateOfTable(tableName); 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/AbstractTestDLS.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/AbstractTestDLS.java index d3d5ce177a..bc4d32c312 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/AbstractTestDLS.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/AbstractTestDLS.java @@ -496,6 +496,7 @@ public abstract class AbstractTestDLS { blockUntilNoRIT(zkw, master); // disable-enable cycle to get rid of table's dead regions left behind // by createMultiRegions + assertTrue(TEST_UTIL.getAdmin().isTableEnabled(tableName)); LOG.debug("Disabling table\n"); TEST_UTIL.getAdmin().disableTable(tableName); LOG.debug("Waiting for no more RIT\n"); 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/master/assignment/MockMasterServices.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/MockMasterServices.java index 6cd399dfa6..346abbabf4 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/MockMasterServices.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/MockMasterServices.java @@ -41,12 +41,14 @@ import org.apache.hadoop.hbase.client.HConnectionTestingUtility; import org.apache.hadoop.hbase.client.RegionInfoBuilder; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; +import org.apache.hadoop.hbase.client.TableState; import org.apache.hadoop.hbase.master.LoadBalancer; import org.apache.hadoop.hbase.master.MasterFileSystem; import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.master.MasterWalManager; import org.apache.hadoop.hbase.master.MockNoopMasterServices; import org.apache.hadoop.hbase.master.ServerManager; +import org.apache.hadoop.hbase.master.TableStateManager; import org.apache.hadoop.hbase.master.balancer.LoadBalancerFactory; import org.apache.hadoop.hbase.master.procedure.MasterProcedureConstants; import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv; @@ -82,6 +84,7 @@ public class MockMasterServices extends MockNoopMasterServices { private final MasterFileSystem fileSystemManager; private final MasterWalManager walManager; private final AssignmentManager assignmentManager; + private final TableStateManager tableStateManager; private MasterProcedureEnv procedureEnv; private ProcedureExecutor procedureExecutor; @@ -132,6 +135,10 @@ public class MockMasterServices extends MockNoopMasterServices { }; this.balancer = LoadBalancerFactory.getLoadBalancer(conf); this.serverManager = new ServerManager(this); + this.tableStateManager = Mockito.mock(TableStateManager.class); + Mockito.when(this.tableStateManager.getTableState(Mockito.any())). + thenReturn(new TableState(TableName.valueOf("AnyTableNameSetInMockMasterServcies"), + TableState.State.ENABLED)); // Mock up a Client Interface ClientProtos.ClientService.BlockingInterface ri = @@ -288,6 +295,11 @@ public class MockMasterServices extends MockNoopMasterServices { } @Override + public TableStateManager getTableStateManager() { + return tableStateManager; + } + + @Override public ClusterConnection getConnection() { return this.connection; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java index 9546b19767..c8bb97de33 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java @@ -263,13 +263,13 @@ public class MasterProcedureTestingUtility { public static void validateTableIsEnabled(final HMaster master, final TableName tableName) throws IOException { TableStateManager tsm = master.getTableStateManager(); - assertTrue(tsm.getTableState(tableName).equals(TableState.State.ENABLED)); + assertTrue(tsm.getTableState(tableName).getState().equals(TableState.State.ENABLED)); } public static void validateTableIsDisabled(final HMaster master, final TableName tableName) throws IOException { TableStateManager tsm = master.getTableStateManager(); - assertTrue(tsm.getTableState(tableName).equals(TableState.State.DISABLED)); + assertTrue(tsm.getTableState(tableName).getState().equals(TableState.State.DISABLED)); } public static void validateColumnFamilyAddition(final HMaster master, final TableName tableName, diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDisableTableProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDisableTableProcedure.java index 437cda2098..55a9059629 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDisableTableProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDisableTableProcedure.java @@ -21,6 +21,7 @@ import static org.junit.Assert.assertTrue; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.TableNotDisabledException; import org.apache.hadoop.hbase.TableNotEnabledException; import org.apache.hadoop.hbase.procedure2.Procedure; import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; @@ -79,16 +80,26 @@ public class TestDisableTableProcedure extends TestTableDDLProcedureBase { ProcedureTestingUtility.assertProcNotFailed(procExec, procId1); MasterProcedureTestingUtility.validateTableIsDisabled(getMaster(), tableName); - // Disable the table again - expect failure - long procId2 = procExec.submitProcedure(new DisableTableProcedure( - procExec.getEnvironment(), tableName, false)); - // Wait the completion - ProcedureTestingUtility.waitProcedure(procExec, procId2); - Procedure result = procExec.getResult(procId2); - assertTrue(result.isFailed()); - LOG.debug("Disable failed with exception: " + result.getException()); - assertTrue( - ProcedureTestingUtility.getExceptionCause(result) instanceof TableNotEnabledException); + // Disable the table again - expect failure. We used to get it via procExec#getResult but we + // added fail fast so now happens on construction. + Throwable e = null; + Throwable cause = null; + try { + long procId2 = procExec.submitProcedure(new DisableTableProcedure( + procExec.getEnvironment(), tableName, false)); + // Wait the completion + ProcedureTestingUtility.waitProcedure(procExec, procId2); + Procedure result = procExec.getResult(procId2); + assertTrue(result.isFailed()); + cause = ProcedureTestingUtility.getExceptionCause(result); + e = result.getException(); + } catch (TableNotEnabledException tnde) { + // Expected. + e = tnde; + cause = tnde; + } + LOG.debug("Disable failed with exception {}" + e); + assertTrue(cause instanceof TableNotEnabledException); // Disable the table - expect failure from ProcedurePrepareLatch try { @@ -100,15 +111,20 @@ public class TestDisableTableProcedure extends TestTableDDLProcedureBase { Assert.fail("Disable should throw exception through latch."); } catch (TableNotEnabledException tnee) { // Expected - LOG.debug("Disable failed with expected exception."); + LOG.debug("Disable failed with expected exception {}", tnee); } // Disable the table again with skipping table state check flag (simulate recovery scenario) - long procId4 = procExec.submitProcedure(new DisableTableProcedure( + try { + long procId4 = procExec.submitProcedure(new DisableTableProcedure( procExec.getEnvironment(), tableName, true)); - // Wait the completion - ProcedureTestingUtility.waitProcedure(procExec, procId4); - ProcedureTestingUtility.assertProcNotFailed(procExec, procId4); + // Wait the completion + ProcedureTestingUtility.waitProcedure(procExec, procId4); + ProcedureTestingUtility.assertProcNotFailed(procExec, procId4); + } catch (TableNotEnabledException tnee) { + // Expected + LOG.debug("Disable failed with expected exception {}", tnee); + } MasterProcedureTestingUtility.validateTableIsDisabled(getMaster(), tableName); } 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)