From 4008436d75d8498911f25adeb22d46fe82ab480c Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Fri, 7 Aug 2015 11:11:46 +0100 Subject: [PATCH] Better handling of zk bad version - Remove propagating BadVersionException from ZookeeperStateMachineEnsemble via event so that machine can be kept running. Further logs are then read anyway via watcher. Thought we still need better handling but this is better for now. - Relates to #92 --- .../test/StateMachineTestPlan.java | 18 +++-- .../test/StateMachineTestPlanBuilder.java | 33 +++++++-- .../ZookeeperStateMachineEnsemble.java | 7 -- .../zookeeper/ZookeeperStateMachineTests.java | 73 +++++++++++++++++++ 4 files changed, 111 insertions(+), 20 deletions(-) diff --git a/spring-statemachine-test/src/main/java/org/springframework/statemachine/test/StateMachineTestPlan.java b/spring-statemachine-test/src/main/java/org/springframework/statemachine/test/StateMachineTestPlan.java index eac1da68..7f67e27b 100644 --- a/spring-statemachine-test/src/main/java/org/springframework/statemachine/test/StateMachineTestPlan.java +++ b/spring-statemachine-test/src/main/java/org/springframework/statemachine/test/StateMachineTestPlan.java @@ -18,6 +18,8 @@ package org.springframework.statemachine.test; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.empty; import static org.junit.Assert.assertThat; import java.util.ArrayList; @@ -112,15 +114,19 @@ public class StateMachineTestPlan { } if (step.sendEvent != null) { - StateMachine sendVia = null; + ArrayList> sendVia = new ArrayList>(); if (step.sendEventMachineId != null) { - sendVia = stateMachines.get(step.sendEventMachineId); + sendVia.add(stateMachines.get(step.sendEventMachineId)); + } else if (step.sendEventToAll) { + sendVia.addAll(stateMachines.values()); } else { - sendVia = stateMachines.values().iterator().next(); + sendVia.add(stateMachines.values().iterator().next()); + } + assertThat("Error finding machine to send via", sendVia, not(empty())); + for (StateMachine machine : sendVia) { + log.info("Sending test event " + step.sendEvent + " via machine " + machine); + machine.sendEvent(step.sendEvent); } - assertThat("Error finding machine to send via", sendVia, notNullValue()); - log.info("Sending test event " + step.sendEvent + " via machine " + sendVia); - sendVia.sendEvent(step.sendEvent); } if (step.expectStateChanged != null) { diff --git a/spring-statemachine-test/src/main/java/org/springframework/statemachine/test/StateMachineTestPlanBuilder.java b/spring-statemachine-test/src/main/java/org/springframework/statemachine/test/StateMachineTestPlanBuilder.java index 280d2129..20a8cd6c 100644 --- a/spring-statemachine-test/src/main/java/org/springframework/statemachine/test/StateMachineTestPlanBuilder.java +++ b/spring-statemachine-test/src/main/java/org/springframework/statemachine/test/StateMachineTestPlanBuilder.java @@ -111,6 +111,7 @@ public class StateMachineTestPlanBuilder { E sendEvent; Object sendEventMachineId; + boolean sendEventToAll = false; final Collection expectStates = new ArrayList(); Integer expectStateChanged; Integer expectStateEntered; @@ -156,7 +157,22 @@ public class StateMachineTestPlanBuilder { * @return the state machine test plan step builder */ public StateMachineTestPlanStepBuilder sendEvent(E event) { - return sendEvent(event, null); + return sendEvent(event, false); + } + + /** + * Send an event {@code E}. If {@code sendToAll} is set to {@code TRUE} event + * will be send to all existing machines. + * + * @param event the event + * @param sendToAll send to all machines + * @return the state machine test plan step builder + */ + public StateMachineTestPlanStepBuilder sendEvent(E event, boolean sendToAll) { + this.sendEvent = event; + this.sendEventMachineId = null; + this.sendEventToAll = sendToAll; + return this; } /** @@ -330,7 +346,7 @@ public class StateMachineTestPlanBuilder { * @return the state machine test plan builder for chaining */ public StateMachineTestPlanBuilder and() { - steps.add(new StateMachineTestPlanStep(sendEvent, sendEventMachineId, expectStates, + steps.add(new StateMachineTestPlanStep(sendEvent, sendEventMachineId, sendEventToAll, expectStates, expectStateChanged, expectStateEntered, expectStateExited, expectEventNotAccepted, expectTransition, expectTransitionStarted, expectTransitionEnded, expectStateMachineStarted, expectStateMachineStopped, expectVariableKeys, expectVariables)); @@ -342,6 +358,7 @@ public class StateMachineTestPlanBuilder { static class StateMachineTestPlanStep { E sendEvent; Object sendEventMachineId; + boolean sendEventToAll = false; final Collection expectStates; Integer expectStateChanged; Integer expectStateEntered; @@ -355,13 +372,15 @@ public class StateMachineTestPlanBuilder { final Collection expectVariableKeys; final Map expectVariables; - public StateMachineTestPlanStep(E sendEvent, Object sendEventMachineId, Collection expectStates, Integer expectStateChanged, - Integer expectStateEntered, Integer expectStateExited, Integer expectEventNotAccepted, - Integer expectTransition, Integer expectTransitionStarted, Integer expectTransitionEnded, - Integer expectStateMachineStarted, Integer expectStateMachineStopped, - Collection expectVariableKeys, Map expectVariables) { + public StateMachineTestPlanStep(E sendEvent, Object sendEventMachineId, boolean sendEventToAll, + Collection expectStates, Integer expectStateChanged, Integer expectStateEntered, + Integer expectStateExited, Integer expectEventNotAccepted, Integer expectTransition, + Integer expectTransitionStarted, Integer expectTransitionEnded, Integer expectStateMachineStarted, + Integer expectStateMachineStopped, Collection expectVariableKeys, + Map expectVariables) { this.sendEvent = sendEvent; this.sendEventMachineId = sendEventMachineId; + this.sendEventToAll = sendEventToAll; this.expectStates = expectStates; this.expectStateChanged = expectStateChanged; this.expectStateEntered = expectStateEntered; diff --git a/spring-statemachine-zookeeper/src/main/java/org/springframework/statemachine/zookeeper/ZookeeperStateMachineEnsemble.java b/spring-statemachine-zookeeper/src/main/java/org/springframework/statemachine/zookeeper/ZookeeperStateMachineEnsemble.java index b06327c8..535cad82 100644 --- a/spring-statemachine-zookeeper/src/main/java/org/springframework/statemachine/zookeeper/ZookeeperStateMachineEnsemble.java +++ b/spring-statemachine-zookeeper/src/main/java/org/springframework/statemachine/zookeeper/ZookeeperStateMachineEnsemble.java @@ -32,7 +32,6 @@ import org.apache.curator.framework.imps.CuratorFrameworkState; import org.apache.curator.framework.recipes.locks.InterProcessSemaphoreMutex; import org.apache.curator.framework.recipes.nodes.PersistentEphemeralNode; import org.apache.curator.framework.recipes.nodes.PersistentEphemeralNode.Mode; -import org.apache.zookeeper.KeeperException; import org.apache.zookeeper.WatchedEvent; import org.apache.zookeeper.data.Stat; import org.springframework.statemachine.StateMachine; @@ -189,12 +188,6 @@ public class ZookeeperStateMachineEnsemble extends StateMachineEnsembleObj persist.write(context, stat); stateRef.set(new StateWrapper(context, stat.getVersion())); } catch (Exception e) { - if (e instanceof StateMachineException) { - if (((StateMachineException)e).contains(KeeperException.BadVersionException.class)) { - notifyError(new StateMachineEnsembleException("Cas error during write id=[" + uuid - + "] for context=[" + context + "]", e)); - } - } throw new StateMachineException("Error persisting data", e); } } diff --git a/spring-statemachine-zookeeper/src/test/java/org/springframework/statemachine/zookeeper/ZookeeperStateMachineTests.java b/spring-statemachine-zookeeper/src/test/java/org/springframework/statemachine/zookeeper/ZookeeperStateMachineTests.java index d14b7872..52e64c0c 100644 --- a/spring-statemachine-zookeeper/src/test/java/org/springframework/statemachine/zookeeper/ZookeeperStateMachineTests.java +++ b/spring-statemachine-zookeeper/src/test/java/org/springframework/statemachine/zookeeper/ZookeeperStateMachineTests.java @@ -257,6 +257,51 @@ public class ZookeeperStateMachineTests extends AbstractZookeeperTests { plan.test(); } + @Test + public void testShouldHaveCasErrorDoesNotBreakMachines() throws Exception { + context.register(ZkServerConfig.class, BaseConfig.class); + context.refresh(); + + CuratorFramework curatorClient = + context.getBean("curatorClient", CuratorFramework.class); + + StateMachine machine1 = + buildTestStateMachine2(curatorClient); + StateMachine machine2 = + buildTestStateMachine2(curatorClient); + StateMachine machine3 = + buildTestStateMachine2(curatorClient); + StateMachine machine4 = + buildTestStateMachine2(curatorClient); + StateMachine machine5 = + buildTestStateMachine2(curatorClient); + + StateMachineTestPlan plan = + StateMachineTestPlanBuilder.builder() + .defaultAwaitTime(2) + .stateMachine(machine1) + .stateMachine(machine2) + .stateMachine(machine3) + .stateMachine(machine4) + .stateMachine(machine5) + .step() + .expectStates("SI") + .and() + .step() + .sendEvent("E1", true) + .expectStateChanged(1) + .expectStates("S1") + .and() + .step() + .sendEvent("E2", true) + .expectStateChanged(1) + .expectStates("S2") + .and() + .build(); + + plan.test(); + } + @Test @SuppressWarnings("unchecked") public void testJoinLaterShouldSyncState() throws Exception { @@ -550,6 +595,34 @@ public class ZookeeperStateMachineTests extends AbstractZookeeperTests { return builder.build(); } + private StateMachine buildTestStateMachine2(CuratorFramework curatorClient) + throws Exception { + StateMachineBuilder.Builder builder = StateMachineBuilder.builder(); + + builder.configureConfiguration() + .withConfiguration() + .taskExecutor(new SyncTaskExecutor()) + .autoStartup(true) + .and() + .withDistributed() + .ensemble(stateMachineEnsemble(curatorClient)); + + builder.configureStates() + .withStates() + .initial("SI") + .state("S1") + .state("S2"); + + builder.configureTransitions() + .withExternal() + .source("SI").target("S1").event("E1") + .and() + .withExternal() + .source("S1").target("S2").event("E2"); + + return builder.build(); + } + private static FooGuard foo0Guard() { return new FooGuard(0); }