This is an automated email from the ASF dual-hosted git repository. sakthi pushed a commit to branch branch-2 in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2 by this push: new 4ce1f9b HBASE-23727 Port HBASE-20981 in 2.2 & 2.3 4ce1f9b is described below commit 4ce1f9b832324fe1afa1d9b0fafa0b4a7c179c65 Author: jackbearden <j...@jackbearden.com> AuthorDate: Wed Aug 1 12:50:25 2018 -0700 HBASE-23727 Port HBASE-20981 in 2.2 & 2.3 HBASE-20981 - Rollback stateCount accounting thrown-off when exception out of rollbackState Signed-off-by: Michael Stack <st...@apache.org> Signed-off-by: Sakthi <sak...@apache.org> Signed-off-by: Peter Somogyi <psomo...@apache.org> (cherry picked from commit 8e8b9b698f8c3faf551a0457f5264c6dbfe47950) --- .../hbase/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 13c49df..46c4c5e 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<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; @@ -217,13 +217,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; } 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 8af8874..9545812 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 @@ -150,6 +150,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); @@ -208,6 +226,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<TestProcEnv>[] 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 aa03a47..e359e5c 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