You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by st...@apache.org on 2018/03/09 20:45:00 UTC

hbase git commit: HBASE-20024 Fixed flakyness of TestMergeTableRegionsProcedure

Repository: hbase
Updated Branches:
  refs/heads/branch-2 cdf7be892 -> 55e3dda25


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.


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/55e3dda2
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/55e3dda2
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/55e3dda2

Branch: refs/heads/branch-2
Commit: 55e3dda25d5e558442d8e4899010938e63a9d3f7
Parents: cdf7be8
Author: Umesh Agashe <ua...@cloudera.com>
Authored: Tue Feb 27 08:25:37 2018 -0800
Committer: Michael Stack <st...@apache.org>
Committed: Fri Mar 9 12:44:49 2018 -0800

----------------------------------------------------------------------
 .../hbase/procedure2/StateMachineProcedure.java | 13 +++++++++++
 .../MasterProcedureTestingUtility.java          | 23 ++++++++++++++++++--
 2 files changed, 34 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/55e3dda2/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 0880238..222ae93 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<TEnvironment, TState>
   }
 
   /**
+   * 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)
    */

http://git-wip-us.apache.org/repos/asf/hbase/blob/55e3dda2/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 84b0094..9546b19 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,28 @@ 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. If and when
+    // that happens the states can not be treated as sequential steps and the condition in
+    // following while loop needs to be changed. We can use euqals/ not equals operator to check
+    // if the procedure has reached the user specified state. But there is a possibility that
+    // while loop may not get the control back exaclty when the procedure is in lastStep. Proper
+    // fix would be get all visited states by the procedure and then check if user speccified
+    // state is in that list. Current assumption of sequential proregression of steps/ states is
+    // made at multiple places so we can keep while condition below for simplicity.
+    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());