HBASE-20024 TestMergeTableRegionsProcedure is STILL flakey
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/51cea3e2 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/51cea3e2 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/51cea3e2 Branch: refs/heads/HBASE-19064 Commit: 51cea3e2c3c75b82148f14ca7903a88daa3738d9 Parents: 2b19698 Author: Michael Stack <st...@apache.org> Authored: Tue Feb 20 07:21:52 2018 -0800 Committer: Michael Stack <st...@apache.org> Committed: Tue Feb 20 11:08:27 2018 -0800 ---------------------------------------------------------------------- .../hadoop/hbase/procedure2/ProcedureExecutor.java | 8 ++++---- .../hadoop/hbase/procedure2/StateMachineProcedure.java | 9 ++------- .../master/assignment/MergeTableRegionsProcedure.java | 8 ++++++++ .../master/assignment/SplitTableRegionProcedure.java | 13 ++++++++++--- .../hbase/master/procedure/DeleteTableProcedure.java | 4 ++-- .../master/procedure/MasterProcedureScheduler.java | 2 +- .../assignment/TestMergeTableRegionsProcedure.java | 2 +- .../procedure/MasterProcedureTestingUtility.java | 2 +- 8 files changed, 29 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/51cea3e2/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java ---------------------------------------------------------------------- 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 af0a61b..665d223 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<TEnvironment> { * 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); http://git-wip-us.apache.org/repos/asf/hbase/blob/51cea3e2/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/StateMachineProcedure.java ---------------------------------------------------------------------- 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 20bba58..c530386 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<TEnvironment, TState> @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; } http://git-wip-us.apache.org/repos/asf/hbase/blob/51cea3e2/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 bd41208..c65dbe5 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; + } } http://git-wip-us.apache.org/repos/asf/hbase/blob/51cea3e2/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 e898d6a..f7f49bc 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; + } } http://git-wip-us.apache.org/repos/asf/hbase/blob/51cea3e2/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java ---------------------------------------------------------------------- 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 c519fe6..487bb27 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; http://git-wip-us.apache.org/repos/asf/hbase/blob/51cea3e2/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java ---------------------------------------------------------------------- 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 5cc9298..120bba3 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 @@ -633,7 +633,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; http://git-wip-us.apache.org/repos/asf/hbase/blob/51cea3e2/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMergeTableRegionsProcedure.java ---------------------------------------------------------------------- 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 3285d3d..094a5a0 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<MasterProcedureEnv> procExec = getMasterProcedureExecutor(); List<RegionInfo> tableRegions = createTable(tableName); http://git-wip-us.apache.org/repos/asf/hbase/blob/51cea3e2/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 fc8953b..84b0094 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<MasterProcedureEnv> procExec, final long procId) throws Exception {