BROOKLYN-212: more improvements

- Resizable.resize: throw InsufficientCapacityException if didn’t 
  manage to reach target size, rather than only if didn’t manage to
  increase in size at all.
- DynamicCluster: delete nodes that throw NoMachinesAvailableException,
  rather than putting them in quarantine.
- Fix AutoScalerPolicy’s max-capacity high-water mark, when 
  resizeUpStabilizationDelay is used.

Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: 
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/4245bd62
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/4245bd62
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/4245bd62

Branch: refs/heads/master
Commit: 4245bd628172ad25bfbf745a9ad259f11050b3ea
Parents: ffbad25
Author: Aled Sage <aled.s...@gmail.com>
Authored: Thu Jan 14 10:43:50 2016 +0000
Committer: Aled Sage <aled.s...@gmail.com>
Committed: Thu Jan 14 10:46:09 2016 +0000

----------------------------------------------------------------------
 .../brooklyn/core/entity/trait/Resizable.java   |  6 +-
 .../brooklyn/entity/group/DynamicCluster.java   | 10 +++
 .../entity/group/DynamicClusterImpl.java        | 45 ++++++----
 .../core/test/entity/TestClusterImpl.java       | 17 ++--
 .../entity/group/DynamicClusterTest.java        | 88 +++++++++++++++++---
 .../policy/autoscaling/AutoScalerPolicy.java    | 31 ++++---
 .../autoscaling/AutoScalerPolicyMetricTest.java | 34 +++++++-
 .../AutoScalerPolicyNoMoreMachinesTest.java     | 56 +++++++++----
 8 files changed, 223 insertions(+), 64 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/4245bd62/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/entity/trait/Resizable.java
----------------------------------------------------------------------
diff --git 
a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/entity/trait/Resizable.java
 
