From 22c8a2f7f53c41f1befe6ded9b28593dad01e5c0 Mon Sep 17 00:00:00 2001 From: jackbearden Date: Wed, 1 Aug 2018 12:50:25 -0700 Subject: [PATCH] HBASE-20981. Rollback stateCount accounting thrown-off when exception out of rollbackState Signed-off-by: Michael Stack --- .../procedure2/StateMachineProcedure.java | 6 +- .../procedure2/TestStateMachineProcedure.java | 76 +++++++++++++++++++ .../hbase/procedure2/TestYieldProcedures.java | 8 +- 3 files changed, 84 insertions(+), 6 deletions(-) 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 13c49dfa48e6266ba0dd0b5bf4fa26da3ffdfa5f..46c4c5e545ea0817c08542445754b19e1d81c265 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 8af8874d70118c5f45ad26f2223e1a84975cd42f..9545812c298934210d5803ff0a695bbe07b001db 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 aa03a47a10949ff906162fd0b5abbce6f4d9325c..e359e5cedfe60eaebfd82d1fff8c80a752265378 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 -- 2.20.1 (Apple Git-117)