From 3a7c280603befac786f24454bd2c8f21804c4cdc Mon Sep 17 00:00:00 2001 From: Umesh Agashe Date: Tue, 27 Feb 2018 08:25:37 -0800 Subject: [PATCH] HBASE-20024 Fixed flakyness of TestMergeTableRegionsProcedure We assumed that we can run for loop from 0 to lastStep sequentially. MergeTableRegionProcedure skips step 2. So, when i is 0 the procedure is already at step 3. Added a method StateMachineProcedure#getCurrentStateId that can be used from test code only. --- .../hadoop/hbase/procedure2/StateMachineProcedure.java | 13 +++++++++++++ .../master/procedure/MasterProcedureTestingUtility.java | 16 ++++++++++++++-- 2 files changed, 27 insertions(+), 2 deletions(-) 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 c530386d6e91d446da3be4e7454b63d823aa07ea..91e65dacf9c356d45acfea204e6d95ca554b491d 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 @@ -28,6 +28,9 @@ import org.apache.yetus.audience.InterfaceAudience; import org.apache.yetus.audience.InterfaceStability; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + +import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; + import org.apache.hadoop.hbase.shaded.protobuf.generated.ProcedureProtos.StateMachineProcedureData; /** @@ -254,6 +257,16 @@ public abstract class StateMachineProcedure return stateCount > 0 ? getState(states[stateCount-1]) : getInitialState(); } + /** + * This method is used from test code as it cannot be assumed that state transition will happen + * sequentially. Some procedures may skip steps/ states, some may add intermediate steps in + * future. + */ + @VisibleForTesting + public int getCurrentStateId() { + return getStateId(getCurrentState()); + } + /** * Set the next state for the procedure. * @param stateId the ordinal() of the state enum (or state id) 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 84b0094a6b8b54378c730f3d20b1da4b1ec22bdd..d65568f3442e886a5a3fc57d08daca7f26791b54 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 @@ -54,8 +54,10 @@ import org.apache.hadoop.hbase.master.RegionState; import org.apache.hadoop.hbase.master.TableStateManager; import org.apache.hadoop.hbase.master.assignment.AssignmentManager; import org.apache.hadoop.hbase.monitoring.TaskMonitor; +import org.apache.hadoop.hbase.procedure2.Procedure; import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility; +import org.apache.hadoop.hbase.procedure2.StateMachineProcedure; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.FSUtils; import org.apache.hadoop.hbase.util.MD5Hash; @@ -377,11 +379,21 @@ public class MasterProcedureTestingUtility { // execute step N - kill before store update // restart executor/store // execute step N - save on store - for (int i = 0; i < numSteps; ++i) { - LOG.info("Restart " + i + " exec state=" + procExec.getProcedure(procId)); + // NOTE: currently we make assumption that states/ steps are sequential. There are already + // instances of a procedures which skip (don't use) intermediate states/ steps. In future, + // intermediate states/ steps can be added with ordinal greater than lastStep. + Procedure proc = procExec.getProcedure(procId); + int stepNum = proc instanceof StateMachineProcedure ? + ((StateMachineProcedure) proc).getCurrentStateId() : 0; + while (stepNum < numSteps) { + LOG.info("Restart " + stepNum + " exec state=" + proc); ProcedureTestingUtility.assertProcNotYetCompleted(procExec, procId); restartMasterProcedureExecutor(procExec); ProcedureTestingUtility.waitProcedure(procExec, procId); + // Old proc object is stale, need to get the new one after ProcedureExecutor restart + proc = procExec.getProcedure(procId); + stepNum = proc instanceof StateMachineProcedure ? + ((StateMachineProcedure) proc).getCurrentStateId() : stepNum + 1; } assertEquals(expectExecRunning, procExec.isRunning()); -- 2.14.3 (Apple Git-98)