HBASE-19839 Fixed flakey tests TestMergeTableRegionsProcedure#testRollbackAndDoubleExecution and TestSplitTableRegionProcedure#testRollbackAndDoubleExecution
* Added a comment in MergeTableRegionsProcedure and SplitTableRegionProcedure explaining specific rollbacks has side effect that AssignProcedure/s are submitted asynchronously and those procedures may continue to execute even after rollback() is done. * Updated comments in tests with correct rollback state to abort * Added overloaded method MasterProcedureTestingUtility#testRollbackAndDoubleExecution which takes additional argument for waiting on all procedures to finish before asserting conditions * Updated TestMergeTableRegionsProcedure#testRollbackAndDoubleExecution and TestSplitTableRegionProcedure#testRollbackAndDoubleExecution to use newly added method Signed-off-by: Michael Stack <st...@apache.org> Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/57911d02 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/57911d02 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/57911d02 Branch: refs/heads/HBASE-19064 Commit: 57911d02c65b80eb198530b2a7d1fa39dba11a2c Parents: b21b8bf Author: Umesh Agashe <uaga...@cloudera.com> Authored: Mon Jan 22 13:23:41 2018 -0800 Committer: Michael Stack <st...@apache.org> Committed: Thu Feb 1 12:01:30 2018 -0800 ---------------------------------------------------------------------- .../assignment/MergeTableRegionsProcedure.java | 6 ++++++ .../assignment/SplitTableRegionProcedure.java | 6 ++++++ .../TestMergeTableRegionsProcedure.java | 7 ++++--- .../TestSplitTableRegionProcedure.java | 7 ++++--- .../procedure/MasterProcedureTestingUtility.java | 19 +++++++++++++++++++ 5 files changed, 39 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/57911d02/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 8c59776..4bccab7 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 @@ -263,6 +263,12 @@ public class MergeTableRegionsProcedure return Flow.HAS_MORE_STATE; } + /** + * To rollback {@link MergeTableRegionsProcedure}, two AssignProcedures are asynchronously + * submitted for each region to be merged (rollback doesn't wait on the completion of the + * AssignProcedures) . This can be improved by changing rollback() to support sub-procedures. + * See HBASE-19851 for details. + */ @Override protected void rollbackState( final MasterProcedureEnv env, http://git-wip-us.apache.org/repos/asf/hbase/blob/57911d02/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 1513c25..88e6012 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 @@ -263,6 +263,12 @@ public class SplitTableRegionProcedure return Flow.HAS_MORE_STATE; } + /** + * To rollback {@link SplitTableRegionProcedure}, an AssignProcedure is asynchronously + * submitted for parent region to be split (rollback doesn't wait on the completion of the + * AssignProcedure) . This can be improved by changing rollback() to support sub-procedures. + * See HBASE-19851 for details. + */ @Override protected void rollbackState(final MasterProcedureEnv env, final SplitTableRegionState state) throws IOException, InterruptedException { http://git-wip-us.apache.org/repos/asf/hbase/blob/57911d02/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 5b70e20..3285d3d 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 @@ -266,11 +266,12 @@ public class TestMergeTableRegionsProcedure { long procId = procExec.submitProcedure( new MergeTableRegionsProcedure(procExec.getEnvironment(), regionsToMerge, true)); - // Failing before MERGE_TABLE_REGIONS_UPDATE_META we should trigger the rollback - // NOTE: the 5 (number before MERGE_TABLE_REGIONS_UPDATE_META step) is + // Failing before MERGE_TABLE_REGIONS_CREATE_MERGED_REGION we should trigger the rollback + // NOTE: the 5 (number before MERGE_TABLE_REGIONS_CREATE_MERGED_REGION step) is // hardcoded, so you have to look at this test at least once when you add a new step. int numberOfSteps = 5; - MasterProcedureTestingUtility.testRollbackAndDoubleExecution(procExec, procId, numberOfSteps); + MasterProcedureTestingUtility.testRollbackAndDoubleExecution(procExec, procId, numberOfSteps, + true); } @Test http://git-wip-us.apache.org/repos/asf/hbase/blob/57911d02/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestSplitTableRegionProcedure.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestSplitTableRegionProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestSplitTableRegionProcedure.java index 1edb8e5..32b7539 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestSplitTableRegionProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestSplitTableRegionProcedure.java @@ -366,12 +366,13 @@ public class TestSplitTableRegionProcedure { long procId = procExec.submitProcedure( new SplitTableRegionProcedure(procExec.getEnvironment(), regions[0], splitKey)); - // Failing before SPLIT_TABLE_REGION_UPDATE_META we should trigger the + // Failing before SPLIT_TABLE_REGION_CREATE_DAUGHTER_REGIONS we should trigger the // rollback - // NOTE: the 3 (number before SPLIT_TABLE_REGION_UPDATE_META step) is + // NOTE: the 3 (number before SPLIT_TABLE_REGION_CREATE_DAUGHTER_REGIONS step) is // hardcoded, so you have to look at this test at least once when you add a new step. int numberOfSteps = 3; - MasterProcedureTestingUtility.testRollbackAndDoubleExecution(procExec, procId, numberOfSteps); + MasterProcedureTestingUtility.testRollbackAndDoubleExecution(procExec, procId, numberOfSteps, + true); // check that we have only 1 region assertEquals(1, UTIL.getHBaseAdmin().getTableRegions(tableName).size()); List<HRegion> daughters = UTIL.getMiniHBaseCluster().getRegions(tableName); http://git-wip-us.apache.org/repos/asf/hbase/blob/57911d02/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 243bb14..fc8953b 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 @@ -427,6 +427,12 @@ public class MasterProcedureTestingUtility { public static void testRollbackAndDoubleExecution( final ProcedureExecutor<MasterProcedureEnv> procExec, final long procId, final int lastStep) throws Exception { + testRollbackAndDoubleExecution(procExec, procId, lastStep, false); + } + + public static void testRollbackAndDoubleExecution( + final ProcedureExecutor<MasterProcedureEnv> procExec, final long procId, + final int lastStep, boolean waitForAsyncProcs) throws Exception { // Execute up to last step testRecoveryAndDoubleExecution(procExec, procId, lastStep, false); @@ -448,6 +454,19 @@ public class MasterProcedureTestingUtility { assertTrue(procExec.unregisterListener(abortListener)); } + if (waitForAsyncProcs) { + // Sometimes there are other procedures still executing (including asynchronously spawned by + // procId) and due to KillAndToggleBeforeStoreUpdate flag ProcedureExecutor is stopped before + // store update. Let all pending procedures finish normally. + if (!procExec.isRunning()) { + LOG.warn("ProcedureExecutor not running, may have been stopped by pending procedure due to" + + " KillAndToggleBeforeStoreUpdate flag."); + ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(procExec, false); + restartMasterProcedureExecutor(procExec); + ProcedureTestingUtility.waitNoProcedureRunning(procExec); + } + } + assertEquals(true, procExec.isRunning()); ProcedureTestingUtility.assertIsAbortException(procExec.getResult(procId)); }
