back out logic where DynamicCluster tried to be smart about setting transitions starting/stopping when resizing from/to size 0; now it does not. 0 is not a valid size for a running cluster. makes sense as a default. notes for how to do something different here. also minor tidies to ServerPool which was sensitive to this behaviour.
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/ea10fef2 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/ea10fef2 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/ea10fef2 Branch: refs/heads/master Commit: ea10fef26853efcede2376a41e172e32d5e7889f Parents: 527604e Author: Alex Heneveld <[email protected]> Authored: Sat Oct 4 01:42:50 2014 +0100 Committer: Alex Heneveld <[email protected]> Committed: Sat Oct 4 01:42:50 2014 +0100 ---------------------------------------------------------------------- .../entity/group/DynamicClusterImpl.java | 22 -------------------- .../entity/group/DynamicClusterTest.java | 11 +++++----- .../brooklyn/entity/pool/ServerPoolImpl.java | 2 +- .../brooklyn/entity/pool/ServerPoolTest.java | 3 +-- 4 files changed, 8 insertions(+), 30 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/ea10fef2/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java b/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java index c5b4957..75bdc19 100644 --- a/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java +++ b/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java @@ -37,14 +37,12 @@ import org.slf4j.LoggerFactory; import brooklyn.config.render.RendererHints; import brooklyn.entity.Entity; import brooklyn.entity.basic.AbstractGroupImpl; -import brooklyn.entity.basic.Attributes; import brooklyn.entity.basic.DelegateEntity; import brooklyn.entity.basic.Entities; import brooklyn.entity.basic.EntityFactory; import brooklyn.entity.basic.EntityFactoryForLocation; import brooklyn.entity.basic.EntityLocal; import brooklyn.entity.basic.Lifecycle; -import brooklyn.entity.basic.Lifecycle.Transition; import brooklyn.entity.basic.QuorumCheck.QuorumChecks; import brooklyn.entity.basic.ServiceStateLogic; import brooklyn.entity.basic.ServiceStateLogic.ServiceProblemsLogic; @@ -416,38 +414,18 @@ public class DynamicClusterImpl extends AbstractGroupImpl implements DynamicClus @Override public Integer resize(Integer desiredSize) { synchronized (mutex) { - Transition originalExpected = getAttribute(Attributes.SERVICE_STATE_EXPECTED); - int originalSize = getCurrentSize(); - if (originalSize==0) { - ServiceStateLogic.setExpectedState(this, Lifecycle.STARTING); - } int delta = desiredSize - originalSize; - if (desiredSize==0) { - ServiceStateLogic.setExpectedState(this, Lifecycle.STOPPING); - } if (delta != 0) { LOG.info("Resize {} from {} to {}", new Object[] {this, originalSize, desiredSize}); } else { if (LOG.isDebugEnabled()) LOG.debug("Resize no-op {} from {} to {}", new Object[] {this, originalSize, desiredSize}); } resizeByDelta(delta); - - restoreStateAfterResize(originalExpected, originalSize, desiredSize); } return getCurrentSize(); } - // TODO this logic maybe belongs in abstract group? fixes test in - // see test in DynamicClusterTest.testResizeFromZeroToOneDoesNotGoThroughFailing - protected void restoreStateAfterResize(Transition originalExpected, int originalSize, Integer desiredSize) { - if (originalExpected!=null) { - ServiceStateLogic.setExpectedState(this, originalExpected.getState()); - } else { - ServiceStateLogic.setExpectedState(this, desiredSize==0 ? Lifecycle.STOPPED : Lifecycle.RUNNING); - } - } - /** * {@inheritDoc} * http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/ea10fef2/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java b/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java index a7166ab..ad5394a 100644 --- a/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java +++ b/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java @@ -51,6 +51,7 @@ import brooklyn.entity.basic.Entities; import brooklyn.entity.basic.EntityFactory; import brooklyn.entity.basic.EntitySubscriptionTest.RecordingSensorEventListener; import brooklyn.entity.basic.Lifecycle; +import brooklyn.entity.basic.ServiceStateLogic; import brooklyn.entity.proxying.EntitySpec; import brooklyn.entity.trait.Changeable; import brooklyn.entity.trait.FailingEntity; @@ -182,11 +183,11 @@ public class DynamicClusterTest extends BrooklynAppUnitTestSupport { assertEquals(entity.getApplication(), app); } - /** two "errors" which could cause this: - * <li> TestEntity set expected RUNNING before setting SERVICE_UP, means it goes on fire briefly, so parent might also; - * <li> DynamicCluster could see service up false when expected running because it was resizing to/from 0 - * (so there may be a moment when UP_QUORUM_CHECK fails as >=1 child, but 0 up, because all children are transitioning; - * this is fixed with logic in {@link DynamicClusterImpl#restoreStateAfterResize(brooklyn.entity.basic.Lifecycle.Transition, int, Integer)}. + /** This can be sensitive to order, e.g. if TestEntity set expected RUNNING before setting SERVICE_UP, + * there would be a point when TestEntity is ON_FIRE. + * <p> + * There can also be issues if a cluster is resizing from/to 0 while in a RUNNING state. + * To correct that, use {@link ServiceStateLogic#newEnricherFromChildrenUp()}. */ @Test public void testResizeFromZeroToOneDoesNotGoThroughFailing() throws Exception { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/ea10fef2/software/base/src/main/java/brooklyn/entity/pool/ServerPoolImpl.java ---------------------------------------------------------------------- diff --git a/software/base/src/main/java/brooklyn/entity/pool/ServerPoolImpl.java b/software/base/src/main/java/brooklyn/entity/pool/ServerPoolImpl.java index ddc9add..7e13591 100644 --- a/software/base/src/main/java/brooklyn/entity/pool/ServerPoolImpl.java +++ b/software/base/src/main/java/brooklyn/entity/pool/ServerPoolImpl.java @@ -297,7 +297,7 @@ public class ServerPoolImpl extends DynamicClusterImpl implements ServerPool { } if (delta < removable) { - LOG.info("Too few removable machines in {} to shrink by delta {}. Altered delta to {}", + LOG.warn("Too few removable machines in {} to shrink by delta {}. Altered delta to {}", new Object[]{this, delta, removable}); delta = removable; } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/ea10fef2/software/base/src/test/java/brooklyn/entity/pool/ServerPoolTest.java ---------------------------------------------------------------------- diff --git a/software/base/src/test/java/brooklyn/entity/pool/ServerPoolTest.java b/software/base/src/test/java/brooklyn/entity/pool/ServerPoolTest.java index 584fbfe..1eb222b 100644 --- a/software/base/src/test/java/brooklyn/entity/pool/ServerPoolTest.java +++ b/software/base/src/test/java/brooklyn/entity/pool/ServerPoolTest.java @@ -119,11 +119,10 @@ public class ServerPoolTest extends AbstractServerPoolTest { assertAvailableCountEquals(1); assertClaimedCountEquals(getInitialPoolSize() - 1); - LOG.info("Test attempting to resize to 0 members. Should only drop one machine."); + LOG.info("Test attempting to resize to 0 members. Should only drop the one available machine."); pool.resize(0); assertAvailableCountEventuallyEquals(0); - assertEquals(Iterables.size(pool.getMembers()), getInitialPoolSize() - 1); assertAvailableCountEquals(0); assertClaimedCountEquals(getInitialPoolSize() - 1);
