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 8c597769d97..4bccab71f38 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 1513c25de8f..88e6012b149 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 5b70e204ba7..3285d3d298a 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 1edb8e5314c..32b7539e199 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 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 243bb1487d5..fc8953b83e3 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)); }