From d806e1423c489502f0beaece4d0fd29b5b32b93d Mon Sep 17 00:00:00 2001 From: Umesh Agashe Date: Mon, 22 Jan 2018 13:23:41 -0800 Subject: [PATCH] HBASE-19839 Fixed flakey tests TestMergeTableRegionsProcedure#testRollbackAndDoubleExecution and TestSplitTableRegionProcedure#testRollbackAndDoubleExecution --- .../master/assignment/MergeTableRegionsProcedure.java | 6 ++++++ .../master/assignment/SplitTableRegionProcedure.java | 6 ++++++ .../assignment/TestMergeTableRegionsProcedure.java | 7 ++++--- .../assignment/TestSplitTableRegionProcedure.java | 9 +++++---- .../procedure/MasterProcedureTestingUtility.java | 19 +++++++++++++++++++ 5 files changed, 40 insertions(+), 7 deletions(-) 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 8c597769d97c7ea188ae6a53551380b8f7eeda20..4bccab71f381f6b333f97b0a4db8cb7591206a72 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, 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 1513c25de8f748099fef6ece5776a3132fcb6c55..88e6012b149ba763018f2160939c2f1812bd785d 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 { 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 5b70e204ba764f97ecb1da0288d2dcb9a6a01b74..3285d3d298a33ec34980b27c284cb307eef08cbc 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 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 1edb8e5314c2164a7d452aacd9ba8e6826c280b4..814c97843524aaf29b704c756f660213a3607aae 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 4 (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); + int numberOfSteps = 4; + MasterProcedureTestingUtility.testRollbackAndDoubleExecution(procExec, procId, numberOfSteps, + true); // check that we have only 1 region assertEquals(1, UTIL.getHBaseAdmin().getTableRegions(tableName).size()); List daughters = UTIL.getMiniHBaseCluster().getRegions(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 243bb1487d53fd554ef3fe02f4d099ce581697cd..fc8953b83e377d6ad9bc3aefff0bf5f0930e84a6 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 procExec, final long procId, final int lastStep) throws Exception { + testRollbackAndDoubleExecution(procExec, procId, lastStep, false); + } + + public static void testRollbackAndDoubleExecution( + final ProcedureExecutor 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)); } -- 2.10.1 (Apple Git-78)