diff --git a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/StateMachineProcedure.java b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/StateMachineProcedure.java index a403193..484720d 100644 --- a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/StateMachineProcedure.java +++ b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/StateMachineProcedure.java @@ -21,9 +21,12 @@ package org.apache.hadoop.hbase.procedure2; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.ArrayList; import java.util.Arrays; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.classification.InterfaceStability; import org.apache.hadoop.hbase.protobuf.generated.ProcedureProtos.StateMachineProcedureData; @@ -43,6 +46,10 @@ import org.apache.hadoop.hbase.protobuf.generated.ProcedureProtos.StateMachinePr @InterfaceStability.Evolving public abstract class StateMachineProcedure extends Procedure { + private static final Log LOG = LogFactory.getLog(StateMachineProcedure.class); + + private final AtomicBoolean aborted = new AtomicBoolean(false); + private Flow stateFlow = Flow.HAS_MORE_STATE; private int stateCount = 0; private int[] states = null; @@ -96,7 +103,11 @@ public abstract class StateMachineProcedure * @param state the state enum object */ protected void setNextState(final TState state) { - setNextState(getStateId(state)); + if (aborted.get()) { + setAbortFailure(getClass().getSimpleName(), "abort requested"); + } else { + setNextState(getStateId(state)); + } } /** @@ -129,7 +140,7 @@ public abstract class StateMachineProcedure throws ProcedureSuspendedException, ProcedureYieldException, InterruptedException { updateTimestamp(); try { - if (!hasMoreState()) return null; + if (!hasMoreState() || isFailed()) return null; TState state = getCurrentState(); if (stateCount == 0) { @@ -163,6 +174,25 @@ public abstract class StateMachineProcedure } @Override + protected boolean abort(final TEnvironment env) { + final TState state = getCurrentState(); + if (isRollbackSupported(env, state)) { + LOG.debug("abort requested for " + getClass().getSimpleName() + " state=" + state); + aborted.set(true); + return true; + } + return false; + } + + /** + * Used by the default implementation of abort() to know if the current state can be aborted + * and rollback can be triggered. + */ + protected boolean isRollbackSupported(final TEnvironment env, final TState state) { + return false; + } + + @Override protected boolean isYieldAfterExecutionStep(final TEnvironment env) { return isYieldBeforeExecuteFromState(env, getCurrentState()); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AddColumnFamilyProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AddColumnFamilyProcedure.java index 195f738..90c2fff 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AddColumnFamilyProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AddColumnFamilyProcedure.java @@ -22,7 +22,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.util.List; -import java.util.concurrent.atomic.AtomicBoolean; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -49,8 +48,6 @@ public class AddColumnFamilyProcedure implements TableProcedureInterface { private static final Log LOG = LogFactory.getLog(AddColumnFamilyProcedure.class); - private final AtomicBoolean aborted = new AtomicBoolean(false); - private TableName tableName; private HTableDescriptor unmodifiedHTableDescriptor; private HColumnDescriptor cfDescriptor; @@ -110,10 +107,12 @@ public class AddColumnFamilyProcedure throw new UnsupportedOperationException(this + " unhandled state=" + state); } } catch (IOException e) { - LOG.warn("Error trying to add the column family" + getColumnFamilyName() + " to the table " - + tableName + " (in state=" + state + ")", e); - - setFailure("master-add-columnfamily", e); + if (isRollbackSupported(env, state)) { + setFailure("master-add-columnfamily", e); + } else { + LOG.warn("Retriable error trying to add the column family " + getColumnFamilyName() + + " to the table " + tableName + " (in state=" + state + ")", e); + } } return Flow.HAS_MORE_STATE; } @@ -121,33 +120,26 @@ public class AddColumnFamilyProcedure @Override protected void rollbackState(final MasterProcedureEnv env, final AddColumnFamilyState state) throws IOException { - if (isTraceEnabled()) { - LOG.trace(this + " rollback state=" + state); + if (state == AddColumnFamilyState.ADD_COLUMN_FAMILY_PREPARE || + state == AddColumnFamilyState.ADD_COLUMN_FAMILY_PRE_OPERATION) { + // nothing to rollback, pre is just table-state checks. + // We can fail if the table does not exist or is not disabled. + // TODO: coprocessor rollback semantic is still undefined. + return; } - try { - switch (state) { - case ADD_COLUMN_FAMILY_REOPEN_ALL_REGIONS: - break; // Nothing to undo. - case ADD_COLUMN_FAMILY_POST_OPERATION: - // TODO-MAYBE: call the coprocessor event to undo? - break; - case ADD_COLUMN_FAMILY_UPDATE_TABLE_DESCRIPTOR: - restoreTableDescriptor(env); - break; - case ADD_COLUMN_FAMILY_PRE_OPERATION: - // TODO-MAYBE: call the coprocessor event to undo? - break; + + // The procedure doesn't have a rollback. The execution will succeed, at some point. + throw new UnsupportedOperationException("unhandled state=" + state); + } + + @Override + protected boolean isRollbackSupported(MasterProcedureEnv env, AddColumnFamilyState state) { + switch (state) { case ADD_COLUMN_FAMILY_PREPARE: - break; // nothing to do + case ADD_COLUMN_FAMILY_PRE_OPERATION: + return true; default: - throw new UnsupportedOperationException(this + " unhandled state=" + state); - } - } catch (IOException e) { - // This will be retried. Unless there is a bug in the code, - // this should be just a "temporary error" (e.g. network down) - LOG.warn("Failed rollback attempt step " + state + " for adding the column family" - + getColumnFamilyName() + " to the table " + tableName, e); - throw e; + return false; } } @@ -167,21 +159,6 @@ public class AddColumnFamilyProcedure } @Override - protected void setNextState(AddColumnFamilyState state) { - if (aborted.get()) { - setAbortFailure("add-columnfamily", "abort requested"); - } else { - super.setNextState(state); - } - } - - @Override - public boolean abort(final MasterProcedureEnv env) { - aborted.set(true); - return true; - } - - @Override protected boolean acquireLock(final MasterProcedureEnv env) { if (env.waitInitialized(this)) return false; return env.getProcedureQueue().tryAcquireTableExclusiveLock(this, tableName); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java index 861ac56..c8a804e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java @@ -27,7 +27,6 @@ import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.concurrent.atomic.AtomicBoolean; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -72,8 +71,6 @@ public class CloneSnapshotProcedure implements TableProcedureInterface { private static final Log LOG = LogFactory.getLog(CloneSnapshotProcedure.class); - private final AtomicBoolean aborted = new AtomicBoolean(false); - private User user; private HTableDescriptor hTableDescriptor; private SnapshotDescription snapshot; @@ -166,8 +163,12 @@ public class CloneSnapshotProcedure throw new UnsupportedOperationException("unhandled state=" + state); } } catch (IOException e) { - LOG.error("Error trying to create table=" + getTableName() + " state=" + state, e); - setFailure("master-create-table", e); + if (isRollbackSupported(env, state)) { + setFailure("master-clone-snapshot", e); + } else { + LOG.warn("Retriable error trying to clone snapshot=" + snapshot.getName() + + " to table=" + getTableName() + " state=" + state, e); + } } return Flow.HAS_MORE_STATE; } @@ -175,38 +176,23 @@ public class CloneSnapshotProcedure @Override protected void rollbackState(final MasterProcedureEnv env, final CloneSnapshotState state) throws IOException { - if (isTraceEnabled()) { - LOG.trace(this + " rollback state=" + state); + if (state == CloneSnapshotState.CLONE_SNAPSHOT_PRE_OPERATION) { + DeleteTableProcedure.deleteTableStates(env, getTableName()); + // TODO-MAYBE: call the deleteTable coprocessor event? + return; } - try { - switch (state) { - case CLONE_SNAPSHOT_POST_OPERATION: - // TODO-MAYBE: call the deleteTable coprocessor event? - break; - case CLONE_SNAPSHOT_UPDATE_DESC_CACHE: - DeleteTableProcedure.deleteTableDescriptorCache(env, getTableName()); - break; - case CLONE_SNAPSHOT_ASSIGN_REGIONS: - DeleteTableProcedure.deleteAssignmentState(env, getTableName()); - break; - case CLONE_SNAPSHOT_ADD_TO_META: - DeleteTableProcedure.deleteFromMeta(env, getTableName(), newRegions); - break; - case CLONE_SNAPSHOT_WRITE_FS_LAYOUT: - DeleteTableProcedure.deleteFromFs(env, getTableName(), newRegions, false); - break; - case CLONE_SNAPSHOT_PRE_OPERATION: - DeleteTableProcedure.deleteTableStates(env, getTableName()); - // TODO-MAYBE: call the deleteTable coprocessor event? - break; - default: - throw new UnsupportedOperationException("unhandled state=" + state); - } - } catch (IOException e) { - // This will be retried. Unless there is a bug in the code, - // this should be just a "temporary error" (e.g. network down) - LOG.warn("Failed rollback attempt step=" + state + " table=" + getTableName(), e); - throw e; + + // The procedure doesn't have a rollback. The execution will succeed, at some point. + throw new UnsupportedOperationException("unhandled state=" + state); + } + + @Override + protected boolean isRollbackSupported(MasterProcedureEnv env, CloneSnapshotState state) { + switch (state) { + case CLONE_SNAPSHOT_PRE_OPERATION: + return true; + default: + return false; } } @@ -226,15 +212,6 @@ public class CloneSnapshotProcedure } @Override - protected void setNextState(final CloneSnapshotState state) { - if (aborted.get()) { - setAbortFailure("clone-snapshot", "abort requested"); - } else { - super.setNextState(state); - } - } - - @Override public TableName getTableName() { return hTableDescriptor.getTableName(); } @@ -245,12 +222,6 @@ public class CloneSnapshotProcedure } @Override - public boolean abort(final MasterProcedureEnv env) { - aborted.set(true); - return true; - } - - @Override public void toStringClassDetails(StringBuilder sb) { sb.append(getClass().getSimpleName()); sb.append(" (table="); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateNamespaceProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateNamespaceProcedure.java index a9a0968..4c30ce6 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateNamespaceProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateNamespaceProcedure.java @@ -21,7 +21,6 @@ package org.apache.hadoop.hbase.master.procedure; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.concurrent.atomic.AtomicBoolean; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -46,8 +45,6 @@ public class CreateNamespaceProcedure implements TableProcedureInterface { private static final Log LOG = LogFactory.getLog(CreateNamespaceProcedure.class); - private final AtomicBoolean aborted = new AtomicBoolean(false); - private NamespaceDescriptor nsDescriptor; private Boolean traceEnabled; @@ -95,10 +92,12 @@ public class CreateNamespaceProcedure throw new UnsupportedOperationException(this + " unhandled state=" + state); } } catch (IOException e) { - LOG.warn("Error trying to create the namespace" + nsDescriptor.getName() - + " (in state=" + state + ")", e); - - setFailure("master-create-namespace", e); + if (isRollbackSupported(env, state)) { + setFailure("master-create-namespace", e); + } else { + LOG.warn("Retriable error trying to create namespace=" + nsDescriptor.getName() + + " (in state=" + state + ")", e); + } } return Flow.HAS_MORE_STATE; } @@ -106,34 +105,22 @@ public class CreateNamespaceProcedure @Override protected void rollbackState(final MasterProcedureEnv env, final CreateNamespaceState state) throws IOException { - if (isTraceEnabled()) { - LOG.trace(this + " rollback state=" + state); + if (state == CreateNamespaceState.CREATE_NAMESPACE_PREPARE) { + // nothing to rollback, pre-create is just state checks. + // TODO: coprocessor rollback semantic is still undefined. + return; } - try { - switch (state) { - case CREATE_NAMESPACE_SET_NAMESPACE_QUOTA: - rollbackSetNamespaceQuota(env); - break; - case CREATE_NAMESPACE_UPDATE_ZK: - rollbackZKNamespaceManagerChange(env); - break; - case CREATE_NAMESPACE_INSERT_INTO_NS_TABLE: - rollbackInsertIntoNSTable(env); - break; - case CREATE_NAMESPACE_CREATE_DIRECTORY: - rollbackCreateDirectory(env); - break; + // The procedure doesn't have a rollback. The execution will succeed, at some point. + throw new UnsupportedOperationException("unhandled state=" + state); + } + + @Override + protected boolean isRollbackSupported(MasterProcedureEnv env, CreateNamespaceState state) { + switch (state) { case CREATE_NAMESPACE_PREPARE: - break; // nothing to do + return true; default: - throw new UnsupportedOperationException(this + " unhandled state=" + state); - } - } catch (IOException e) { - // This will be retried. Unless there is a bug in the code, - // this should be just a "temporary error" (e.g. network down) - LOG.warn("Failed rollback attempt step " + state + " for creating the namespace " - + nsDescriptor.getName(), e); - throw e; + return false; } } @@ -153,21 +140,6 @@ public class CreateNamespaceProcedure } @Override - protected void setNextState(CreateNamespaceState state) { - if (aborted.get()) { - setAbortFailure("create-namespace", "abort requested"); - } else { - super.setNextState(state); - } - } - - @Override - public boolean abort(final MasterProcedureEnv env) { - aborted.set(true); - return true; - } - - @Override public void serializeStateData(final OutputStream stream) throws IOException { super.serializeStateData(stream); @@ -258,20 +230,6 @@ public class CreateNamespaceProcedure } /** - * undo create directory - * @param env MasterProcedureEnv - * @throws IOException - */ - private void rollbackCreateDirectory(final MasterProcedureEnv env) throws IOException { - try { - DeleteNamespaceProcedure.deleteDirectory(env, nsDescriptor.getName()); - } catch (Exception e) { - // Ignore exception - LOG.debug("Rollback of createDirectory throws exception: " + e); - } - } - - /** * Insert the row into ns table * @param env MasterProcedureEnv * @param nsDescriptor NamespaceDescriptor @@ -284,20 +242,6 @@ public class CreateNamespaceProcedure } /** - * Undo the insert. - * @param env MasterProcedureEnv - * @throws IOException - */ - private void rollbackInsertIntoNSTable(final MasterProcedureEnv env) throws IOException { - try { - DeleteNamespaceProcedure.deleteFromNSTable(env, nsDescriptor.getName()); - } catch (Exception e) { - // Ignore exception - LOG.debug("Rollback of insertIntoNSTable throws exception: " + e); - } - } - - /** * Update ZooKeeper. * @param env MasterProcedureEnv * @param nsDescriptor NamespaceDescriptor @@ -310,20 +254,6 @@ public class CreateNamespaceProcedure } /** - * rollback ZooKeeper update. - * @param env MasterProcedureEnv - * @throws IOException - */ - private void rollbackZKNamespaceManagerChange(final MasterProcedureEnv env) throws IOException { - try { - DeleteNamespaceProcedure.removeFromZKNamespaceManager(env, nsDescriptor.getName()); - } catch (Exception e) { - // Ignore exception - LOG.debug("Rollback of updateZKNamespaceManager throws exception: " + e); - } - } - - /** * Set quota for the namespace * @param env MasterProcedureEnv * @param nsDescriptor NamespaceDescriptor diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java index f6ade6e..e6eb96d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java @@ -24,7 +24,6 @@ import java.io.IOException; import java.security.PrivilegedExceptionAction; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.atomic.AtomicBoolean; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -61,8 +60,6 @@ public class CreateTableProcedure implements TableProcedureInterface { private static final Log LOG = LogFactory.getLog(CreateTableProcedure.class); - private final AtomicBoolean aborted = new AtomicBoolean(false); - // used for compatibility with old clients private final ProcedurePrepareLatch syncLatch; @@ -139,8 +136,11 @@ public class CreateTableProcedure throw new UnsupportedOperationException("unhandled state=" + state); } } catch (IOException e) { - LOG.error("Error trying to create table=" + getTableName() + " state=" + state, e); - setFailure("master-create-table", e); + if (isRollbackSupported(env, state)) { + setFailure("master-create-table", e); + } else { + LOG.warn("Retriable error trying to create table=" + getTableName() + " state=" + state, e); + } } return Flow.HAS_MORE_STATE; } @@ -148,38 +148,25 @@ public class CreateTableProcedure @Override protected void rollbackState(final MasterProcedureEnv env, final CreateTableState state) throws IOException { - if (LOG.isTraceEnabled()) { - LOG.trace(this + " rollback state=" + state); + if (state == CreateTableState.CREATE_TABLE_PRE_OPERATION) { + // nothing to rollback, pre-create is just table-state checks. + // We can fail if the table does exist or the descriptor is malformed. + // TODO: coprocessor rollback semantic is still undefined. + ProcedurePrepareLatch.releaseLatch(syncLatch, this); + return; } - try { - switch (state) { - case CREATE_TABLE_POST_OPERATION: - break; - case CREATE_TABLE_UPDATE_DESC_CACHE: - DeleteTableProcedure.deleteTableDescriptorCache(env, getTableName()); - break; - case CREATE_TABLE_ASSIGN_REGIONS: - DeleteTableProcedure.deleteAssignmentState(env, getTableName()); - break; - case CREATE_TABLE_ADD_TO_META: - DeleteTableProcedure.deleteFromMeta(env, getTableName(), newRegions); - break; - case CREATE_TABLE_WRITE_FS_LAYOUT: - DeleteTableProcedure.deleteFromFs(env, getTableName(), newRegions, false); - break; - case CREATE_TABLE_PRE_OPERATION: - DeleteTableProcedure.deleteTableStates(env, getTableName()); - // TODO-MAYBE: call the deleteTable coprocessor event? - ProcedurePrepareLatch.releaseLatch(syncLatch, this); - break; - default: - throw new UnsupportedOperationException("unhandled state=" + state); - } - } catch (IOException e) { - // This will be retried. Unless there is a bug in the code, - // this should be just a "temporary error" (e.g. network down) - LOG.warn("Failed rollback attempt step=" + state + " table=" + getTableName(), e); - throw e; + + // The procedure doesn't have a rollback. The execution will succeed, at some point. + throw new UnsupportedOperationException("unhandled state=" + state); + } + + @Override + protected boolean isRollbackSupported(MasterProcedureEnv env, final CreateTableState state) { + switch (state) { + case CREATE_TABLE_PRE_OPERATION: + return true; + default: + return false; } } @@ -199,15 +186,6 @@ public class CreateTableProcedure } @Override - protected void setNextState(final CreateTableState state) { - if (aborted.get()) { - setAbortFailure("create-table", "abort requested"); - } else { - super.setNextState(state); - } - } - - @Override public TableName getTableName() { return hTableDescriptor.getTableName(); } @@ -218,12 +196,6 @@ public class CreateTableProcedure } @Override - public boolean abort(final MasterProcedureEnv env) { - aborted.set(true); - return true; - } - - @Override public void toStringClassDetails(StringBuilder sb) { sb.append(getClass().getSimpleName()); sb.append(" (table="); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteColumnFamilyProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteColumnFamilyProcedure.java index 8bcbd82..edfc171 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteColumnFamilyProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteColumnFamilyProcedure.java @@ -23,7 +23,6 @@ import java.io.InputStream; import java.io.OutputStream; import java.security.PrivilegedExceptionAction; import java.util.List; -import java.util.concurrent.atomic.AtomicBoolean; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -51,8 +50,6 @@ public class DeleteColumnFamilyProcedure implements TableProcedureInterface { private static final Log LOG = LogFactory.getLog(DeleteColumnFamilyProcedure.class); - private final AtomicBoolean aborted = new AtomicBoolean(false); - private HTableDescriptor unmodifiedHTableDescriptor; private TableName tableName; private byte [] familyName; @@ -117,14 +114,11 @@ public class DeleteColumnFamilyProcedure throw new UnsupportedOperationException(this + " unhandled state=" + state); } } catch (IOException e) { - if (!isRollbackSupported(state)) { - // We reach a state that cannot be rolled back. We just need to keep retry. - LOG.warn("Error trying to delete the column family " + getColumnFamilyName() - + " from table " + tableName + "(in state=" + state + ")", e); + if (isRollbackSupported(env, state)) { + setFailure("master-delete-columnfamily", e); } else { - LOG.error("Error trying to delete the column family " + getColumnFamilyName() - + " from table " + tableName + "(in state=" + state + ")", e); - setFailure("master-delete-column-family", e); + LOG.warn("Retriable error trying to delete the column family " + getColumnFamilyName() + + " from table " + tableName + " (in state=" + state + ")", e); } } return Flow.HAS_MORE_STATE; @@ -133,39 +127,26 @@ public class DeleteColumnFamilyProcedure @Override protected void rollbackState(final MasterProcedureEnv env, final DeleteColumnFamilyState state) throws IOException { - if (isTraceEnabled()) { - LOG.trace(this + " rollback state=" + state); + if (state == DeleteColumnFamilyState.DELETE_COLUMN_FAMILY_PREPARE || + state == DeleteColumnFamilyState.DELETE_COLUMN_FAMILY_PRE_OPERATION) { + // nothing to rollback, pre is just table-state checks. + // We can fail if the table does not exist or is not disabled. + // TODO: coprocessor rollback semantic is still undefined. + return; } - try { - switch (state) { - case DELETE_COLUMN_FAMILY_REOPEN_ALL_REGIONS: - break; // Nothing to undo. - case DELETE_COLUMN_FAMILY_POST_OPERATION: - // TODO-MAYBE: call the coprocessor event to undo? - break; - case DELETE_COLUMN_FAMILY_DELETE_FS_LAYOUT: - // Once we reach to this state - we could NOT rollback - as it is tricky to undelete - // the deleted files. We are not suppose to reach here, throw exception so that we know - // there is a code bug to investigate. - throw new UnsupportedOperationException(this + " rollback of state=" + state - + " is unsupported."); - case DELETE_COLUMN_FAMILY_UPDATE_TABLE_DESCRIPTOR: - restoreTableDescriptor(env); - break; + + // The procedure doesn't have a rollback. The execution will succeed, at some point. + throw new UnsupportedOperationException("unhandled state=" + state); + } + + @Override + protected boolean isRollbackSupported(MasterProcedureEnv env, DeleteColumnFamilyState state) { + switch (state) { case DELETE_COLUMN_FAMILY_PRE_OPERATION: - // TODO-MAYBE: call the coprocessor event to undo? - break; case DELETE_COLUMN_FAMILY_PREPARE: - break; // nothing to do + return true; default: - throw new UnsupportedOperationException(this + " unhandled state=" + state); - } - } catch (IOException e) { - // This will be retried. Unless there is a bug in the code, - // this should be just a "temporary error" (e.g. network down) - LOG.warn("Failed rollback attempt step " + state + " for deleting the column family" - + getColumnFamilyName() + " to the table " + tableName, e); - throw e; + return false; } } @@ -185,21 +166,6 @@ public class DeleteColumnFamilyProcedure } @Override - protected void setNextState(DeleteColumnFamilyState state) { - if (aborted.get() && isRollbackSupported(state)) { - setAbortFailure("delete-columnfamily", "abort requested"); - } else { - super.setNextState(state); - } - } - - @Override - public boolean abort(final MasterProcedureEnv env) { - aborted.set(true); - return true; - } - - @Override protected boolean acquireLock(final MasterProcedureEnv env) { if (env.waitInitialized(this)) return false; return env.getProcedureQueue().tryAcquireTableExclusiveLock(this, tableName); @@ -416,22 +382,6 @@ public class DeleteColumnFamilyProcedure } } - /* - * Check whether we are in the state that can be rollback - */ - private boolean isRollbackSupported(final DeleteColumnFamilyState state) { - switch (state) { - case DELETE_COLUMN_FAMILY_REOPEN_ALL_REGIONS: - case DELETE_COLUMN_FAMILY_POST_OPERATION: - case DELETE_COLUMN_FAMILY_DELETE_FS_LAYOUT: - // It is not safe to rollback if we reach to these states. - return false; - default: - break; - } - return true; - } - private List getRegionInfoList(final MasterProcedureEnv env) throws IOException { if (regionInfoList == null) { regionInfoList = ProcedureSyncWait.getRegionsFromMeta(env, getTableName()); 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 bd71aa3..e6bc898 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 @@ -22,7 +22,6 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.concurrent.atomic.AtomicBoolean; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -52,8 +51,6 @@ public class DeleteNamespaceProcedure implements TableProcedureInterface { private static final Log LOG = LogFactory.getLog(DeleteNamespaceProcedure.class); - private final AtomicBoolean aborted = new AtomicBoolean(false); - private NamespaceDescriptor nsDescriptor; private String namespaceName; private Boolean traceEnabled; @@ -78,6 +75,7 @@ public class DeleteNamespaceProcedure if (isTraceEnabled()) { LOG.trace(this + " execute state=" + state); } + LOG.info(this + " execute state=" + state); try { switch (state) { @@ -104,10 +102,12 @@ public class DeleteNamespaceProcedure throw new UnsupportedOperationException(this + " unhandled state=" + state); } } catch (IOException e) { - LOG.warn("Error trying to delete the namespace " + namespaceName - + " (in state=" + state + ")", e); - - setFailure("master-delete-namespace", e); + if (isRollbackSupported(env, state)) { + setFailure("master-delete-namespace", e); + } else { + LOG.warn("Retriable error trying to delete namespace " + namespaceName + + " (in state=" + state + ")", e); + } } return Flow.HAS_MORE_STATE; } @@ -115,34 +115,24 @@ public class DeleteNamespaceProcedure @Override protected void rollbackState(final MasterProcedureEnv env, final DeleteNamespaceState state) throws IOException { - if (isTraceEnabled()) { - LOG.trace(this + " rollback state=" + state); + if (state == DeleteNamespaceState.DELETE_NAMESPACE_PREPARE) { + // nothing to rollback, pre is just table-state checks. + // We can fail if the table does not exist or is not disabled. + // TODO: coprocessor rollback semantic is still undefined. + return; } - try { - switch (state) { - case DELETE_NAMESPACE_REMOVE_NAMESPACE_QUOTA: - rollbacRemoveNamespaceQuota(env); - break; - case DELETE_NAMESPACE_DELETE_DIRECTORIES: - rollbackDeleteDirectory(env); - break; - case DELETE_NAMESPACE_REMOVE_FROM_ZK: - undoRemoveFromZKNamespaceManager(env); - break; - case DELETE_NAMESPACE_DELETE_FROM_NS_TABLE: - undoDeleteFromNSTable(env); - break; + + // The procedure doesn't have a rollback. The execution will succeed, at some point. + throw new UnsupportedOperationException("unhandled state=" + state); + } + + @Override + protected boolean isRollbackSupported(MasterProcedureEnv env, DeleteNamespaceState state) { + switch (state) { case DELETE_NAMESPACE_PREPARE: - break; // nothing to do + return true; default: - throw new UnsupportedOperationException(this + " unhandled state=" + state); - } - } catch (IOException e) { - // This will be retried. Unless there is a bug in the code, - // this should be just a "temporary error" (e.g. network down) - LOG.warn("Failed rollback attempt step " + state + " for deleting the namespace " - + namespaceName, e); - throw e; + return false; } } @@ -162,21 +152,6 @@ public class DeleteNamespaceProcedure } @Override - protected void setNextState(DeleteNamespaceState state) { - if (aborted.get()) { - setAbortFailure("delete-namespace", "abort requested"); - } else { - super.setNextState(state); - } - } - - @Override - public boolean abort(final MasterProcedureEnv env) { - aborted.set(true); - return true; - } - - @Override public void serializeStateData(final OutputStream stream) throws IOException { super.serializeStateData(stream); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java index 2881ed5..05b657f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java @@ -149,7 +149,11 @@ public class DeleteTableProcedure throw new UnsupportedOperationException("unhandled state=" + state); } } catch (HBaseException|IOException e) { - LOG.warn("Retriable error trying to delete table=" + getTableName() + " state=" + state, e); + if (isRollbackSupported(env, state)) { + setFailure("master-delete-table", e); + } else { + LOG.warn("Retriable error trying to delete table=" + getTableName() + " state=" + state, e); + } } return Flow.HAS_MORE_STATE; } @@ -159,6 +163,7 @@ public class DeleteTableProcedure if (state == DeleteTableState.DELETE_TABLE_PRE_OPERATION) { // nothing to rollback, pre-delete is just table-state checks. // We can fail if the table does not exist or is not disabled. + // TODO: coprocessor rollback semantic is still undefined. ProcedurePrepareLatch.releaseLatch(syncLatch, this); return; } @@ -168,6 +173,16 @@ public class DeleteTableProcedure } @Override + protected boolean isRollbackSupported(MasterProcedureEnv env, DeleteTableState state) { + switch (state) { + case DELETE_TABLE_PRE_OPERATION: + return true; + default: + return false; + } + } + + @Override protected DeleteTableState getState(final int stateId) { return DeleteTableState.valueOf(stateId); } @@ -193,12 +208,6 @@ public class DeleteTableProcedure } @Override - public boolean abort(final MasterProcedureEnv env) { - // TODO: We may be able to abort if the procedure is not started yet. - return false; - } - - @Override protected boolean acquireLock(final MasterProcedureEnv env) { if (env.waitInitialized(this)) return false; return env.getProcedureQueue().tryAcquireTableExclusiveLock(this, getTableName()); 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 be21590..2faedbe 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 @@ -24,7 +24,6 @@ import java.io.OutputStream; import java.security.PrivilegedExceptionAction; import java.util.List; import java.util.concurrent.ExecutorService; -import java.util.concurrent.atomic.AtomicBoolean; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -57,8 +56,6 @@ public class DisableTableProcedure implements TableProcedureInterface { private static final Log LOG = LogFactory.getLog(DisableTableProcedure.class); - private final AtomicBoolean aborted = new AtomicBoolean(false); - // This is for back compatible with 1.0 asynchronized operations. private final ProcedurePrepareLatch syncLatch; @@ -164,7 +161,12 @@ public class DisableTableProcedure throw new UnsupportedOperationException("unhandled state=" + state); } } catch (IOException e) { - LOG.warn("Retriable error trying to disable table=" + tableName + " state=" + state, e); + if (isRollbackSupported(env, state)) { + setFailure("master-disable-table", e); + } else { + LOG.warn("Retriable error trying to disable table=" + tableName + + " (in state=" + state + ")", e); + } } return Flow.HAS_MORE_STATE; } @@ -172,11 +174,16 @@ public class DisableTableProcedure @Override protected void rollbackState(final MasterProcedureEnv env, final DisableTableState state) throws IOException { - if (state == DisableTableState.DISABLE_TABLE_PREPARE) { - // nothing to rollback, prepare-disable is just table-state checks. - // We can fail if the table does not exist or is not disabled. - ProcedurePrepareLatch.releaseLatch(syncLatch, this); - return; + // nothing to rollback, prepare-disable is just table-state checks. + // We can fail if the table does not exist or is not disabled. + switch (state) { + case DISABLE_TABLE_PRE_OPERATION: + return; + case DISABLE_TABLE_PREPARE: + ProcedurePrepareLatch.releaseLatch(syncLatch, this); + return; + default: + break; } // The delete doesn't have a rollback. The execution will succeed, at some point. @@ -184,6 +191,17 @@ public class DisableTableProcedure } @Override + protected boolean isRollbackSupported(MasterProcedureEnv env, DisableTableState state) { + switch (state) { + case DISABLE_TABLE_PREPARE: + case DISABLE_TABLE_PRE_OPERATION: + return true; + default: + return false; + } + } + + @Override protected DisableTableState getState(final int stateId) { return DisableTableState.valueOf(stateId); } @@ -199,21 +217,6 @@ public class DisableTableProcedure } @Override - protected void setNextState(final DisableTableState state) { - if (aborted.get()) { - setAbortFailure("disable-table", "abort requested"); - } else { - super.setNextState(state); - } - } - - @Override - public boolean abort(final MasterProcedureEnv env) { - aborted.set(true); - return true; - } - - @Override protected boolean acquireLock(final MasterProcedureEnv env) { if (env.waitInitialized(this)) return false; return env.getProcedureQueue().tryAcquireTableExclusiveLock(this, tableName); 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 1893543..f9f8819 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 @@ -26,7 +26,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.concurrent.atomic.AtomicBoolean; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -60,8 +59,6 @@ public class EnableTableProcedure implements TableProcedureInterface { private static final Log LOG = LogFactory.getLog(EnableTableProcedure.class); - private final AtomicBoolean aborted = new AtomicBoolean(false); - // This is for back compatible with 1.0 asynchronized operations. private final ProcedurePrepareLatch syncLatch; @@ -157,8 +154,12 @@ public class EnableTableProcedure throw new UnsupportedOperationException("unhandled state=" + state); } } catch (IOException e) { - LOG.error("Error trying to enable table=" + tableName + " state=" + state, e); - setFailure("master-enable-table", e); + if (isRollbackSupported(env, state)) { + setFailure("master-enable-table", e); + } else { + LOG.warn("Retriable error trying to enable table=" + tableName + + " (in state=" + state + ")", e); + } } return Flow.HAS_MORE_STATE; } @@ -166,39 +167,30 @@ public class EnableTableProcedure @Override protected void rollbackState(final MasterProcedureEnv env, final EnableTableState state) throws IOException { - if (isTraceEnabled()) { - LOG.trace(this + " rollback state=" + state); - } - try { - switch (state) { - case ENABLE_TABLE_POST_OPERATION: - // TODO-MAYBE: call the coprocessor event to undo (eg. DisableTableProcedure.preDisable())? - break; - case ENABLE_TABLE_SET_ENABLED_TABLE_STATE: - DisableTableProcedure.setTableStateToDisabling(env, tableName); - break; - case ENABLE_TABLE_MARK_REGIONS_ONLINE: - markRegionsOfflineDuringRecovery(env); - break; - case ENABLE_TABLE_SET_ENABLING_TABLE_STATE: - DisableTableProcedure.setTableStateToDisabled(env, tableName); - break; + // nothing to rollback, prepare-disable is just table-state checks. + // We can fail if the table does not exist or is not disabled. + switch (state) { case ENABLE_TABLE_PRE_OPERATION: - // TODO-MAYBE: call the coprocessor event to undo (eg. DisableTableProcedure.postDisable())? - break; + return; case ENABLE_TABLE_PREPARE: - // Nothing to undo for this state. - // We do need to count down the latch count so that we don't stuck. ProcedurePrepareLatch.releaseLatch(syncLatch, this); + return; + default: break; + } + + // The delete doesn't have a rollback. The execution will succeed, at some point. + throw new UnsupportedOperationException("unhandled state=" + state); + } + + @Override + protected boolean isRollbackSupported(MasterProcedureEnv env, EnableTableState state) { + switch (state) { + case ENABLE_TABLE_PREPARE: + case ENABLE_TABLE_PRE_OPERATION: + return true; default: - throw new UnsupportedOperationException("unhandled state=" + state); - } - } catch (IOException e) { - // This will be retried. Unless there is a bug in the code, - // this should be just a "temporary error" (e.g. network down) - LOG.warn("Failed enable table rollback attempt step=" + state + " table=" + tableName, e); - throw e; + return false; } } @@ -218,21 +210,6 @@ public class EnableTableProcedure } @Override - protected void setNextState(final EnableTableState state) { - if (aborted.get()) { - setAbortFailure("Enable-table", "abort requested"); - } else { - super.setNextState(state); - } - } - - @Override - public boolean abort(final MasterProcedureEnv env) { - aborted.set(true); - return true; - } - - @Override protected boolean acquireLock(final MasterProcedureEnv env) { if (env.waitInitialized(this)) return false; return env.getProcedureQueue().tryAcquireTableExclusiveLock(this, tableName); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyColumnFamilyProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyColumnFamilyProcedure.java index 6a408da..779204d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyColumnFamilyProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyColumnFamilyProcedure.java @@ -23,7 +23,6 @@ import java.io.InputStream; import java.io.OutputStream; import java.security.PrivilegedExceptionAction; import java.util.List; -import java.util.concurrent.atomic.AtomicBoolean; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -50,8 +49,6 @@ public class ModifyColumnFamilyProcedure implements TableProcedureInterface { private static final Log LOG = LogFactory.getLog(ModifyColumnFamilyProcedure.class); - private final AtomicBoolean aborted = new AtomicBoolean(false); - private TableName tableName; private HTableDescriptor unmodifiedHTableDescriptor; private HColumnDescriptor cfDescriptor; @@ -108,10 +105,12 @@ public class ModifyColumnFamilyProcedure throw new UnsupportedOperationException(this + " unhandled state=" + state); } } catch (IOException e) { - LOG.warn("Error trying to modify the column family " + getColumnFamilyName() - + " of the table " + tableName + "(in state=" + state + ")", e); - - setFailure("master-modify-columnfamily", e); + if (isRollbackSupported(env, state)) { + setFailure("master-modify-columnfamily", e); + } else { + LOG.warn("Retriable error trying to disable table=" + tableName + + " (in state=" + state + ")", e); + } } return Flow.HAS_MORE_STATE; } @@ -119,33 +118,25 @@ public class ModifyColumnFamilyProcedure @Override protected void rollbackState(final MasterProcedureEnv env, final ModifyColumnFamilyState state) throws IOException { - if (isTraceEnabled()) { - LOG.trace(this + " rollback state=" + state); + if (state == ModifyColumnFamilyState.MODIFY_COLUMN_FAMILY_PREPARE || + state == ModifyColumnFamilyState.MODIFY_COLUMN_FAMILY_PRE_OPERATION) { + // nothing to rollback, pre-modify is just checks. + // TODO: coprocessor rollback semantic is still undefined. + return; } - try { - switch (state) { - case MODIFY_COLUMN_FAMILY_REOPEN_ALL_REGIONS: - break; // Nothing to undo. - case MODIFY_COLUMN_FAMILY_POST_OPERATION: - // TODO-MAYBE: call the coprocessor event to undo? - break; - case MODIFY_COLUMN_FAMILY_UPDATE_TABLE_DESCRIPTOR: - restoreTableDescriptor(env); - break; + + // The delete doesn't have a rollback. The execution will succeed, at some point. + throw new UnsupportedOperationException("unhandled state=" + state); + } + + @Override + protected boolean isRollbackSupported(MasterProcedureEnv env, ModifyColumnFamilyState state) { + switch (state) { case MODIFY_COLUMN_FAMILY_PRE_OPERATION: - // TODO-MAYBE: call the coprocessor event to undo? - break; case MODIFY_COLUMN_FAMILY_PREPARE: - break; // nothing to do + return true; default: - throw new UnsupportedOperationException(this + " unhandled state=" + state); - } - } catch (IOException e) { - // This will be retried. Unless there is a bug in the code, - // this should be just a "temporary error" (e.g. network down) - LOG.warn("Failed rollback attempt step " + state + " for adding the column family" - + getColumnFamilyName() + " to the table " + tableName, e); - throw e; + return false; } } @@ -165,21 +156,6 @@ public class ModifyColumnFamilyProcedure } @Override - protected void setNextState(ModifyColumnFamilyState state) { - if (aborted.get()) { - setAbortFailure("modify-columnfamily", "abort requested"); - } else { - super.setNextState(state); - } - } - - @Override - public boolean abort(final MasterProcedureEnv env) { - aborted.set(true); - return true; - } - - @Override protected boolean acquireLock(final MasterProcedureEnv env) { if (env.waitInitialized(this)) return false; return env.getProcedureQueue().tryAcquireTableExclusiveLock(this, tableName); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyNamespaceProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyNamespaceProcedure.java index 1f885bd..d7e36fd 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyNamespaceProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyNamespaceProcedure.java @@ -21,7 +21,6 @@ package org.apache.hadoop.hbase.master.procedure; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.concurrent.atomic.AtomicBoolean; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -44,8 +43,6 @@ public class ModifyNamespaceProcedure implements TableProcedureInterface { private static final Log LOG = LogFactory.getLog(ModifyNamespaceProcedure.class); - private final AtomicBoolean aborted = new AtomicBoolean(false); - private NamespaceDescriptor oldNsDescriptor; private NamespaceDescriptor newNsDescriptor; private Boolean traceEnabled; @@ -88,10 +85,12 @@ public class ModifyNamespaceProcedure throw new UnsupportedOperationException(this + " unhandled state=" + state); } } catch (IOException e) { - LOG.warn("Error trying to modify the namespace" + newNsDescriptor.getName() - + " (in state=" + state + ")", e); - - setFailure("master-modify-namespace", e); + if (isRollbackSupported(env, state)) { + setFailure("master-modify-namespace", e); + } else { + LOG.warn("Retriable error trying to modify namespace=" + newNsDescriptor.getName() + + " (in state=" + state + ")", e); + } } return Flow.HAS_MORE_STATE; } @@ -99,28 +98,23 @@ public class ModifyNamespaceProcedure @Override protected void rollbackState(final MasterProcedureEnv env, final ModifyNamespaceState state) throws IOException { - if (isTraceEnabled()) { - LOG.trace(this + " rollback state=" + state); + if (state == ModifyNamespaceState.MODIFY_NAMESPACE_PREPARE) { + // nothing to rollback, pre-modify is just checks. + // TODO: coprocessor rollback semantic is still undefined. + return; } - try { - switch (state) { - case MODIFY_NAMESPACE_UPDATE_ZK: - rollbackZKNamespaceManagerChange(env); - break; - case MODIFY_NAMESPACE_UPDATE_NS_TABLE: - rollbackUpdateInNSTable(env); - break; + + // The procedure doesn't have a rollback. The execution will succeed, at some point. + throw new UnsupportedOperationException("unhandled state=" + state); + } + + @Override + protected boolean isRollbackSupported(MasterProcedureEnv env, ModifyNamespaceState state) { + switch (state) { case MODIFY_NAMESPACE_PREPARE: - break; // nothing to do + return true; default: - throw new UnsupportedOperationException(this + " unhandled state=" + state); - } - } catch (IOException e) { - // This will be retried. Unless there is a bug in the code, - // this should be just a "temporary error" (e.g. network down) - LOG.warn("Failed rollback attempt step " + state + " for creating the namespace " - + newNsDescriptor.getName(), e); - throw e; + return false; } } @@ -140,21 +134,6 @@ public class ModifyNamespaceProcedure } @Override - protected void setNextState(ModifyNamespaceState state) { - if (aborted.get()) { - setAbortFailure("modify-namespace", "abort requested"); - } else { - super.setNextState(state); - } - } - - @Override - public boolean abort(final MasterProcedureEnv env) { - aborted.set(true); - return true; - } - - @Override public void serializeStateData(final OutputStream stream) throws IOException { super.serializeStateData(stream); @@ -240,17 +219,6 @@ public class ModifyNamespaceProcedure } /** - * rollback the row into namespace table - * @param env MasterProcedureEnv - * @throws IOException - */ - private void rollbackUpdateInNSTable(final MasterProcedureEnv env) throws IOException { - if (oldNsDescriptor != null) { - getTableNamespaceManager(env).insertIntoNSTable(oldNsDescriptor); - } - } - - /** * Update ZooKeeper. * @param env MasterProcedureEnv * @throws IOException @@ -259,17 +227,6 @@ public class ModifyNamespaceProcedure getTableNamespaceManager(env).updateZKNamespaceManager(newNsDescriptor); } - /** - * Update ZooKeeper during undo. - * @param env MasterProcedureEnv - * @throws IOException - */ - private void rollbackZKNamespaceManagerChange(final MasterProcedureEnv env) throws IOException { - if (oldNsDescriptor != null) { - getTableNamespaceManager(env).updateZKNamespaceManager(oldNsDescriptor); - } - } - private TableNamespaceManager getTableNamespaceManager(final MasterProcedureEnv env) { return env.getMasterServices().getClusterSchema().getTableNamespaceManager(); } 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 c523f23..446cbce 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 @@ -25,7 +25,6 @@ import java.security.PrivilegedExceptionAction; import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.concurrent.atomic.AtomicBoolean; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -58,8 +57,6 @@ public class ModifyTableProcedure implements TableProcedureInterface { private static final Log LOG = LogFactory.getLog(ModifyTableProcedure.class); - private final AtomicBoolean aborted = new AtomicBoolean(false); - private HTableDescriptor unmodifiedHTableDescriptor = null; private HTableDescriptor modifiedHTableDescriptor; private User user; @@ -132,12 +129,11 @@ public class ModifyTableProcedure throw new UnsupportedOperationException("unhandled state=" + state); } } catch (IOException e) { - if (!isRollbackSupported(state)) { - // We reach a state that cannot be rolled back. We just need to keep retry. - LOG.warn("Error trying to modify table=" + getTableName() + " state=" + state, e); - } else { - LOG.error("Error trying to modify table=" + getTableName() + " state=" + state, e); + if (isRollbackSupported(env, state)) { setFailure("master-modify-table", e); + } else { + LOG.warn("Retriable error trying to modify table=" + getTableName() + + " (in state=" + state + ")", e); } } return Flow.HAS_MORE_STATE; @@ -146,41 +142,25 @@ public class ModifyTableProcedure @Override protected void rollbackState(final MasterProcedureEnv env, final ModifyTableState state) throws IOException { - if (isTraceEnabled()) { - LOG.trace(this + " rollback state=" + state); + if (state == ModifyTableState.MODIFY_TABLE_PREPARE || + state == ModifyTableState.MODIFY_TABLE_PRE_OPERATION) { + // nothing to rollback, pre-modify is just checks. + // TODO: coprocessor rollback semantic is still undefined. + return; } - try { - switch (state) { - case MODIFY_TABLE_REOPEN_ALL_REGIONS: - break; // Nothing to undo. - case MODIFY_TABLE_POST_OPERATION: - // TODO-MAYBE: call the coprocessor event to un-modify? - break; - case MODIFY_TABLE_DELETE_FS_LAYOUT: - // Once we reach to this state - we could NOT rollback - as it is tricky to undelete - // the deleted files. We are not suppose to reach here, throw exception so that we know - // there is a code bug to investigate. - assert deleteColumnFamilyInModify; - throw new UnsupportedOperationException(this + " rollback of state=" + state - + " is unsupported."); - case MODIFY_TABLE_REMOVE_REPLICA_COLUMN: - // Undo the replica column update. - updateReplicaColumnsIfNeeded(env, modifiedHTableDescriptor, unmodifiedHTableDescriptor); - break; - case MODIFY_TABLE_UPDATE_TABLE_DESCRIPTOR: - restoreTableDescriptor(env); - break; + + // The delete doesn't have a rollback. The execution will succeed, at some point. + throw new UnsupportedOperationException("unhandled state=" + state); + } + + @Override + protected boolean isRollbackSupported(MasterProcedureEnv env, ModifyTableState state) { + switch (state) { case MODIFY_TABLE_PRE_OPERATION: - // TODO-MAYBE: call the coprocessor event to un-modify? - break; case MODIFY_TABLE_PREPARE: - break; // Nothing to undo. + return true; default: - throw new UnsupportedOperationException("unhandled state=" + state); - } - } catch (IOException e) { - LOG.warn("Fail trying to rollback modify table=" + getTableName() + " state=" + state, e); - throw e; + return false; } } @@ -200,21 +180,6 @@ public class ModifyTableProcedure } @Override - protected void setNextState(final ModifyTableState state) { - if (aborted.get() && isRollbackSupported(state)) { - setAbortFailure("modify-table", "abort requested"); - } else { - super.setNextState(state); - } - } - - @Override - public boolean abort(final MasterProcedureEnv env) { - aborted.set(true); - return true; - } - - @Override protected boolean acquireLock(final MasterProcedureEnv env) { if (env.waitInitialized(this)) return false; return env.getProcedureQueue().tryAcquireTableExclusiveLock(this, getTableName()); @@ -480,24 +445,6 @@ public class ModifyTableProcedure } } - /* - * Check whether we are in the state that can be rollback - */ - private boolean isRollbackSupported(final ModifyTableState state) { - if (deleteColumnFamilyInModify) { - switch (state) { - case MODIFY_TABLE_DELETE_FS_LAYOUT: - case MODIFY_TABLE_POST_OPERATION: - case MODIFY_TABLE_REOPEN_ALL_REGIONS: - // It is not safe to rollback if we reach to these states. - return false; - default: - break; - } - } - return true; - } - private List getRegionInfoList(final MasterProcedureEnv env) throws IOException { if (regionInfoList == null) { regionInfoList = ProcedureSyncWait.getRegionsFromMeta(env, getTableName()); 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 23ab3ac..cb94f25 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 @@ -26,7 +26,6 @@ import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.concurrent.atomic.AtomicBoolean; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -66,8 +65,6 @@ public class RestoreSnapshotProcedure implements TableProcedureInterface { private static final Log LOG = LogFactory.getLog(RestoreSnapshotProcedure.class); - private final AtomicBoolean aborted = new AtomicBoolean(false); - private HTableDescriptor modifiedHTableDescriptor; private List regionsToRestore = null; private List regionsToRemove = null; @@ -156,8 +153,12 @@ public class RestoreSnapshotProcedure throw new UnsupportedOperationException("unhandled state=" + state); } } catch (IOException e) { - LOG.error("Error trying to restore snapshot=" + getTableName() + " state=" + state, e); - setFailure("master-restore-snapshot", e); + if (isRollbackSupported(env, state)) { + setFailure("master-restore-snapshot", e); + } else { + LOG.warn("Retriable error trying to restore snapshot=" + snapshot.getName() + + " to table=" + getTableName() + " (in state=" + state + ")", e); + } } return Flow.HAS_MORE_STATE; } @@ -165,10 +166,6 @@ public class RestoreSnapshotProcedure @Override protected void rollbackState(final MasterProcedureEnv env, final RestoreSnapshotState state) throws IOException { - if (isTraceEnabled()) { - LOG.trace(this + " rollback state=" + state); - } - if (state == RestoreSnapshotState.RESTORE_SNAPSHOT_PRE_OPERATION) { // nothing to rollback return; @@ -179,6 +176,16 @@ public class RestoreSnapshotProcedure } @Override + protected boolean isRollbackSupported(MasterProcedureEnv env, RestoreSnapshotState state) { + switch (state) { + case RESTORE_SNAPSHOT_PRE_OPERATION: + return true; + default: + return false; + } + } + + @Override protected RestoreSnapshotState getState(final int stateId) { return RestoreSnapshotState.valueOf(stateId); } @@ -194,15 +201,6 @@ public class RestoreSnapshotProcedure } @Override - protected void setNextState(final RestoreSnapshotState state) { - if (aborted.get()) { - setAbortFailure("create-table", "abort requested"); - } else { - super.setNextState(state); - } - } - - @Override public TableName getTableName() { return modifiedHTableDescriptor.getTableName(); } @@ -214,8 +212,8 @@ public class RestoreSnapshotProcedure @Override public boolean abort(final MasterProcedureEnv env) { - aborted.set(true); - return true; + // TODO: We may be able to abort if the procedure is not started yet. + return false; } @Override 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 0b60cea..15be33d 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 @@ -132,7 +132,11 @@ public class TruncateTableProcedure throw new UnsupportedOperationException("unhandled state=" + state); } } catch (HBaseException|IOException e) { - LOG.warn("Retriable error trying to truncate table=" + getTableName() + " state=" + state, e); + if (isRollbackSupported(env, state)) { + setFailure("master-truncate-table", e); + } else { + LOG.warn("Retriable error trying to truncate table=" + getTableName() + " state=" + state, e); + } } return Flow.HAS_MORE_STATE; } @@ -142,6 +146,7 @@ public class TruncateTableProcedure if (state == TruncateTableState.TRUNCATE_TABLE_PRE_OPERATION) { // nothing to rollback, pre-truncate is just table-state checks. // We can fail if the table does not exist or is not disabled. + // TODO: coprocessor rollback semantic is still undefined. return; } @@ -150,6 +155,16 @@ public class TruncateTableProcedure } @Override + protected boolean isRollbackSupported(MasterProcedureEnv env, TruncateTableState state) { + switch (state) { + case TRUNCATE_TABLE_PRE_OPERATION: + return true; + default: + return false; + } + } + + @Override protected TruncateTableState getState(final int stateId) { return TruncateTableState.valueOf(stateId); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestAddColumnFamilyProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestAddColumnFamilyProcedure.java index a98d468..775d380 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestAddColumnFamilyProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestAddColumnFamilyProcedure.java @@ -280,7 +280,7 @@ public class TestAddColumnFamilyProcedure { nonceGroup, nonce); - int numberOfSteps = AddColumnFamilyState.values().length - 2; // failing in the middle of proc + int numberOfSteps = 1; // failing at "pre operations" MasterProcedureTestingUtility.testRollbackAndDoubleExecution(procExec, procId, numberOfSteps, AddColumnFamilyState.values()); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCloneSnapshotProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCloneSnapshotProcedure.java index 090a00b..a92e96b 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCloneSnapshotProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCloneSnapshotProcedure.java @@ -93,6 +93,12 @@ public class TestCloneSnapshotProcedure { @After public void tearDown() throws Exception { resetProcExecutorTestingKillFlag(); + TableName[] tables = UTIL.getHBaseAdmin().listTableNames(); + for (int i = 0; i < tables.length; ++i) { + UTIL.deleteTable(tables[i]); + } + SnapshotTestingUtils.deleteAllSnapshots(UTIL.getHBaseAdmin()); + snapshot = null; } private void resetProcExecutorTestingKillFlag() { @@ -238,7 +244,7 @@ public class TestCloneSnapshotProcedure { long procId = procExec.submitProcedure( new CloneSnapshotProcedure(procExec.getEnvironment(), htd, snapshotDesc), nonceGroup, nonce); - int numberOfSteps = CloneSnapshotState.values().length - 2; // failing in the middle of proc + int numberOfSteps = 0; // failing at pre operation MasterProcedureTestingUtility.testRollbackAndDoubleExecution( procExec, procId, diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateNamespaceProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateNamespaceProcedure.java index c01755f..e4dd168 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateNamespaceProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateNamespaceProcedure.java @@ -265,7 +265,7 @@ public class TestCreateNamespaceProcedure { nonceGroup, nonce); - int numberOfSteps = CreateNamespaceState.values().length - 2; // failing in the middle of proc + int numberOfSteps = 0; // failing at pre operation MasterProcedureTestingUtility.testRollbackAndDoubleExecution( procExec, procId, diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java index 5cec469..7718d0c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java @@ -230,66 +230,10 @@ public class TestCreateTableProcedure { testRollbackAndDoubleExecution(htd); } - @Test(timeout=90000) - public void testRollbackRetriableFailure() throws Exception { - final TableName tableName = TableName.valueOf("testRollbackRetriableFailure"); - - // create the table - final ProcedureExecutor procExec = getMasterProcedureExecutor(); - ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(procExec, true); - - // Start the Create procedure && kill the executor - final byte[][] splitKeys = new byte[][] { - Bytes.toBytes("a"), Bytes.toBytes("b"), Bytes.toBytes("c") - }; - HTableDescriptor htd = MasterProcedureTestingUtility.createHTD(tableName, "f1", "f2"); - HRegionInfo[] regions = ModifyRegionUtils.createHRegionInfos(htd, splitKeys); - long procId = procExec.submitProcedure( - new FaultyCreateTableProcedure(procExec.getEnvironment(), htd, regions), nonceGroup, nonce); - - // NOTE: the 4 (number of CreateTableState steps) is hardcoded, - // so you have to look at this test at least once when you add a new step. - MasterProcedureTestingUtility.testRollbackRetriableFailure( - procExec, procId, 4, CreateTableState.values()); - - MasterProcedureTestingUtility.validateTableDeletion( - UTIL.getHBaseCluster().getMaster(), tableName); - - // are we able to create the table after a rollback? - resetProcExecutorTestingKillFlag(); - testSimpleCreate(tableName, splitKeys); - } - private ProcedureExecutor getMasterProcedureExecutor() { return UTIL.getHBaseCluster().getMaster().getMasterProcedureExecutor(); } - public static class FaultyCreateTableProcedure extends CreateTableProcedure { - private int retries = 0; - - public FaultyCreateTableProcedure() { - // Required by the Procedure framework to create the procedure on replay - } - - public FaultyCreateTableProcedure(final MasterProcedureEnv env, - final HTableDescriptor hTableDescriptor, final HRegionInfo[] newRegions) - throws IOException { - super(env, hTableDescriptor, newRegions); - } - - @Override - protected void rollbackState(final MasterProcedureEnv env, final CreateTableState state) - throws IOException { - if (retries++ < 3) { - LOG.info("inject rollback failure state=" + state); - throw new IOException("injected failure number " + retries); - } else { - super.rollbackState(env, state); - retries = 0; - } - } - } - private void testRollbackAndDoubleExecution(HTableDescriptor htd) throws Exception { // create the table final ProcedureExecutor procExec = getMasterProcedureExecutor(); @@ -304,10 +248,9 @@ public class TestCreateTableProcedure { long procId = procExec.submitProcedure( new CreateTableProcedure(procExec.getEnvironment(), htd, regions), nonceGroup, nonce); - // NOTE: the 4 (number of CreateTableState steps) is hardcoded, - // so you have to look at this test at least once when you add a new step. + int numberOfSteps = 0; // failing at pre operation MasterProcedureTestingUtility.testRollbackAndDoubleExecution( - procExec, procId, 4, CreateTableState.values()); + procExec, procId, numberOfSteps, CreateTableState.values()); TableName tableName = htd.getTableName(); MasterProcedureTestingUtility.validateTableDeletion( UTIL.getHBaseCluster().getMaster(), tableName); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDeleteColumnFamilyProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDeleteColumnFamilyProcedure.java index 3980274..bd33533 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDeleteColumnFamilyProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDeleteColumnFamilyProcedure.java @@ -302,10 +302,7 @@ public class TestDeleteColumnFamilyProcedure { nonceGroup, nonce); - // Failing before DELETE_COLUMN_FAMILY_DELETE_FS_LAYOUT we should trigger the rollback - // NOTE: the 1 (number before DELETE_COLUMN_FAMILY_DELETE_FS_LAYOUT step) is hardcoded, - // so you have to look at this test at least once when you add a new step. - int numberOfSteps = 1; + int numberOfSteps = 1; // failing at pre operation MasterProcedureTestingUtility.testRollbackAndDoubleExecution( procExec, procId, @@ -316,39 +313,6 @@ public class TestDeleteColumnFamilyProcedure { UTIL.getHBaseCluster().getMaster(), tableName, regions, "f1", "f2", "f3", cf5); } - @Test(timeout = 60000) - public void testRollbackAndDoubleExecutionAfterPONR() throws Exception { - final TableName tableName = TableName.valueOf("testRollbackAndDoubleExecutionAfterPONR"); - final String cf5 = "cf5"; - - final ProcedureExecutor procExec = getMasterProcedureExecutor(); - - // create the table - HRegionInfo[] regions = MasterProcedureTestingUtility.createTable( - procExec, tableName, null, "f1", "f2", "f3", cf5); - ProcedureTestingUtility.waitNoProcedureRunning(procExec); - ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(procExec, true); - - // Start the Delete procedure && kill the executor - long procId = procExec.submitProcedure( - new DeleteColumnFamilyProcedure(procExec.getEnvironment(), tableName, cf5.getBytes()), - nonceGroup, - nonce); - - // Failing after DELETE_COLUMN_FAMILY_DELETE_FS_LAYOUT we should not trigger the rollback. - // NOTE: the 4 (number of DELETE_COLUMN_FAMILY_DELETE_FS_LAYOUT + 1 step) is hardcoded, - // so you have to look at this test at least once when you add a new step. - int numberOfSteps = 4; - MasterProcedureTestingUtility.testRollbackAndDoubleExecutionAfterPONR( - procExec, - procId, - numberOfSteps, - DeleteColumnFamilyState.values()); - - MasterProcedureTestingUtility.validateColumnFamilyDeletion( - UTIL.getHBaseCluster().getMaster(), tableName, cf5); - } - private ProcedureExecutor getMasterProcedureExecutor() { return UTIL.getHBaseCluster().getMaster().getMasterProcedureExecutor(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDeleteNamespaceProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDeleteNamespaceProcedure.java index 4c5f87b..9414dc2 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDeleteNamespaceProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDeleteNamespaceProcedure.java @@ -242,7 +242,7 @@ public class TestDeleteNamespaceProcedure { nonceGroup, nonce); - int numberOfSteps = DeleteNamespaceState.values().length - 2; // failing in the middle of proc + int numberOfSteps = 0; // failing at pre operation MasterProcedureTestingUtility.testRollbackAndDoubleExecution( procExec, procId, diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestEnableTableProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestEnableTableProcedure.java index 5c2aa29..5b27bb0 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestEnableTableProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestEnableTableProcedure.java @@ -211,7 +211,7 @@ public class TestEnableTableProcedure { long procId = procExec.submitProcedure( new EnableTableProcedure(procExec.getEnvironment(), tableName, false), nonceGroup, nonce); - int numberOfSteps = EnableTableState.values().length - 2; // failing in the middle of proc + int numberOfSteps = 1; // failing at pre operation MasterProcedureTestingUtility.testRollbackAndDoubleExecution( procExec, procId, diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyColumnFamilyProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyColumnFamilyProcedure.java index e983459..f045547 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyColumnFamilyProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyColumnFamilyProcedure.java @@ -241,8 +241,7 @@ public class TestModifyColumnFamilyProcedure { nonceGroup, nonce); - // Failing in the middle of proc - int numberOfSteps = ModifyColumnFamilyState.values().length - 2; + int numberOfSteps = 1; // failing at pre operation MasterProcedureTestingUtility.testRollbackAndDoubleExecution( procExec, procId, diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyNamespaceProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyNamespaceProcedure.java index 9208df7..8e6c2d7 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyNamespaceProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyNamespaceProcedure.java @@ -264,8 +264,7 @@ public class TestModifyNamespaceProcedure { nonceGroup, nonce); - // Failing in the middle of proc - int numberOfSteps = ModifyNamespaceState.values().length - 2; + int numberOfSteps = 0; // failing at pre operation MasterProcedureTestingUtility.testRollbackAndDoubleExecution( procExec, procId, diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java index ab86eda..28ca250 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java @@ -334,8 +334,7 @@ public class TestModifyTableProcedure { long procId = procExec.submitProcedure( new ModifyTableProcedure(procExec.getEnvironment(), htd), nonceGroup, nonce); - // Restart the executor and rollback the step twice - int numberOfSteps = ModifyTableState.values().length - 4; // failing in the middle of proc + int numberOfSteps = 1; // failing at pre operation MasterProcedureTestingUtility.testRollbackAndDoubleExecution( procExec, procId, @@ -372,7 +371,7 @@ public class TestModifyTableProcedure { new ModifyTableProcedure(procExec.getEnvironment(), htd), nonceGroup, nonce); // Restart the executor and rollback the step twice - int numberOfSteps = ModifyTableState.values().length - 4; // failing in the middle of proc + int numberOfSteps = 1; // failing at pre operation MasterProcedureTestingUtility.testRollbackAndDoubleExecution( procExec, procId, @@ -409,10 +408,7 @@ public class TestModifyTableProcedure { long procId = procExec.submitProcedure( new ModifyTableProcedure(procExec.getEnvironment(), htd), nonceGroup, nonce); - // Failing after MODIFY_TABLE_DELETE_FS_LAYOUT we should not trigger the rollback. - // NOTE: the 5 (number of MODIFY_TABLE_DELETE_FS_LAYOUT + 1 step) is hardcoded, - // so you have to look at this test at least once when you add a new step. - int numberOfSteps = 5; + int numberOfSteps = 2; // failing at pre operation MasterProcedureTestingUtility.testRollbackAndDoubleExecutionAfterPONR( procExec, procId, diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestProcedureAdmin.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestProcedureAdmin.java index 31190c1..ae6d27f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestProcedureAdmin.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestProcedureAdmin.java @@ -131,8 +131,13 @@ public class TestProcedureAdmin { // Submit an un-abortable procedure long procId = procExec.submitProcedure( new DeleteTableProcedure(procExec.getEnvironment(), tableName), nonceGroup, nonce); - // Wait for one step to complete + // Wait for a couple of steps to complete (first step "prepare" is abortable) ProcedureTestingUtility.waitProcedure(procExec, procId); + for (int i = 0; i < 2; ++i) { + ProcedureTestingUtility.assertProcNotYetCompleted(procExec, procId); + ProcedureTestingUtility.restart(procExec); + ProcedureTestingUtility.waitProcedure(procExec, procId); + } boolean abortResult = procExec.abort(procId, true); assertFalse(abortResult);