diff --git a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java index af0a61bf1d2..665d22319ae 100644 --- a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java +++ b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java @@ -931,18 +931,18 @@ public class ProcedureExecutor { * Send an abort notification the specified procedure. * Depending on the procedure implementation the abort can be considered or ignored. * @param procId the procedure to abort - * @return true if the procedure exist and has received the abort, otherwise false. + * @return true if the procedure exists and has received the abort, otherwise false. */ public boolean abort(final long procId) { return abort(procId, true); } /** - * Send an abort notification the specified procedure. - * Depending on the procedure implementation the abort can be considered or ignored. + * Send an abort notification to the specified procedure. + * Depending on the procedure implementation, the abort can be considered or ignored. * @param procId the procedure to abort * @param mayInterruptIfRunning if the proc completed at least one step, should it be aborted? - * @return true if the procedure exist and has received the abort, otherwise false. + * @return true if the procedure exists and has received the abort, otherwise false. */ public boolean abort(final long procId, final boolean mayInterruptIfRunning) { final Procedure proc = procedures.get(procId); 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 20bba58f6a9..c530386d6e9 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 @@ -209,18 +209,13 @@ public abstract class StateMachineProcedure @Override protected boolean abort(final TEnvironment env) { - final boolean isDebugEnabled = LOG.isDebugEnabled(); final TState state = getCurrentState(); - if (isDebugEnabled) { - LOG.debug("abort requested for " + this + " state=" + state); - } - + LOG.debug("Abort requested for {}", this); if (hasMoreState()) { aborted.set(true); return true; - } else if (isDebugEnabled) { - LOG.debug("ignoring abort request on state=" + state + " for " + this); } + LOG.debug("Ignoring abort request on {}", this); return false; } 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 bd412081d8f..c65dbe5ff11 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 @@ -795,4 +795,12 @@ public class MergeTableRegionsProcedure public RegionInfo getMergedRegion() { return this.mergedRegion; } + + @Override + protected boolean abort(MasterProcedureEnv env) { + // Abort means rollback. We can't rollback all steps. HBASE-18018 added abort to all + // Procedures. Here is a Procedure that has a PONR and cannot be aborted wants it enters this + // range of steps; what do we do for these should an operator want to cancel them? HBASE-20022. + return isRollbackSupported(getCurrentState())? super.abort(env): false; + } } 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 e898d6abc90..f7f49bc5b2d 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 @@ -262,14 +262,13 @@ public class SplitTableRegionProcedure throw new UnsupportedOperationException(this + " unhandled state=" + state); } } catch (IOException e) { - String msg = "Error trying to split region " + getParentRegion().getEncodedName() + " in the table " - + getTableName() + " (in state=" + state + ")"; + String msg = "Error trying to split region " + getParentRegion().getEncodedName() + + " in the table " + getTableName() + " (in state=" + state + ")"; if (!isRollbackSupported(state)) { // We reach a state that cannot be rolled back. We just need to keep retry. LOG.warn(msg, e); } else { LOG.error(msg, e); - setFailure(e); setFailure("master-split-regions", e); } } @@ -831,4 +830,12 @@ public class SplitTableRegionProcedure } return traceEnabled; } + + @Override + protected boolean abort(MasterProcedureEnv env) { + // Abort means rollback. We can't rollback all steps. HBASE-18018 added abort to all + // Procedures. Here is a Procedure that has a PONR and cannot be aborted wants it enters this + // range of steps; what do we do for these should an operator want to cancel them? HBASE-20022. + return isRollbackSupported(getCurrentState())? super.abort(env): false; + } } 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 c519fe6e04f..487bb27f45b 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 @@ -144,8 +144,8 @@ public class DeleteTableProcedure @Override protected boolean abort(MasterProcedureEnv env) { - // TODO: Current behavior is: with no rollback and no abort support, procedure may stuck - // looping in retrying failing step forever. Default behavior of abort is changed to support + // TODO: Current behavior is: with no rollback and no abort support, procedure may get stuck + // looping in retrying failing a step forever. Default behavior of abort is changed to support // aborting all procedures. Override the default wisely. Following code retains the current // behavior. Revisit it later. return isRollbackSupported(getCurrentState()) ? super.abort(env) : false; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java index 64b399808f8..f56c34c2de3 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java @@ -568,7 +568,7 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { boolean hasLock = true; final LockAndQueue[] regionLocks = new LockAndQueue[regionInfo.length]; for (int i = 0; i < regionInfo.length; ++i) { - LOG.info(procedure + ", table=" + table + ", " + regionInfo[i].getRegionNameAsString()); + LOG.info(procedure + ", " + regionInfo[i].getRegionNameAsString()); assert table != null; assert regionInfo[i] != null; assert regionInfo[i].getTable() != null; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMergeTableRegionsProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMergeTableRegionsProcedure.java index 3285d3d298a..094a5a0cf1d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMergeTableRegionsProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMergeTableRegionsProcedure.java @@ -276,7 +276,7 @@ public class TestMergeTableRegionsProcedure { @Test public void testMergeWithoutPONR() throws Exception { - final TableName tableName = TableName.valueOf("testRecoveryAndDoubleExecution"); + final TableName tableName = TableName.valueOf("testMergeWithoutPONR"); final ProcedureExecutor procExec = getMasterProcedureExecutor(); List tableRegions = createTable(tableName); 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 fc8953b83e3..84b0094a6b8 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 @@ -401,7 +401,7 @@ public class MasterProcedureTestingUtility { * idempotent. Use this version of the test when the order in which flow steps are executed is * not start to finish; where the procedure may vary the flow steps dependent on circumstance * found. - * @see #testRecoveryAndDoubleExecution(ProcedureExecutor, long, int) + * @see #testRecoveryAndDoubleExecution(ProcedureExecutor, long, int, boolean) */ public static void testRecoveryAndDoubleExecution( final ProcedureExecutor procExec, final long procId) throws Exception {