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 13c49dfa48e..46c4c5e545e 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 @@ -54,7 +54,7 @@ public abstract class StateMachineProcedure private final AtomicBoolean aborted = new AtomicBoolean(false); private Flow stateFlow = Flow.HAS_MORE_STATE; - private int stateCount = 0; + protected int stateCount = 0; private int[] states = null; private List> subProcList = null; @@ -217,13 +217,13 @@ public abstract class StateMachineProcedure try { updateTimestamp(); rollbackState(env, getCurrentState()); - stateCount--; } finally { + stateCount--; updateTimestamp(); } } - private boolean isEofState() { + protected boolean isEofState() { return stateCount > 0 && states[stateCount-1] == EOF_STATE; } diff --git a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestStateMachineProcedure.java b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestStateMachineProcedure.java index 8af8874d701..9545812c298 100644 --- a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestStateMachineProcedure.java +++ b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestStateMachineProcedure.java @@ -149,6 +149,24 @@ public class TestStateMachineProcedure { assertEquals(TEST_FAILURE_EXCEPTION, cause); } + @Test + public void testChildNormalRollbackStateCount() { + procExecutor.getEnvironment().triggerChildRollback = true; + TestSMProcedureBadRollback testNormalRollback = new TestSMProcedureBadRollback(); + long procId = procExecutor.submitProcedure(testNormalRollback); + ProcedureTestingUtility.waitProcedure(procExecutor, procId); + assertEquals(0, testNormalRollback.stateCount); + } + + @Test + public void testChildBadRollbackStateCount() { + procExecutor.getEnvironment().triggerChildRollback = true; + TestSMProcedureBadRollback testBadRollback = new TestSMProcedureBadRollback(); + long procId = procExecutor.submitProcedure(testBadRollback); + ProcedureTestingUtility.waitProcedure(procExecutor, procId); + assertEquals(0, testBadRollback.stateCount); + } + @Test public void testChildOnLastStepWithRollbackDoubleExecution() throws Exception { procExecutor.getEnvironment().triggerChildRollback = true; @@ -208,6 +226,64 @@ public class TestStateMachineProcedure { } } + public static class TestSMProcedureBadRollback + extends StateMachineProcedure { + @Override + protected Flow executeFromState(TestProcEnv env, TestSMProcedureState state) { + LOG.info("EXEC " + state + " " + this); + env.execCount.incrementAndGet(); + switch (state) { + case STEP_1: + if (!env.loop) { + setNextState(TestSMProcedureState.STEP_2); + } + break; + case STEP_2: + addChildProcedure(new SimpleChildProcedure()); + return Flow.NO_MORE_STATE; + } + return Flow.HAS_MORE_STATE; + } + @Override + protected void rollbackState(TestProcEnv env, TestSMProcedureState state) { + LOG.info("ROLLBACK " + state + " " + this); + env.rollbackCount.incrementAndGet(); + } + + @Override + protected TestSMProcedureState getState(int stateId) { + return TestSMProcedureState.values()[stateId]; + } + + @Override + protected int getStateId(TestSMProcedureState state) { + return state.ordinal(); + } + + @Override + protected TestSMProcedureState getInitialState() { + return TestSMProcedureState.STEP_1; + } + + @Override + protected void rollback(final TestProcEnv env) + throws IOException, InterruptedException { + if (isEofState()) { + stateCount--; + } + try { + updateTimestamp(); + rollbackState(env, getCurrentState()); + throw new IOException(); + } catch(IOException e) { + //do nothing for now + } finally { + stateCount--; + updateTimestamp(); + } + } + } + public static class SimpleChildProcedure extends NoopProcedure { @Override protected Procedure[] execute(TestProcEnv env) { diff --git a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestYieldProcedures.java b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestYieldProcedures.java index aa03a47a109..e359e5cedfe 100644 --- a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestYieldProcedures.java +++ b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestYieldProcedures.java @@ -142,15 +142,17 @@ public class TestYieldProcedures { assertEquals(i, info.getStep().ordinal()); } - // test rollback (we execute steps twice, one has the IE the other completes) + // test rollback (we execute steps twice, rollback counts both IE and completed) for (int i = NUM_STATES - 1; i >= 0; --i) { TestStateMachineProcedure.ExecutionInfo info = proc.getExecutionInfo().get(count++); assertEquals(true, info.isRollback()); assertEquals(i, info.getStep().ordinal()); + } - info = proc.getExecutionInfo().get(count++); + for (int i = NUM_STATES - 1; i >= 0; --i) { + TestStateMachineProcedure.ExecutionInfo info = proc.getExecutionInfo().get(count++); assertEquals(true, info.isRollback()); - assertEquals(i, info.getStep().ordinal()); + assertEquals(0, info.getStep().ordinal()); } // check runnable queue stats