Repository: hbase Updated Branches: refs/heads/branch-2.0 2a15e3a01 -> aa83594b8
HBASE-20981. Rollback stateCount accounting thrown-off when exception out of rollbackState 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/aa83594b Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/aa83594b Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/aa83594b Branch: refs/heads/branch-2.0 Commit: aa83594b843bd46fd710e5c52dd53101c21ad06f Parents: 2a15e3a Author: jackbearden <j...@jackbearden.com> Authored: Wed Aug 1 12:50:25 2018 -0700 Committer: Michael Stack <st...@apache.org> Committed: Sat Aug 11 11:46:11 2018 -0700 ---------------------------------------------------------------------- .../hbase/procedure2/StateMachineProcedure.java | 6 +- .../procedure2/TestStateMachineProcedure.java | 76 ++++++++++++++++++++ .../hbase/procedure2/TestYieldProcedures.java | 8 ++- 3 files changed, 84 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/aa83594b/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 0e3a4cd..15bd715 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 @@ -55,7 +55,7 @@ public abstract class StateMachineProcedure<TEnvironment, TState> 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<Procedure<TEnvironment>> subProcList = null; @@ -201,13 +201,13 @@ public abstract class StateMachineProcedure<TEnvironment, TState> try { updateTimestamp(); rollbackState(env, getCurrentState()); - stateCount--; } finally { + stateCount--; updateTimestamp(); } } - private boolean isEofState() { + protected boolean isEofState() { return stateCount > 0 && states[stateCount-1] == EOF_STATE; } http://git-wip-us.apache.org/repos/asf/hbase/blob/aa83594b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestStateMachineProcedure.java ---------------------------------------------------------------------- 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 07e4330..9160c90 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 @@ -142,6 +142,24 @@ public class TestStateMachineProcedure { } @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; ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(procExecutor, true); @@ -195,6 +213,64 @@ public class TestStateMachineProcedure { } } + public static class TestSMProcedureBadRollback + extends StateMachineProcedure<TestProcEnv, TestSMProcedureState> { + @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<TestProcEnv> { @Override protected Procedure[] execute(TestProcEnv env) { http://git-wip-us.apache.org/repos/asf/hbase/blob/aa83594b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestYieldProcedures.java ---------------------------------------------------------------------- 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 8d9b325..18d92ea 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 @@ -143,15 +143,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