Repository: brooklyn-server Updated Branches: refs/heads/master f22da254e -> adfa95d71
Fix out of order events triggered by ComputeServiceState ComputeServiceState.onEvent should only be called in the event handler executor to keep the ordering of events. Before the change what would happen is on calling setExpectedState(state) a "state.expected" event would be scheduled, with ComputeServiceState.onEvent(null) called in the same thread right after. This would create a race condition when calling setExpectedState in quick succession (onEvent called from the setExpectedState thread and from the event handler thread with different values). Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/33858931 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/33858931 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/33858931 Branch: refs/heads/master Commit: 33858931884831bd76af8b8c209aa6ab47194a79 Parents: 880f842 Author: Svetoslav Neykov <svetoslav.ney...@cloudsoftcorp.com> Authored: Tue Nov 22 12:18:54 2016 +0200 Committer: Svetoslav Neykov <svetoslav.ney...@cloudsoftcorp.com> Committed: Tue Nov 22 12:22:08 2016 +0200 ---------------------------------------------------------------------- .../entity/lifecycle/ServiceStateLogic.java | 24 ++++++------ .../entity/ApplicationLifecycleStateTest.java | 6 --- .../entity/lifecycle/ServiceStateLogicTest.java | 39 ++++++++++++++++++++ 3 files changed, 52 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/33858931/core/src/main/java/org/apache/brooklyn/core/entity/lifecycle/ServiceStateLogic.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/lifecycle/ServiceStateLogic.java b/core/src/main/java/org/apache/brooklyn/core/entity/lifecycle/ServiceStateLogic.java index df45d51..0839262 100644 --- a/core/src/main/java/org/apache/brooklyn/core/entity/lifecycle/ServiceStateLogic.java +++ b/core/src/main/java/org/apache/brooklyn/core/entity/lifecycle/ServiceStateLogic.java @@ -23,6 +23,7 @@ import java.util.Date; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.Nullable; @@ -45,7 +46,6 @@ import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.enricher.AbstractEnricher; import org.apache.brooklyn.core.entity.Attributes; import org.apache.brooklyn.core.entity.Entities; -import org.apache.brooklyn.core.entity.EntityAdjuncts; import org.apache.brooklyn.core.entity.EntityInternal; import org.apache.brooklyn.core.entity.EntityPredicates; import org.apache.brooklyn.core.entity.lifecycle.Lifecycle.Transition; @@ -72,6 +72,7 @@ import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.base.Stopwatch; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.reflect.TypeToken; @@ -154,11 +155,6 @@ public class ServiceStateLogic { public static void setExpectedState(Entity entity, Lifecycle state) { waitBrieflyForServiceUpIfStateIsRunning(entity, state); ((EntityInternal)entity).sensors().set(Attributes.SERVICE_STATE_EXPECTED, new Lifecycle.Transition(state, new Date())); - - Maybe<Enricher> enricher = EntityAdjuncts.tryFindWithUniqueTag(entity.enrichers(), ComputeServiceState.DEFAULT_ENRICHER_UNIQUE_TAG); - if (enricher.isPresent() && enricher.get() instanceof ComputeServiceState) { - ((ComputeServiceState)enricher.get()).onEvent(null); - } } public static Lifecycle getExpectedState(Entity entity) { @@ -258,9 +254,11 @@ public class ServiceStateLogic { * {@link ServiceStateLogic#newEnricherForServiceState(Class)} and added to an entity. */ public static class ComputeServiceState extends AbstractEnricher implements SensorEventListener<Object> { - + private static final Logger log = LoggerFactory.getLogger(ComputeServiceState.class); public static final String DEFAULT_ENRICHER_UNIQUE_TAG = "service.state.actual"; + private final AtomicInteger warnCounter = new AtomicInteger(); + public ComputeServiceState() {} public ComputeServiceState(Map<?,?> flags) { super(flags); } @@ -278,14 +276,18 @@ public class ServiceStateLogic { suppressDuplicates = true; } - subscriptions().subscribe(entity, SERVICE_PROBLEMS, this); - subscriptions().subscribe(entity, SERVICE_UP, this); - subscriptions().subscribe(entity, SERVICE_STATE_EXPECTED, this); - onEvent(null); + Map<String, ?> notifyOfInitialValue = ImmutableMap.of("notifyOfInitialValue", Boolean.TRUE); + subscriptions().subscribe(notifyOfInitialValue, entity, SERVICE_PROBLEMS, this); + subscriptions().subscribe(notifyOfInitialValue, entity, SERVICE_UP, this); + subscriptions().subscribe(notifyOfInitialValue, entity, SERVICE_STATE_EXPECTED, this); } @Override public void onEvent(@Nullable SensorEvent<Object> event) { + if (event == null && warnCounter.getAndIncrement() % 1000 == 0) { + log.warn("Deprecated since 0.10.0. Calling ServiceStateLogic.onEvent explicitly is deprecated to guarantee event ordering."); + } +// Preconditions.checkNotNull(event, "Calling onEvent explicitly no longer supported. Can only be called as an event handler to guarantee ordering."); Preconditions.checkNotNull(entity, "Cannot handle subscriptions or compute state until associated with an entity"); Map<String, Object> serviceProblems = entity.getAttribute(SERVICE_PROBLEMS); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/33858931/core/src/test/java/org/apache/brooklyn/core/entity/ApplicationLifecycleStateTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/entity/ApplicationLifecycleStateTest.java b/core/src/test/java/org/apache/brooklyn/core/entity/ApplicationLifecycleStateTest.java index 3499f13..70057d4 100644 --- a/core/src/test/java/org/apache/brooklyn/core/entity/ApplicationLifecycleStateTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/entity/ApplicationLifecycleStateTest.java @@ -134,12 +134,6 @@ public class ApplicationLifecycleStateTest extends BrooklynMgmtUnitTestSupport { assertHealthEventually(app, Lifecycle.ON_FIRE, false); } - // TODO Fails in a full `mvn clean install`, but I can't get it to fail in Eclipse running - // lots of times, or with `mvn test -Dtest=ApplicationLifecycleStateTest`. The failure is: - // java.lang.AssertionError: (Dumped entity info - see log); entity=Application[6q37l8cu]; state=on-fire; up=true; notUpIndicators={}; serviceProblems={service-lifecycle-indicators-from-children-and-members=Required entity not healthy: FailingEntityImpl{id=exz9n1pti0}} - // at org.apache.brooklyn.core.entity.ApplicationLifecycleStateTest.assertUpAndRunningEventually(ApplicationLifecycleStateTest.java:204) - // at org.apache.brooklyn.core.entity.ApplicationLifecycleStateTest.testChildFailuresOnStartButWithQuorumCausesAppToSucceed(ApplicationLifecycleStateTest.java:146) - @Test(groups="Broken") public void testChildFailuresOnStartButWithQuorumCausesAppToSucceed() throws Exception { TestApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(TestApplication.class) .configure(StartableApplication.UP_QUORUM_CHECK, QuorumCheck.QuorumChecks.atLeastOne()) http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/33858931/core/src/test/java/org/apache/brooklyn/core/entity/lifecycle/ServiceStateLogicTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/entity/lifecycle/ServiceStateLogicTest.java b/core/src/test/java/org/apache/brooklyn/core/entity/lifecycle/ServiceStateLogicTest.java index 0932fce..61fd147 100644 --- a/core/src/test/java/org/apache/brooklyn/core/entity/lifecycle/ServiceStateLogicTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/entity/lifecycle/ServiceStateLogicTest.java @@ -18,17 +18,26 @@ */ package org.apache.brooklyn.core.entity.lifecycle; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; + +import java.util.concurrent.atomic.AtomicInteger; + import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.entity.EntitySpec; import org.apache.brooklyn.api.location.Location; import org.apache.brooklyn.api.sensor.AttributeSensor; import org.apache.brooklyn.api.sensor.Enricher; +import org.apache.brooklyn.api.sensor.EnricherSpec; +import org.apache.brooklyn.api.sensor.SensorEvent; import org.apache.brooklyn.core.entity.Attributes; import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.entity.EntityAdjuncts; import org.apache.brooklyn.core.entity.EntityAsserts; import org.apache.brooklyn.core.entity.EntityInternal; import org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic.ComputeServiceIndicatorsFromChildrenAndMembers; +import org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic.ComputeServiceIndicatorsFromChildrenAndMembersSpec; +import org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic.ComputeServiceState; import org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic.ServiceNotUpLogic; import org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic.ServiceProblemsLogic; import org.apache.brooklyn.core.sensor.Sensors; @@ -36,6 +45,7 @@ import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport; import org.apache.brooklyn.core.test.entity.TestEntity; import org.apache.brooklyn.core.test.entity.TestEntityImpl.TestEntityWithoutEnrichers; import org.apache.brooklyn.entity.group.DynamicCluster; +import org.apache.brooklyn.util.collections.QuorumCheck; import org.apache.brooklyn.util.collections.QuorumCheck.QuorumChecks; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.time.Duration; @@ -281,6 +291,35 @@ public class ServiceStateLogicTest extends BrooklynAppUnitTestSupport { EntityAsserts.assertAttributeEqualsContinually(cluster, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.RUNNING); } + public static class CountingComputeServiceState extends ComputeServiceState { + AtomicInteger cntCalled = new AtomicInteger(); + AtomicInteger cntCalledWithNull = new AtomicInteger(); + + public CountingComputeServiceState() {} + + @Override + public void onEvent(SensorEvent<Object> event) { + cntCalled.incrementAndGet(); + if (event == null) { + cntCalledWithNull.incrementAndGet(); + } + super.onEvent(event); + } + } + + @Test + public void testServiceStateNotCalledExplicitly() throws Exception { + EnricherSpec<CountingComputeServiceState> enricherSpec = EnricherSpec.create(CountingComputeServiceState.class); + CountingComputeServiceState enricher = mgmt.getEntityManager().createEnricher(enricherSpec); + app.enrichers().add(enricher); + + ServiceStateLogic.setExpectedState(entity, Lifecycle.RUNNING); + assertAttributeEqualsEventually(entity, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.ON_FIRE); + + assertTrue(enricher.cntCalled.get() > 0); + assertEquals(enricher.cntCalledWithNull.get(), 0); + } + private static <T> void assertAttributeEqualsEventually(Entity x, AttributeSensor<T> sensor, T value) { try { EntityAsserts.assertAttributeEqualsEventually(ImmutableMap.of("timeout", Duration.seconds(3)), x, sensor, value);
