[GitHub] brooklyn-server pull request #800: AutoScalerPolicy to resize to limits on e...

[m4rkmckenna]

Github user m4rkmckenna commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/800#discussion_r134481212
  
--- Diff: 
policy/src/main/java/org/apache/brooklyn/policy/autoscaling/AutoScalerPolicy.java
 ---
@@ -458,6 +459,25 @@ public void onEvent(SensorEvent event) {
 }
 };
 
+/**
+ * This should usually be a no-op as the min and max pool sizes are 
taken into account when
+ * an automatic resize event occurs, however this covers the special 
case where the user
+ * has manually resized the pool
+ */
+private SensorEventListener poolEventHandler = new 
SensorEventListener() {
+@Override
+public void onEvent(SensorEvent event) {
+Sensor sensor = event.getSensor();
+assert sensor.equals(DynamicCluster.GROUP_SIZE);
--- End diff --

Is `assert` not removed at runtime??


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server issue #803: Adds cluster.max.size to dynamic cluster

[nakomis]

Github user nakomis commented on the issue:

https://github.com/apache/brooklyn-server/pull/803
  
PR comments addressed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request #803: Adds cluster.max.size to dynamic cluster

[m4rkmckenna]

Github user m4rkmckenna commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/803#discussion_r134428459
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/entity/group/DynamicClusterImpl.java ---
@@ -595,6 +595,10 @@ public void restart() {
 @Override
 public Integer resize(Integer desiredSize) {
 synchronized (mutex) {
+Optional optionalMaxSize = 
Optional.fromNullable(config().get(MAX_SIZE));
+if (optionalMaxSize.isPresent() && desiredSize > 
optionalMaxSize.get()) {
+throw new IllegalArgumentException("Desired cluster size " 
+ desiredSize + " exceeds maximum size of " + optionalMaxSize.get());
--- End diff --

Should this not be `InsufficientCapacityException `


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request #803: Adds cluster.max.size to dynamic cluster

[m4rkmckenna]

Github user m4rkmckenna commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/803#discussion_r134428127
  
--- Diff: 
core/src/test/java/org/apache/brooklyn/entity/group/DynamicClusterTest.java ---
@@ -1378,6 +1378,24 @@ public void 
testChildCommandPermitNotReleasedWhenMemberStartTaskCancelledBeforeS
 
assertEquals(clusterImpl.getChildTaskSemaphore().availablePermits(), 1);
 }
 
+@Test
+public void testClusterMaxSize() {
+DynamicCluster cluster = 
app.createAndManageChild(EntitySpec.create(DynamicCluster.class)
+.configure(DynamicCluster.MEMBER_SPEC, 
EntitySpec.create(TestEntity.class))
+.configure(DynamicCluster.INITIAL_SIZE, 2)
+.configure(DynamicCluster.MAX_SIZE, 6));
+cluster.start(ImmutableList.of(loc));
+
+cluster.resize(4);
+EntityAsserts.assertAttributeEqualsEventually(cluster, 
DynamicCluster.GROUP_SIZE, 4);
+try {
+cluster.resize(8);
+Asserts.shouldHaveFailedPreviously("Cluster resize should have 
failed because max size was exceeded");
+} catch (Exception e) {
+// ignored.
--- End diff --

You could assert that the expected exception is thown ... Empty catch 
blocks is a code smell


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request #803: Adds cluster.max.size to dynamic cluster

[nakomis]

GitHub user nakomis opened a pull request:

https://github.com/apache/brooklyn-server/pull/803

Adds cluster.max.size to dynamic cluster

Adds cluster.max.size to dynamic cluster, and prevents the cluster from 
being manually increased beyond that size

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/nakomis/brooklyn-server cluster-max-size

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/brooklyn-server/pull/803.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #803


commit 6598ee1b7d22107434824bfafad6d8d2e31271e5
Author: Martin Harris 
Date:   2017-08-22T08:10:39Z

Adds cluster.max.size to dynamic cluster




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---