You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by nakomis <gi...@git.apache.org> on 2017/08/22 09:12:33 UTC

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

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 <ma...@cloudsoft.io>
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by m4rkmckenna <gi...@git.apache.org>.
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<Integer> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by nakomis <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by m4rkmckenna <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---