Repository: hbase Updated Branches: refs/heads/branch-2 8fdb61955 -> 63ce0c073
HBASE-20178 [AMv2] Throw exception if hostile environment Add Fail-Fast to Procedures by throwing exception out of Procedure constructor so if move but table is disabled or if master is going down, etc., we can give notice before the procedure is scheduled. Will help guard against scheduling Procedures that will have a hard time succeeding; e.g. a move when table is offline. 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. Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/63ce0c07 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/63ce0c07 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/63ce0c07 Branch: refs/heads/branch-2 Commit: 63ce0c073349e96d18588bf517de64f8de346bf8 Parents: 8fdb619 Author: Michael Stack <[email protected]> Authored: Mon Mar 12 16:08:41 2018 -0700 Committer: Michael Stack <[email protected]> Committed: Wed Mar 14 14:43:34 2018 -0700 ---------------------------------------------------------------------- .../apache/hadoop/hbase/MetaTableAccessor.java | 7 +- .../apache/hadoop/hbase/client/TableState.java | 39 +++++- .../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 +- .../hadoop/hbase/master/MasterServices.java | 5 + .../master/MirroringTableStateManager.java | 2 +- .../hbase/master/TableNamespaceManager.java | 4 +- .../hadoop/hbase/master/TableStateManager.java | 21 ++- .../master/assignment/AssignProcedure.java | 3 +- .../master/assignment/AssignmentManager.java | 14 +- .../assignment/MergeTableRegionsProcedure.java | 7 +- .../master/assignment/MoveRegionProcedure.java | 12 +- .../assignment/SplitTableRegionProcedure.java | 3 + .../AbstractStateMachineTableProcedure.java | 49 +++++++ .../procedure/DeleteNamespaceProcedure.java | 6 +- .../master/procedure/DisableTableProcedure.java | 17 ++- .../master/procedure/EnableTableProcedure.java | 9 +- .../master/procedure/ModifyTableProcedure.java | 8 +- .../procedure/RestoreSnapshotProcedure.java | 8 +- .../procedure/TruncateTableProcedure.java | 8 +- .../hbase/regionserver/HRegionServer.java | 3 +- .../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 ++ .../MasterProcedureTestingUtility.java | 4 +- .../procedure/TestDisableTableProcedure.java | 45 ++++--- .../procedure/TestTruncateTableProcedure.java | 49 ++++--- .../hbase/regionserver/TestRegionMove.java | 128 +++++++++++++++++++ 34 files changed, 401 insertions(+), 108 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.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 e779054..3d3edd6 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); } /** http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableState.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 d2334a4..cc3b765 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,34 @@ 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); + } + + /** * Create instance of TableState. * @param tableName name of the table * @param state table state @@ -177,14 +205,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 +240,6 @@ public class TableState { @Override public String toString() { - return "TableState{" + - ", tableName=" + tableName + - ", state=" + state + - '}'; + return "tableName=" + tableName + ", state=" + state; } } http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java ---------------------------------------------------------------------- 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 e41f04a..d2f4763 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(); } http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java ---------------------------------------------------------------------- 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 e18518e..e4763a5 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); http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java ---------------------------------------------------------------------- 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 cd967ed..1812d8c 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); } } http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java ---------------------------------------------------------------------- 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 70f4cd4..1252501 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 @@ -1040,10 +1040,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); http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java ---------------------------------------------------------------------- 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 0e552d6..52046c5 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(); } http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MirroringTableStateManager.java ---------------------------------------------------------------------- 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 752c941..417e11d 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, http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableNamespaceManager.java ---------------------------------------------------------------------- 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 f334d32..f63a7b0 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() { http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableStateManager.java ---------------------------------------------------------------------- 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 affb684..8a33de6 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 http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java ---------------------------------------------------------------------- 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 fd31553..0ece343 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; } http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java ---------------------------------------------------------------------- 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 33a8d21..754731b 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<byte[]> moveAsync(final RegionPlan regionPlan) { + public Future<byte[]> 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> regionInfo) { + public MoveRegionProcedure[] createReopenProcedures(final Collection<RegionInfo> 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<ServerName> 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); } } http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java ---------------------------------------------------------------------- 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 052ba7f..b94c0d8 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. + * <p>Throws exception on construction if determines context hostile to merge (cluster going + * down or master is shutting down or table is disabled).</p> */ @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; } http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java ---------------------------------------------------------------------- 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 a29bfee..065987f 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. + * + * <p>Throws exception on construction if determines context hostile to move (cluster going + * down or master is shutting down or table is disabled).</p> */ @InterfaceAudience.Private public class MoveRegionProcedure extends AbstractStateMachineRegionProcedure<MoveRegionState> { @@ -51,9 +55,15 @@ public class MoveRegionProcedure extends AbstractStateMachineRegionProcedure<Mov super(); } - public MoveRegionProcedure(final MasterProcedureEnv env, final RegionPlan plan) { + /** + * @throws IOException If the cluster is offline or master is stopping or if table is disabled + * or non-existent. + */ + public MoveRegionProcedure(final MasterProcedureEnv env, final RegionPlan plan) + throws HBaseIOException { super(env, plan.getRegionInfo()); this.plan = plan; + preflightChecks(env, true); } @Override http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java index 70ddbe5..8bbc428 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java @@ -86,6 +86,8 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos.S * The procedure to split a region in a table. * Takes lock on the parent region. * It holds the lock for the life of the procedure. + * <p>Throws exception on construction if determines context hostile to spllt (cluster going + * down or master is shutting down or table is disabled).</p> */ @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(); http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineTableProcedure.java ---------------------------------------------------------------------- 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 833b659..3182d45 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<TState> 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 <code>enabled</code>, 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); + } + } } http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteNamespaceProcedure.java ---------------------------------------------------------------------- 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 5f7959e..8369a19 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: http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DisableTableProcedure.java ---------------------------------------------------------------------- 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 e748c6c..413e3ae 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; } } http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.java ---------------------------------------------------------------------- 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 c501e53..c46070c 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; } } http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java ---------------------------------------------------------------------- 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 f0be1e0..1f1ba3c 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() { http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java ---------------------------------------------------------------------- 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 9aa5171..09f6259 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, null/*Table can be online when restore is called?*/); // Snapshot information this.snapshot = snapshot; this.restoreAcl = restoreAcl; http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TruncateTableProcedure.java ---------------------------------------------------------------------- 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 80cc5a8..4b2c21f 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; } http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ---------------------------------------------------------------------- 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 81febc0..e03757f 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()); } http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java ---------------------------------------------------------------------- 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 60bd215..023efd9 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(); } http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/main/resources/hbase-webapps/master/rsgroup.jsp ---------------------------------------------------------------------- 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 43753a5..d866008 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 @@ <tr> <td><%= tableName.getNamespaceAsString() %></td> <td><a href="/table.jsp?name=<%= tableName.getNameAsString() %>"><%= tableName.getQualifierAsString() %></a></td> - <% 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()) { %> - <td style="color:red;"><%= tableState.name() %></td> + <td style="color:red;"><%= tableState.getState().name() %></td> <% } else { %> - <td><%= tableState.name() %></td> + <td><%= tableState.getState().name() %></td> <% } %> <% Map<RegionState.State, List<RegionInfo>> tableRegions = master.getAssignmentManager().getRegionStates().getRegionByStateOfTable(tableName); http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java ---------------------------------------------------------------------- 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 f1e020f..879b592 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; + } } http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/test/java/org/apache/hadoop/hbase/master/AbstractTestDLS.java ---------------------------------------------------------------------- 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 d3d5ce1..bc4d32c 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"); http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java ---------------------------------------------------------------------- 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 4e66676..cc2f43d 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 http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java ---------------------------------------------------------------------- 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 cabd769..d366b67 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; + } } http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/MockMasterServices.java ---------------------------------------------------------------------- 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 6cd399d..346abba 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<MasterProcedureEnv> 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; } http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java ---------------------------------------------------------------------- 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 9546b19..c8bb97d 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, http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDisableTableProcedure.java ---------------------------------------------------------------------- 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 437cda2..2694d35 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 @@ -79,16 +79,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 +110,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); } http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestTruncateTableProcedure.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestTruncateTableProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestTruncateTableProcedure.java index d7f385c..acd883d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestTruncateTableProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestTruncateTableProcedure.java @@ -56,14 +56,22 @@ public class TestTruncateTableProcedure extends TestTableDDLProcedureBase { final TableName tableName = TableName.valueOf(name.getMethodName()); final ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor(); - long procId = ProcedureTestingUtility.submitAndWait(procExec, - new TruncateTableProcedure(procExec.getEnvironment(), tableName, true)); - - // Second delete should fail with TableNotFound - Procedure<?> result = procExec.getResult(procId); - assertTrue(result.isFailed()); - LOG.debug("Truncate failed with exception: " + result.getException()); - assertTrue(ProcedureTestingUtility.getExceptionCause(result) instanceof TableNotFoundException); + // HBASE-20178 has us fail-fast, in the constructor, so add try/catch for this case. + // Keep old way of looking at procedure too. + Throwable cause = null; + try { + long procId = ProcedureTestingUtility.submitAndWait(procExec, + new TruncateTableProcedure(procExec.getEnvironment(), tableName, true)); + + // Second delete should fail with TableNotFound + Procedure<?> result = procExec.getResult(procId); + assertTrue(result.isFailed()); + cause = ProcedureTestingUtility.getExceptionCause(result); + } catch (Throwable t) { + cause = t; + } + LOG.debug("Truncate failed with exception: " + cause); + assertTrue(cause instanceof TableNotFoundException); } @Test @@ -73,15 +81,22 @@ public class TestTruncateTableProcedure extends TestTableDDLProcedureBase { final ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor(); MasterProcedureTestingUtility.createTable(procExec, tableName, null, "f"); - long procId = ProcedureTestingUtility.submitAndWait(procExec, - new TruncateTableProcedure(procExec.getEnvironment(), tableName, false)); - - // Second delete should fail with TableNotDisabled - Procedure<?> result = procExec.getResult(procId); - assertTrue(result.isFailed()); - LOG.debug("Truncate failed with exception: " + result.getException()); - assertTrue( - ProcedureTestingUtility.getExceptionCause(result) instanceof TableNotDisabledException); + // HBASE-20178 has us fail-fast, in the constructor, so add try/catch for this case. + // Keep old way of looking at procedure too. + Throwable cause = null; + try { + long procId = ProcedureTestingUtility.submitAndWait(procExec, + new TruncateTableProcedure(procExec.getEnvironment(), tableName, false)); + + // Second delete should fail with TableNotDisabled + Procedure<?> result = procExec.getResult(procId); + assertTrue(result.isFailed()); + cause = ProcedureTestingUtility.getExceptionCause(result); + } catch (Throwable t) { + cause = t; + } + LOG.debug("Truncate failed with exception: " + cause); + assertTrue(cause instanceof TableNotDisabledException); } @Test http://git-wip-us.apache.org/repos/asf/hbase/blob/63ce0c07/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMove.java ---------------------------------------------------------------------- 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 0000000..8940bea --- /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<RegionInfo> 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())); + } +}