b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/entity/trait/Resizable.java
index 1fca9f3..36e6ba8 100644
--- 
a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/entity/trait/Resizable.java
+++ 
b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/entity/trait/Resizable.java
@@ -32,7 +32,8 @@ import org.apache.brooklyn.core.effector.MethodEffector;
 public interface Resizable {
 
     /**
-     * Indicates that resizing up (at all) is not possible, because there is 
insufficient capacity.
+     * Indicates that resizing up to the desired size is not possible - only 
resized to the 
+     * {@link Resizable#getCurrentSize()}, because there is insufficient 
capacity.
      */
     public static class InsufficientCapacityException extends RuntimeException 
{
         private static final long serialVersionUID = 953230498564942446L;
@@ -53,7 +54,8 @@ public interface Resizable {
      * @param desiredSize the new size of the entity group.
      * @return the new size of the group.
      * 
-     * @throws InsufficientCapacityException If the request was to grow, but 
there is no capacity to grow at all
+     * @throws InsufficientCapacityException If the request was to grow, but 
there is no capacity to grow to
+     *         the desired size.
      */
     @Effector(description="Changes the size of the entity (e.g. the number of 
nodes in a cluster)")
     Integer resize(@EffectorParam(name="desiredSize", description="The new 
size of the cluster") Integer desiredSize);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/4245bd62/brooklyn-server/core/src/main/java/org/apache/brooklyn/entity/group/DynamicCluster.java
----------------------------------------------------------------------
diff --git 
a/brooklyn-server/core/src/main/java/org/apache/brooklyn/entity/group/DynamicCluster.java
 
b/brooklyn-server/core/src/main/java/org/apache/brooklyn/entity/group/DynamicCluster.java
index f528db7..781cb0c 100644
--- 
a/brooklyn-server/core/src/main/java/org/apache/brooklyn/entity/group/DynamicCluster.java
+++ 
b/brooklyn-server/core/src/main/java/org/apache/brooklyn/entity/group/DynamicCluster.java
@@ -48,6 +48,7 @@ import org.apache.brooklyn.util.time.Duration;
 
 import com.google.common.annotations.Beta;
 import com.google.common.base.Function;
+import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Multimap;
 import com.google.common.reflect.TypeToken;
@@ -101,6 +102,15 @@ public interface DynamicCluster extends AbstractGroup, 
Cluster, MemberReplaceabl
     ConfigKey<Boolean> QUARANTINE_FAILED_ENTITIES = 
ConfigKeys.newBooleanConfigKey(
             "dynamiccluster.quarantineFailedEntities", "If true, will 
quarantine entities that fail to start; if false, will get rid of them (i.e. 
delete them)", true);
 
+    @SetFromFlag("quarantineFilter")
+    ConfigKey<Predicate<? super Throwable>> QUARANTINE_FILTER = 
ConfigKeys.newConfigKey(
+            new TypeToken<Predicate<? super Throwable>>() {},
+            "dynamiccluster.quarantineFilter", 
+            "Quarantine the failed nodes that pass this filter (given the 
exception thrown by the node). "
+                    + "Default is those that did not fail with 
NoMachinesAvailableException "
+                    + "(Config ignored if quarantineFailedEntities is false)", 
+            null);
+
     AttributeSensor<Lifecycle> SERVICE_STATE_ACTUAL = 
Attributes.SERVICE_STATE_ACTUAL;
 
     BasicNotificationSensor<Entity> ENTITY_QUARANTINED = new 
BasicNotificationSensor<Entity>(Entity.class, 
"dynamiccluster.entityQuarantined", "Entity failed to start, and has been 
quarantined");

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/4245bd62/brooklyn-server/core/src/main/java/org/apache/brooklyn/entity/group/DynamicClusterImpl.java
----------------------------------------------------------------------
diff --git 
a/brooklyn-server/core/src/main/java/org/apache/brooklyn/entity/group/DynamicClusterImpl.java
 
b/brooklyn-server/core/src/main/java/org/apache/brooklyn/entity/group/DynamicClusterImpl.java
index bfe0b90..16a82d4 100644
--- 
a/brooklyn-server/core/src/main/java/org/apache/brooklyn/entity/group/DynamicClusterImpl.java
+++ 
b/brooklyn-server/core/src/main/java/org/apache/brooklyn/entity/group/DynamicClusterImpl.java
@@ -82,6 +82,7 @@ import com.google.common.base.Function;
 import com.google.common.base.Functions;
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
 import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
@@ -333,6 +334,19 @@ public class DynamicClusterImpl extends AbstractGroupImpl 
implements DynamicClus
         return getAttribute(QUARANTINE_GROUP);
     }
 
+    protected Predicate<? super Throwable> getQuarantineFilter() {
+        Predicate<? super Throwable> result = getConfig(QUARANTINE_FILTER);
+        if (result != null) {
+            return result;
+        } else {
+            return new Predicate<Throwable>() {
+                @Override public boolean apply(Throwable input) {
+                    return Exceptions.getFirstThrowableOfType(input, 
NoMachinesAvailableException.class) == null;
+                }
+            };
+        }
+    }
+
     protected int getInitialQuorumSize() {
         int initialSize = getConfig(INITIAL_SIZE).intValue();
         int initialQuorumSize = getConfig(INITIAL_QUORUM_SIZE).intValue();
@@ -711,16 +725,10 @@ public class DynamicClusterImpl extends AbstractGroupImpl 
implements DynamicClus
             chosenLocations = Collections.nCopies(delta, getLocation());
         }
 
-        // create and start the entities
+        // create and start the entities.
+        // if any fail, then propagate the error.
         ReferenceWithError<Collection<Entity>> result = 
addInEachLocation(chosenLocations, ImmutableMap.of());
-        
-        // If any entities were created, return them (even if we didn't manage 
to create them all).
-        // Otherwise, propagate any error that happened.
-        if (result.get().size() > 0) {
-            return result.get();
-        } else {
-            return result.getWithError();
-        }
+        return result.getWithError();
     }
 
     /** <strong>Note</strong> for sub-clases; this method can be called while 
synchronized on {@link #mutex}. */
@@ -809,7 +817,7 @@ public class DynamicClusterImpl extends AbstractGroupImpl 
implements DynamicClus
         // quarantine/cleanup as necessary
         if (!errors.isEmpty()) {
             if (isQuarantineEnabled()) {
-                quarantineFailedNodes(errors.keySet());
+                quarantineFailedNodes(errors);
             } else {
                 cleanupFailedNodes(errors.keySet());
             }
@@ -819,11 +827,18 @@ public class DynamicClusterImpl extends AbstractGroupImpl 
implements DynamicClus
         return ReferenceWithError.newInstanceWithoutError(result);
     }
 
-    protected void quarantineFailedNodes(Collection<Entity> failedEntities) {
-        for (Entity entity : failedEntities) {
-            sensors().emit(ENTITY_QUARANTINED, entity);
-            getQuarantineGroup().addMember(entity);
-            removeMember(entity);
+    protected void quarantineFailedNodes(Map<Entity, Throwable> 
failedEntities) {
+        for (Map.Entry<Entity, Throwable> entry : failedEntities.entrySet()) {
+            Entity entity = entry.getKey();
+            Throwable cause = entry.getValue();
+            if (cause == null || getQuarantineFilter().apply(cause)) {
+                sensors().emit(ENTITY_QUARANTINED, entity);
+                getQuarantineGroup().addMember(entity);
+                removeMember(entity);
+            } else {
+                LOG.info("Cluster {} discarding failed node {}, rather than 
quarantining", this, entity);
+                discardNode(entity);
+            }
         }
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/4245bd62/brooklyn-server/core/src/test/java/org/apache/brooklyn/core/test/entity/TestClusterImpl.java
----------------------------------------------------------------------
diff --git 
a/brooklyn-server/core/src/test/java/org/apache/brooklyn/core/test/entity/TestClusterImpl.java
 
b/brooklyn-server/core/src/test/java/org/apache/brooklyn/core/test/entity/TestClusterImpl.java
index d318e5e..0edea8f 100644
--- 
a/brooklyn-server/core/src/test/java/org/apache/brooklyn/core/test/entity/TestClusterImpl.java
+++ 
b/brooklyn-server/core/src/test/java/org/apache/brooklyn/core/test/entity/TestClusterImpl.java
@@ -59,16 +59,17 @@ public class TestClusterImpl extends DynamicClusterImpl 
implements TestCluster {
     @Override
     public Integer resize(Integer desiredSize) {
         desiredSizeHistory.add(desiredSize);
+        int achievableSize = Math.min(desiredSize, getConfig(MAX_SIZE));
         
-        if (desiredSize > size) {
-            if (size < getConfig(MAX_SIZE)) {
-                desiredSize = Math.min(desiredSize, getConfig(MAX_SIZE));
-            } else {
-                throw new InsufficientCapacityException("Simulating 
insufficient capacity (desiredSize="+desiredSize+"; 
maxSize="+getConfig(MAX_SIZE)+"; currentSize="+size+")");
-            }
+        if (achievableSize != size) {
+            this.sizeHistory.add(achievableSize);
+            this.size = achievableSize;
         }
-        this.sizeHistory.add(desiredSize);
-        this.size = desiredSize;
+        
+        if (desiredSize > achievableSize) {
+            throw new InsufficientCapacityException("Simulating insufficient 
capacity (desiredSize="+desiredSize+"; maxSize="+getConfig(MAX_SIZE)+"; 
newSize="+size+")");
+        }
+
         return size;
     }
     

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/4245bd62/brooklyn-server/core/src/test/java/org/apache/brooklyn/entity/group/DynamicClusterTest.java
----------------------------------------------------------------------
diff --git 
a/brooklyn-server/core/src/test/java/org/apache/brooklyn/entity/group/DynamicClusterTest.java
 
b/brooklyn-server/core/src/test/java/org/apache/brooklyn/entity/group/DynamicClusterTest.java
index 33194f0..f58ac90 100644
--- 
a/brooklyn-server/core/src/test/java/org/apache/brooklyn/entity/group/DynamicClusterTest.java
+++ 
b/brooklyn-server/core/src/test/java/org/apache/brooklyn/entity/group/DynamicClusterTest.java
@@ -222,7 +222,7 @@ public class DynamicClusterTest extends 
BrooklynAppUnitTestSupport {
     }
 
     @Test
-    public void 
testResizeWhereSubsetOfChildrenThrowsNoMachineAvailableExceptionReturnsNormally()
 throws Exception {
+    public void 
testResizeWhereSubsetOfChildrenThrowsNoMachineAvailableExceptionIsPropagatedAsInsuffientCapacityException()
 throws Exception {
         final DynamicCluster cluster = 
app.createAndManageChild(EntitySpec.create(DynamicCluster.class)
             .configure(DynamicCluster.MEMBER_SPEC, 
EntitySpec.create(FailingEntity.class)
                     .configure(FailingEntity.FAIL_ON_START_CONDITION, new 
Predicate<FailingEntity>() {
@@ -236,19 +236,31 @@ public class DynamicClusterTest extends 
BrooklynAppUnitTestSupport {
             .configure(DynamicCluster.INITIAL_SIZE, 0));
         cluster.start(ImmutableList.of(loc));
 
-        // Managed to partially resize, so should not fail entirely.
-        // Instead just say how big we managed to get.
-        Integer newSize = cluster.resize(2);
-        assertEquals(newSize, (Integer)1);
+        // Managed to partially resize, but will still throw exception.
+        // The getCurrentSize will report how big we managed to get.
+        // The children that failed due to NoMachinesAvailableException will 
have been unmanaged automatically.
+        try {
+            cluster.resize(2);
+            Asserts.shouldHaveFailedPreviously();
+        } catch (Exception e) {
+            Asserts.expectedFailureOfType(e, 
Resizable.InsufficientCapacityException.class);
+        }
         assertEquals(cluster.getCurrentSize(), (Integer)1);
-
-        // This attempt will fail, because all new children will fail
+        Iterable<FailingEntity> children1 = 
Iterables.filter(cluster.getChildren(), FailingEntity.class);
+        assertEquals(Iterables.size(children1), 1);
+        
assertEquals(Iterables.getOnlyElement(children1).sensors().get(TestEntity.SERVICE_UP),
 Boolean.TRUE);
+        
+        // This attempt will also fail, because all new children will fail
         try {
             cluster.resize(2);
             Asserts.shouldHaveFailedPreviously();
         } catch (Exception e) {
             Asserts.expectedFailureOfType(e, 
Resizable.InsufficientCapacityException.class);
         }
+        assertEquals(cluster.getCurrentSize(), (Integer)1);
+        Iterable<FailingEntity> children2 = 
Iterables.filter(cluster.getChildren(), FailingEntity.class);
+        assertEquals(Iterables.size(children2), 1);
+        assertEquals(Iterables.getOnlyElement(children2), 
Iterables.getOnlyElement(children1));
     }
 
     /** This can be sensitive to order, e.g. if TestEntity set expected 
RUNNING before setting SERVICE_UP, 
@@ -471,7 +483,7 @@ public class DynamicClusterTest extends 
BrooklynAppUnitTestSupport {
                     }}));
 
         cluster.start(ImmutableList.of(loc));
-        cluster.resize(3);
+        resizeExpectingError(cluster, 3);
         assertEquals(cluster.getCurrentSize(), (Integer)2);
         assertEquals(cluster.getMembers().size(), 2);
         for (Entity member : cluster.getMembers()) {
@@ -583,7 +595,7 @@ public class DynamicClusterTest extends 
BrooklynAppUnitTestSupport {
                     }}));
 
         cluster.start(ImmutableList.of(loc));
-        cluster.resize(3);
+        resizeExpectingError(cluster, 3);
         assertEquals(cluster.getCurrentSize(), (Integer)2);
         assertEquals(cluster.getMembers().size(), 2);
         assertEquals(Iterables.size(Iterables.filter(cluster.getChildren(), 
Predicates.instanceOf(FailingEntity.class))), 3);
@@ -620,7 +632,7 @@ public class DynamicClusterTest extends 
BrooklynAppUnitTestSupport {
         assertEquals(cluster.getChildren().size(), 0, 
"children="+cluster.getChildren());
         
         // Failed node will not be a member or child
-        cluster.resize(3);
+        resizeExpectingError(cluster, 3);
         assertEquals(cluster.getCurrentSize(), (Integer)2);
         assertEquals(cluster.getMembers().size(), 2);
         assertEquals(cluster.getChildren().size(), 2, 
"children="+cluster.getChildren());
@@ -633,6 +645,62 @@ public class DynamicClusterTest extends 
BrooklynAppUnitTestSupport {
     }
 
     @Test
+    public void testQuarantineFailedEntitiesRespectsCustomFilter() throws 
Exception {
+        Predicate<Throwable> filter = new Predicate<Throwable>() {
+            @Override public boolean apply(Throwable input) {
+                return Exceptions.getFirstThrowableOfType(input, 
AllowedException.class) != null;
+            }
+        };
+        runQuarantineFailedEntitiesRespectsFilter(AllowedException.class, 
DisallowedException.class, filter);
+    }
+    @SuppressWarnings("serial")
+    public static class AllowedException extends RuntimeException {
+        public AllowedException(String message) {
+            super(message);
+        }
+    }
+    @SuppressWarnings("serial")
+    public static class DisallowedException extends RuntimeException {
+        public DisallowedException(String message) {
+            super(message);
+        }
+    }
+
+    @Test
+    public void testQuarantineFailedEntitiesRespectsDefaultFilter() throws 
Exception {
+        Predicate<Throwable> filter = null;
+        runQuarantineFailedEntitiesRespectsFilter(AllowedException.class, 
NoMachinesAvailableException.class, filter);
+    }
+    
+    protected void runQuarantineFailedEntitiesRespectsFilter(Class<? extends 
Exception> allowedException, 
+            Class<? extends Exception> disallowedException, 
Predicate<Throwable> quarantineFilter) throws Exception {
+        final List<Class<? extends Exception>> failureCauses = 
ImmutableList.<Class<? extends Exception>>of(allowedException, 
disallowedException);
+        final AtomicInteger counter = new AtomicInteger(0);
+        DynamicCluster cluster = 
app.createAndManageChild(EntitySpec.create(DynamicCluster.class)
+                .configure("quarantineFailedEntities", true)
+                .configure("initialSize", 0)
+                .configure("quarantineFilter", quarantineFilter)
+                .configure("factory", new EntityFactory() {
+                    @Override public Entity newEntity(Map flags, Entity 
parent) {
+                        int num = counter.getAndIncrement();
+                        return 
app.getManagementContext().getEntityManager().createEntity(EntitySpec.create(FailingEntity.class)
+                                .configure(flags)
+                                .configure(FailingEntity.FAIL_ON_START, true)
+                                .configure(FailingEntity.EXCEPTION_CLAZZ, 
failureCauses.get(num))
+                                .parent(parent));
+                    }}));
+
+        cluster.start(ImmutableList.of(loc));
+        resizeExpectingError(cluster, 2);
+        Iterable<FailingEntity> children = 
Iterables.filter(cluster.getChildren(), FailingEntity.class);
+        Collection<Entity> quarantineMembers = 
cluster.sensors().get(DynamicCluster.QUARANTINE_GROUP).getMembers();
+        
+        assertEquals(cluster.getCurrentSize(), (Integer)0);
+        
assertEquals(Iterables.getOnlyElement(children).config().get(FailingEntity.EXCEPTION_CLAZZ),
 allowedException);
+        assertEquals(Iterables.getOnlyElement(quarantineMembers), 
Iterables.getOnlyElement(children));
+    }
+
+    @Test
     public void defaultRemovalStrategyShutsDownNewestFirstWhenResizing() 
throws Exception {
         final List<Entity> creationOrder = Lists.newArrayList();
         DynamicCluster cluster = 
app.createAndManageChild(EntitySpec.create(DynamicCluster.class)

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/4245bd62/brooklyn-server/policy/src/main/java/org/apache/brooklyn/policy/autoscaling/AutoScalerPolicy.java
----------------------------------------------------------------------
diff --git 
a/brooklyn-server/policy/src/main/java/org/apache/brooklyn/policy/autoscaling/AutoScalerPolicy.java
 
b/brooklyn-server/policy/src/main/java/org/apache/brooklyn/policy/autoscaling/AutoScalerPolicy.java
index a1406ca..b484359 100644
--- 
a/brooklyn-server/policy/src/main/java/org/apache/brooklyn/policy/autoscaling/AutoScalerPolicy.java
+++ 
b/brooklyn-server/policy/src/main/java/org/apache/brooklyn/policy/autoscaling/AutoScalerPolicy.java
@@ -20,7 +20,6 @@ package org.apache.brooklyn.policy.autoscaling;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.apache.brooklyn.util.JavaGroovyEquivalents.groovyTruth;
-import groovy.lang.Closure;
 
 import java.util.Map;
 import java.util.concurrent.Callable;
@@ -30,8 +29,6 @@ import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.apache.brooklyn.api.catalog.Catalog;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.entity.EntityLocal;
@@ -55,6 +52,8 @@ import org.apache.brooklyn.util.core.flags.SetFromFlag;
 import org.apache.brooklyn.util.core.flags.TypeCoercions;
 import org.apache.brooklyn.util.core.task.Tasks;
 import org.apache.brooklyn.util.time.Duration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
@@ -62,6 +61,8 @@ import com.google.common.base.Throwables;
 import com.google.common.reflect.TypeToken;
 import com.google.common.util.concurrent.ThreadFactoryBuilder;
 
+import groovy.lang.Closure;
+
 
 /**
  * Policy that is attached to a {@link Resizable} entity and dynamically 
adjusts its size in response to
@@ -869,6 +870,10 @@ public class AutoScalerPolicy extends AbstractPolicy {
         onNewUnboundedPoolSize(desiredSizeUnconstrained);
     }
 
+    private int applyMinMaxConstraints(long desiredSize) {
+        return applyMinMaxConstraints(desiredSize > Integer.MAX_VALUE ? 
Integer.MAX_VALUE : (int)desiredSize);
+    }
+    
     private int applyMinMaxConstraints(int desiredSize) {
         int minSize = getMinPoolSize();
         int maxSize = getMaxPoolSize();
@@ -1014,40 +1019,42 @@ public class AutoScalerPolicy extends AbstractPolicy {
     }
 
     private void resizeNow() {
-        final long currentPoolSize = 
getCurrentSizeOperator().apply(poolEntity);
+        final int currentPoolSize = getCurrentSizeOperator().apply(poolEntity);
         CalculatedDesiredPoolSize calculatedDesiredPoolSize = 
calculateDesiredPoolSize(currentPoolSize);
-        final long desiredPoolSize = calculatedDesiredPoolSize.size;
+        long desiredPoolSize = calculatedDesiredPoolSize.size;
         boolean stable = calculatedDesiredPoolSize.stable;
         
+        final int targetPoolSize = applyMinMaxConstraints(desiredPoolSize);
+        
         if (!stable) {
             // the desired size fluctuations are not stable; ensure we check 
again later (due to time-window)
             // even if no additional events have been received
             // (note we continue now with as "good" a resize as we can given 
the instability)
             if (LOG.isTraceEnabled()) LOG.trace("{} re-scheduling resize check 
for {}, as desired size not stable (current {}, desired {}); continuing with 
resize...", 
-                    new Object[] {this, poolEntity, currentPoolSize, 
desiredPoolSize});
+                    new Object[] {this, poolEntity, currentPoolSize, 
targetPoolSize});
             scheduleResize();
         }
-        if (currentPoolSize == desiredPoolSize) {
+        if (currentPoolSize == targetPoolSize) {
             if (LOG.isTraceEnabled()) LOG.trace("{} not resizing pool {} from 
{} to {}", 
-                    new Object[] {this, poolEntity, currentPoolSize, 
desiredPoolSize});
+                    new Object[] {this, poolEntity, currentPoolSize, 
targetPoolSize});
             return;
         }
         
         if (LOG.isDebugEnabled()) LOG.debug("{} requesting resize to {}; 
current {}, min {}, max {}", 
-                new Object[] {this, desiredPoolSize, currentPoolSize, 
getMinPoolSize(), getMaxPoolSize()});
+                new Object[] {this, targetPoolSize, currentPoolSize, 
getMinPoolSize(), getMaxPoolSize()});
         
         Entities.submit(entity, 
Tasks.<Void>builder().displayName("Auto-scaler")
-            .description("Auto-scaler recommending resize from 
"+currentPoolSize+" to "+desiredPoolSize)
+            .description("Auto-scaler recommending resize from 
"+currentPoolSize+" to "+targetPoolSize)
             .tag(BrooklynTaskTags.NON_TRANSIENT_TASK_TAG)
             .body(new Callable<Void>() {
                 @Override
                 public Void call() throws Exception {
                     // TODO Should we use int throughout, rather than casting 
here?
                     try {
-                        getResizeOperator().resize(poolEntity, (int) 
desiredPoolSize);
+                        getResizeOperator().resize(poolEntity, (int) 
targetPoolSize);
                     } catch (Resizable.InsufficientCapacityException e) {
                         // cannot resize beyond this; set the high-water mark
-                        int insufficientCapacityHighWaterMark = 
(currentPoolSize > Integer.MAX_VALUE ? Integer.MAX_VALUE : 
(int)currentPoolSize);
+                        int insufficientCapacityHighWaterMark = 
getCurrentSizeOperator().apply(poolEntity);
                         LOG.warn("{} failed to resize {} due to insufficient 
capacity; setting high-water mark to {}, "
                                 + "and will not attempt to resize above that 
level again", 
                                 new Object[] {AutoScalerPolicy.this, 
poolEntity, insufficientCapacityHighWaterMark});

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/4245bd62/brooklyn-server/policy/src/test/java/org/apache/brooklyn/policy/autoscaling/AutoScalerPolicyMetricTest.java
----------------------------------------------------------------------
diff --git 
a/brooklyn-server/policy/src/test/java/org/apache/brooklyn/policy/autoscaling/AutoScalerPolicyMetricTest.java
 
b/brooklyn-server/policy/src/test/java/org/apache/brooklyn/policy/autoscaling/AutoScalerPolicyMetricTest.java
index 2794d3a..ad67b75 100644
--- 
a/brooklyn-server/policy/src/test/java/org/apache/brooklyn/policy/autoscaling/AutoScalerPolicyMetricTest.java
+++ 
b/brooklyn-server/policy/src/test/java/org/apache/brooklyn/policy/autoscaling/AutoScalerPolicyMetricTest.java
@@ -36,6 +36,7 @@ import org.apache.brooklyn.core.test.entity.TestApplication;
 import org.apache.brooklyn.core.test.entity.TestCluster;
 import org.apache.brooklyn.core.test.entity.TestEntity;
 import org.apache.brooklyn.test.Asserts;
+import org.apache.brooklyn.util.time.Duration;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
@@ -274,7 +275,7 @@ public class AutoScalerPolicyMetricTest {
     @Test(groups="Integration")
     public void 
testOnFailedGrowWillSetHighwaterMarkAndNotResizeAboveThatAgain() {
         tc = app.createAndManageChild(EntitySpec.create(TestCluster.class)
-                .configure("initialSize", 1)
+                .configure("initialSize", 0)
                 .configure(TestCluster.MAX_SIZE, 2));
 
         tc.resize(1);
@@ -317,4 +318,35 @@ public class AutoScalerPolicyMetricTest {
         assertEquals(tc.getDesiredSizeHistory(), ImmutableList.of(1, 2, 3, 1, 
2), "desired="+tc.getDesiredSizeHistory());
         assertEquals(tc.getSizeHistory(), ImmutableList.of(0, 1, 2, 1, 2), 
"sizes="+tc.getSizeHistory());
     }
+    
+    // When there is a resizeUpStabilizationDelay, it remembers all the 
previously requested sizes (in the recent history)
+    // and then looks at those in the stabilization-delay to determine the 
sustained desired. This test checks that
+    // we apply the highwater-mark even when the desired size had been 
recorded prior to the highwater mark being
+    // discovered.
+    @Test(groups="Integration")
+    public void 
testOnFailedGrowWithStabilizationDelayWillSetHighwaterMarkAndNotResizeAboveThatAgain()
 throws Exception {
+        tc = app.createAndManageChild(EntitySpec.create(TestCluster.class)
+                .configure("initialSize", 0)
+                .configure(TestCluster.MAX_SIZE, 2));
+
+        tc.resize(1);
+        
+        tc.policies().add(AutoScalerPolicy.builder()
+                .metric(MY_ATTRIBUTE)
+                .metricLowerBound(50)
+                .metricUpperBound(100)
+                .resizeUpStabilizationDelay(Duration.ONE_SECOND)
+                .buildSpec());
+        
+        // workload 200 so requires doubling size to 2 to handle: (200*1)/100 
= 2
+        for (int i = 0; i < 10; i++) {
+            tc.sensors().set(MY_ATTRIBUTE, 200 + (i*100));
+            Thread.sleep(100);
+        }
+        
+        Asserts.succeedsEventually(currentSizeAsserter(tc, 2));
+        Asserts.succeedsContinually(currentSizeAsserter(tc, 2));
+        assertEquals(tc.getDesiredSizeHistory(), ImmutableList.of(1, 2, 3), 
"desired="+tc.getDesiredSizeHistory());
+        assertEquals(tc.getSizeHistory(), ImmutableList.of(0, 1, 2), 
"sizes="+tc.getSizeHistory());
+    }
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/4245bd62/brooklyn-server/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/autoscaling/AutoScalerPolicyNoMoreMachinesTest.java
----------------------------------------------------------------------
diff --git 
a/brooklyn-server/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/autoscaling/AutoScalerPolicyNoMoreMachinesTest.java
 
b/brooklyn-server/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/autoscaling/AutoScalerPolicyNoMoreMachinesTest.java
index 2d36cd3..77175d2 100644
--- 
a/brooklyn-server/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/autoscaling/AutoScalerPolicyNoMoreMachinesTest.java
+++ 
b/brooklyn-server/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/autoscaling/AutoScalerPolicyNoMoreMachinesTest.java
@@ -21,13 +21,16 @@ package 
org.apache.brooklyn.entity.software.base.test.autoscaling;
 import static org.testng.Assert.assertEquals;
 
 import java.util.Map;
+import java.util.Set;
 
+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.policy.PolicySpec;
 import org.apache.brooklyn.api.sensor.AttributeSensor;
 import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
 import org.apache.brooklyn.core.entity.trait.Resizable;
+import org.apache.brooklyn.core.mgmt.internal.CollectionChangeListener;
 import org.apache.brooklyn.core.sensor.Sensors;
 import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.core.test.entity.TestCluster;
@@ -44,6 +47,7 @@ import org.testng.annotations.Test;
 import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Sets;
 
 public class AutoScalerPolicyNoMoreMachinesTest extends 
BrooklynAppUnitTestSupport {
 
@@ -55,6 +59,8 @@ public class AutoScalerPolicyNoMoreMachinesTest extends 
BrooklynAppUnitTestSuppo
     DynamicCluster cluster;
     Location loc;
     AutoScalerPolicy policy;
+    Set<Entity> entitiesAdded;
+    Set<Entity> entitiesRemoved;
     
     @BeforeMethod(alwaysRun=true)
     public void setUp() throws Exception {
@@ -65,6 +71,16 @@ public class AutoScalerPolicyNoMoreMachinesTest extends 
BrooklynAppUnitTestSuppo
                         
.configure(BrooklynConfigKeys.SKIP_ON_BOX_BASE_DIR_RESOLUTION, true)));
         loc = 
mgmt.getLocationRegistry().resolve("byon(hosts='1.1.1.1,1.1.1.2')");
         app.start(ImmutableList.of(loc));
+        
+        entitiesAdded = Sets.newLinkedHashSet();
+        entitiesRemoved = Sets.newLinkedHashSet();
+        mgmt.addEntitySetListener(new CollectionChangeListener<Entity>() {
+            @Override public void onItemAdded(Entity item) {
+                entitiesAdded.add(item);
+            }
+            @Override public void onItemRemoved(Entity item) {
+                entitiesRemoved.add(item);
+            }});
     }
 
     @Test
@@ -75,22 +91,22 @@ public class AutoScalerPolicyNoMoreMachinesTest extends 
BrooklynAppUnitTestSuppo
         assertSize(2);
         
         // Won't get a location to successfully resize (byon location only has 
2 machines); 
-        // so still left with 2 members + 1 in quarantine
+        // so still left with 2 members (failed node not quarantined, because 
exception well understood)
         try {
             cluster.resize(3);
             Asserts.shouldHaveFailedPreviously();
         } catch (Exception e) {
             Asserts.expectedFailureOfType(e, 
Resizable.InsufficientCapacityException.class);
         }
-        assertSize(2, 1);
+        assertSize(2, 0, 1);
 
-        // Resize down; still have 1 in quarantine
+        // Resize down; will delete one of our nodes
         cluster.resize(1);
-        assertSize(1, 1);
+        assertSize(1, 0, 2);
         
         // Resize back up to 2 should be allowed
         cluster.resize(2);
-        assertSize(2, 1);
+        assertSize(2, 0, 2);
     }
     
     @Test
@@ -106,14 +122,14 @@ public class AutoScalerPolicyNoMoreMachinesTest extends 
BrooklynAppUnitTestSuppo
         // Two nodes handing an aggregated load of 41; too high for 2 nodes so 
tries to scale to 3.
         // But byon location only has 2 nodes so will fail.
         cluster.sensors().emit(AutoScalerPolicy.DEFAULT_POOL_HOT_SENSOR, 
message(21L, 10L, 20L));
-        assertSizeEventually(2, 1);
-        
+        assertSizeEventually(2, 0, 1);
+
         // Should not repeatedly retry
-        assertSizeContinually(2, 1);
+        assertSizeContinually(2, 0, 1);
         
         // If there is another indication of too much load, should not retry 
yet again.
         cluster.sensors().emit(AutoScalerPolicy.DEFAULT_POOL_HOT_SENSOR, 
message(42L, 10L, 20L));
-        assertSizeContinually(2, 1);
+        assertSizeContinually(2, 0, 1);
     }
     
     @Test
@@ -135,13 +151,14 @@ public class AutoScalerPolicyNoMoreMachinesTest extends 
BrooklynAppUnitTestSuppo
 
         // With two nodes, load is now too high, so will try (and fail) to add 
one more node.
         // Trigger another attempt to resize.
+        // Any nodes that fail with NoMachinesAvailableException will be 
immediately deleted.
         cluster.sensors().set(metric, 22);
-        assertSizeEventually(2, 1);
-        assertSizeContinually(2, 1);
-
+        assertSizeEventually(2, 0, 1);
+        assertSizeContinually(2, 0, 1);
+        
         // Metric is re-published; should not keep retrying
         cluster.sensors().set(metric, 21);
-        assertSizeContinually(2, 1);
+        assertSizeContinually(2, 0, 1);
     }
 
     protected Map<String, Object> message(double currentWorkrate, double 
lowThreshold, double highThreshold) {
@@ -160,6 +177,11 @@ public class AutoScalerPolicyNoMoreMachinesTest extends 
BrooklynAppUnitTestSuppo
         assertSize(targetSize, 0);
     }
 
+    protected void assertSize(int targetSize, int quarantineSize, final int 
deletedSize) {
+        assertSize(targetSize, quarantineSize);
+        assertEquals(entitiesRemoved.size(), deletedSize, 
"removed="+entitiesRemoved);
+    }
+    
     protected void assertSize(int targetSize, int quarantineSize) {
         assertEquals(cluster.getCurrentSize(), (Integer) targetSize, 
"cluster.currentSize");
         assertEquals(cluster.getMembers().size(), targetSize, 
"cluster.members.size");
@@ -168,20 +190,22 @@ public class AutoScalerPolicyNoMoreMachinesTest extends 
BrooklynAppUnitTestSuppo
     }
     
     protected void assertSizeEventually(int targetSize) {
-        assertSizeEventually(targetSize, 0);
+        assertSizeEventually(targetSize, 0, 0);
     }
     
-    protected void assertSizeEventually(final int targetSize, final int 
quarantineSize) {
+    protected void assertSizeEventually(final int targetSize, final int 
quarantineSize, final int deletedSize) {
         Asserts.succeedsEventually(new Runnable() {
             public void run() {
                 assertSize(targetSize, quarantineSize);
+                assertEquals(entitiesRemoved.size(), deletedSize, 
"removed="+entitiesRemoved);
             }});
     }
     
-    protected void assertSizeContinually(final int targetSize, final int 
quarantineSize) {
+    protected void assertSizeContinually(final int targetSize, final int 
quarantineSize, final int deletedSize) {
         Asserts.succeedsContinually(ImmutableMap.of("timeout", SHORT_WAIT_MS), 
new Runnable() {
             public void run() {
                 assertSize(targetSize, quarantineSize);
+                assertEquals(entitiesRemoved.size(), deletedSize, 
"removed="+entitiesRemoved);
             }});
     }
 }

Reply via email to