You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by neykov <gi...@git.apache.org> on 2017/01/16 16:50:57 UTC

[GitHub] brooklyn-server pull request #520: Limit parallelism of start/stop steps on ...

GitHub user neykov opened a pull request:

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

    Limit parallelism of start/stop steps on SoftwareProcess

    Setting a `ReleaseableLatch` as the latch of a `SoftwareProcess` allows for controlling the behaviour of the start/stop effector.
    When `ReleaseableLatch.Factory.maxConcurrency(int)` is used as the value of the latch (either directly or by referncing a `MaxConcurrencySensor`) only the specified maximum number of threads will be able to execute the immediate step the latch is guarding.
    
    It's very similar to `DynamicCluster.MAX_CONCURRENT_CHILD_COMMANDS` but allows for finer-grained control (though is specific to `SoftwareProcess`).


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

    $ git pull https://github.com/neykov/brooklyn-server max-concurrency

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

    https://github.com/apache/brooklyn-server/pull/520.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 #520
    
----
commit 112269ae484dcc5283d9cabc8de2e987f013e79f
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2017-01-16T16:37:46Z

    Add iterator to ValueResolver
    
    Resolves the initial object iteratively until either failure or object can't be resolved any more.

commit b7b5ab25a0c483b9c97d03404db2cad40a5485a4
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2017-01-16T16:40:55Z

    Let latches limit the paralellism for the step they guard

----


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r96844228
  
    --- Diff: core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolverIterator.java ---
    @@ -0,0 +1,173 @@
    +package org.apache.brooklyn.util.core.task;
    --- End diff --
    
    This file is missing the ASF license header.


---
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 #520: Limit parallelism of start/stop steps on Softwar...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:

    https://github.com/apache/brooklyn-server/pull/520
  
    @sjcorbett It's generalizing `DslDeferredFunctionCall.resolve` to be reused in the newly added code.
    
    When using DSL the actual value we want to resolve could be wrapped in several layers of Suppliers. `ValueResolver` will resolve recursively until it hits a non-supplier. That's not desirable in some situations - we are interested in the intermediate values as well. What the above method did was to resolve iteratively until it hit a certain condition (the value is of a specific type or is non-resolvable).
    
    I needed the same behaviour for the latches changes so I abstracted it in the `ValueResolverIterator`. It will resolve the value one step at a time, returning each step of the iteration or `absent` if there's a resolve error.
    
    I'll add some of this as a javadoc.


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r97598531
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/sensor/ReleaseableLatch.java ---
    @@ -0,0 +1,95 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.core.sensor;
    +
    +import java.util.concurrent.Semaphore;
    +
    +import org.apache.brooklyn.api.entity.Entity;
    +import org.apache.brooklyn.util.core.task.DeferredSupplier;
    +import org.apache.brooklyn.util.core.task.ImmediateSupplier;
    +import org.apache.brooklyn.util.exceptions.Exceptions;
    +import org.apache.brooklyn.util.guava.Maybe;
    +
    +// DeferredSupplier used as a marker interface to prevent coercion. When resolved it must evaluate to {@code Boolean.TRUE}.
    +public interface ReleaseableLatch extends DeferredSupplier<Boolean>, ImmediateSupplier<Boolean> {
    +    /**
    +     * Increment usage count for the {@code caller} entity
    +     */
    +    void acquire(Entity caller);
    +
    +    /**
    +     * Decrement usage count for the {@code caller} entity
    +     */
    +    void release(Entity caller);
    +
    +    static abstract class AbstractReleaseableLatch implements ReleaseableLatch {
    +        // Instances coerce to TRUE as they are non-null.
    +        @Override public Boolean get() {return Boolean.TRUE;}
    +        @Override public Maybe<Boolean> getImmediately() {return Maybe.of(Boolean.TRUE);}
    +    }
    +
    +    ReleaseableLatch NOP = new Factory.NopLatch();
    +
    +    static class Factory {
    +        private static class NopLatch extends AbstractReleaseableLatch {
    +            @Override public void acquire(Entity caller) {}
    +            @Override public void release(Entity caller) {}
    +        }
    +
    +        private static class MaxConcurrencyLatch extends AbstractReleaseableLatch {
    --- End diff --
    
    why put this inside here? Is it not better to put this more specific implementation in its own file? 


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r97356285
  
    --- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java ---
    @@ -444,15 +460,27 @@ public MachineLocation call() throws NoMachinesAvailableException {
             }
         }
     
    -    /** Wraps a call to {@link #preStartCustom(MachineLocation)}, after setting the hostname and address. */
    +    /**
    +     * Wraps a call to {@link #preStartCustom(MachineLocation)}, after setting the hostname and address.
    +     * @deprecated since 0.11.0. Use {@link #preStartAtMachineAsync(Supplier)} instead.
    --- End diff --
    
    good spot, updated.


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r97789809
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/sensor/ReleaseableLatch.java ---
    @@ -0,0 +1,95 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.core.sensor;
    +
    +import java.util.concurrent.Semaphore;
    +
    +import org.apache.brooklyn.api.entity.Entity;
    +import org.apache.brooklyn.util.core.task.DeferredSupplier;
    +import org.apache.brooklyn.util.core.task.ImmediateSupplier;
    +import org.apache.brooklyn.util.exceptions.Exceptions;
    +import org.apache.brooklyn.util.guava.Maybe;
    +
    +// DeferredSupplier used as a marker interface to prevent coercion. When resolved it must evaluate to {@code Boolean.TRUE}.
    +public interface ReleaseableLatch extends DeferredSupplier<Boolean>, ImmediateSupplier<Boolean> {
    +    /**
    +     * Increment usage count for the {@code caller} entity
    +     */
    +    void acquire(Entity caller);
    +
    +    /**
    +     * Decrement usage count for the {@code caller} entity
    +     */
    +    void release(Entity caller);
    +
    +    static abstract class AbstractReleaseableLatch implements ReleaseableLatch {
    +        // Instances coerce to TRUE as they are non-null.
    +        @Override public Boolean get() {return Boolean.TRUE;}
    +        @Override public Maybe<Boolean> getImmediately() {return Maybe.of(Boolean.TRUE);}
    +    }
    +
    +    ReleaseableLatch NOP = new Factory.NopLatch();
    +
    +    static class Factory {
    +        private static class NopLatch extends AbstractReleaseableLatch {
    +            @Override public void acquire(Entity caller) {}
    +            @Override public void release(Entity caller) {}
    +        }
    +
    +        private static class MaxConcurrencyLatch extends AbstractReleaseableLatch {
    +            private int permits;
    +            private transient final Semaphore sem;
    +
    +            public MaxConcurrencyLatch(int permits) {
    +                this.permits = permits;
    +                this.sem = new Semaphore(permits);
    +            }
    +
    +            @Override
    +            public void acquire(Entity caller) {
    --- End diff --
    
    I've designed the API such that it doesn't assume a semaphore below. Could be used in different ways - for example to replicate the `maxConcurrentChildCommands` behaviour. So while it's not used in this particular implementation, could be useful for alternative implementations.


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r97794132
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/sensor/ReleaseableLatch.java ---
    @@ -0,0 +1,95 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.core.sensor;
    +
    +import java.util.concurrent.Semaphore;
    +
    +import org.apache.brooklyn.api.entity.Entity;
    +import org.apache.brooklyn.util.core.task.DeferredSupplier;
    +import org.apache.brooklyn.util.core.task.ImmediateSupplier;
    +import org.apache.brooklyn.util.exceptions.Exceptions;
    +import org.apache.brooklyn.util.guava.Maybe;
    +
    +// DeferredSupplier used as a marker interface to prevent coercion. When resolved it must evaluate to {@code Boolean.TRUE}.
    +public interface ReleaseableLatch extends DeferredSupplier<Boolean>, ImmediateSupplier<Boolean> {
    +    /**
    +     * Increment usage count for the {@code caller} entity
    +     */
    +    void acquire(Entity caller);
    +
    +    /**
    +     * Decrement usage count for the {@code caller} entity
    +     */
    +    void release(Entity caller);
    +
    +    static abstract class AbstractReleaseableLatch implements ReleaseableLatch {
    +        // Instances coerce to TRUE as they are non-null.
    +        @Override public Boolean get() {return Boolean.TRUE;}
    +        @Override public Maybe<Boolean> getImmediately() {return Maybe.of(Boolean.TRUE);}
    +    }
    +
    +    ReleaseableLatch NOP = new Factory.NopLatch();
    +
    +    static class Factory {
    +        private static class NopLatch extends AbstractReleaseableLatch {
    +            @Override public void acquire(Entity caller) {}
    +            @Override public void release(Entity caller) {}
    +        }
    +
    +        private static class MaxConcurrencyLatch extends AbstractReleaseableLatch {
    +            private int permits;
    +            private transient final Semaphore sem;
    +
    +            public MaxConcurrencyLatch(int permits) {
    +                this.permits = permits;
    +                this.sem = new Semaphore(permits);
    +            }
    +
    +            @Override
    +            public void acquire(Entity caller) {
    --- End diff --
    
    makes sense; I think it probably would be worth adding the comment that the entity MAY be ignored by implementations, and callers shouldn't assume any specific behaviour based on entity, i.e. don't assume you have to match `acquire()`/`release()` on the same entity.


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r97756033
  
    --- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java ---
    @@ -944,14 +1002,19 @@ protected static boolean canStop(StopMode stopMode, boolean isStopped) {
                     stopMode == StopMode.IF_NOT_STOPPED && !isStopped;
         }
     
    +    /** @deprecated since 0.11.0. Use {@link #preStopConfirmCustom(AtomicReference)} instead. */
    +    @Deprecated
    +    protected void preStopConfirmCustom() {
    +        preStopConfirmCustom(RELEASEABLE_LATCH_TL.get());
    +    }
    +
         /** 
          * Override to check whether stop can be executed.
          * Throw if stop should be aborted.
          */
    -    protected void preStopConfirmCustom() {
    +    protected void preStopConfirmCustom(AtomicReference<ReleaseableLatch> stopLatchRef) {
             // Opportunity to block stop() until other dependent components are ready for it
    -        Object val = entity().getConfig(SoftwareProcess.STOP_LATCH);
    -        if (val != null) log.debug("{} finished waiting for stop-latch {}; continuing...", entity(), val);
    +        stopLatchRef.set(waitForLatch(entity(), SoftwareProcess.STOP_LATCH));
    --- End diff --
    
    Is it advisable to do this outside the protected method, to avoid possibility of subclasses forgetting to call super(). - i.e. wrap the`protected` method in something `private` that sets the latch reference then calls the protected method.  Perhaps this line should go into `doStopLatching` just before the call to `doStop()`.  Same for the other latches, START etc.


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r97790853
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/sensor/ReleaseableLatch.java ---
    @@ -0,0 +1,95 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.core.sensor;
    +
    +import java.util.concurrent.Semaphore;
    +
    +import org.apache.brooklyn.api.entity.Entity;
    +import org.apache.brooklyn.util.core.task.DeferredSupplier;
    +import org.apache.brooklyn.util.core.task.ImmediateSupplier;
    +import org.apache.brooklyn.util.exceptions.Exceptions;
    +import org.apache.brooklyn.util.guava.Maybe;
    +
    +// DeferredSupplier used as a marker interface to prevent coercion. When resolved it must evaluate to {@code Boolean.TRUE}.
    +public interface ReleaseableLatch extends DeferredSupplier<Boolean>, ImmediateSupplier<Boolean> {
    +    /**
    +     * Increment usage count for the {@code caller} entity
    +     */
    +    void acquire(Entity caller);
    +
    +    /**
    +     * Decrement usage count for the {@code caller} entity
    +     */
    +    void release(Entity caller);
    +
    +    static abstract class AbstractReleaseableLatch implements ReleaseableLatch {
    +        // Instances coerce to TRUE as they are non-null.
    +        @Override public Boolean get() {return Boolean.TRUE;}
    +        @Override public Maybe<Boolean> getImmediately() {return Maybe.of(Boolean.TRUE);}
    +    }
    +
    +    ReleaseableLatch NOP = new Factory.NopLatch();
    +
    +    static class Factory {
    +        private static class NopLatch extends AbstractReleaseableLatch {
    +            @Override public void acquire(Entity caller) {}
    +            @Override public void release(Entity caller) {}
    +        }
    +
    +        private static class MaxConcurrencyLatch extends AbstractReleaseableLatch {
    --- End diff --
    
    Two reasons:
      * Personal preference to inline small classes like this.
      * More importantly to hide the implementation and make it private.


---
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 #520: Limit parallelism of start/stop steps on Softwar...

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the issue:

    https://github.com/apache/brooklyn-server/pull/520
  
    The bulk of these changes look good. It would help me if you gave some context for 112269ae484dcc5283d9cabc8de2e987f013e79f.


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r97790274
  
    --- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java ---
    @@ -1083,4 +1146,39 @@ protected void clearEntityLocationAttributes(Location machine) {
             entity().sensors().set(Attributes.SUBNET_ADDRESS, null);
         }
     
    +    public static ReleaseableLatch waitForLatch(EntityInternal entity, ConfigKey<Boolean> configKey) {
    +        Maybe<?> rawValue = entity.config().getRaw(configKey);
    +        if (rawValue.isAbsent()) {
    +            return ReleaseableLatch.NOP;
    +        } else {
    +            ValueResolverIterator<Boolean> iter = resolveLatchIterator(entity, rawValue.get(), configKey);
    +            Maybe<ReleaseableLatch> releasableLatchMaybe = iter.next(ReleaseableLatch.class);
    --- End diff --
    
    `next(Class<?> type)` will return the first element instance of `type` - it's doing the looping under the hood.


---
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 #520: Limit parallelism of start/stop steps on Softwar...

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the issue:

    https://github.com/apache/brooklyn-server/pull/520
  
    @neykov looks good. I'm going to merge this. Could I ask you to add a couple of lines describing the new feature to the section on latches in the docs (https://brooklyn.apache.org/v/latest/yaml/advanced-example.html#latches)?
    
    One final thought for future work is how to improve behaviour when the user does something they shouldn't do. For example, if someone deployed this (totally not contrived!) example:
    ```yaml
    services:
    - type: org.apache.brooklyn.entity.software.base.VanillaSoftwareProcess
    
      brooklyn.initializers:
      - type: org.apache.brooklyn.core.sensor.MaxConcurrencySensor
        brooklyn.config:
          name: my-latch
          latch.concurrency.max: 1
          targetType: org.apache.brooklyn.core.sensor.ReleaseableLatch
    
      brooklyn.config: 
        launch.command: sleep 2
        checkRunning.command: true
        install.latch: $brooklyn:self().attributeWhenReady("my-latch")
        start.latch: $brooklyn:self().attributeWhenReady("my-latch")
        stop.latch: $brooklyn:self().attributeWhenReady("my-latch")
    ```
    They'll get an application that is stuck starting forever and that can't be stopped either. We could consider having a (long) timeout on the `acquire` or perhaps more interesting would be to have reentrant latches. Some strong wording in the documentation that this is a bad idea would be sufficient for the moment.
    



---
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 #520: Limit parallelism of start/stop steps on ...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r97789510
  
    --- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java ---
    @@ -731,15 +774,30 @@ protected void restartChildren(ConfigBag parameters) {
          * If no errors were encountered call {@link #postStopCustom()} at the end.
          */
         public void stop(ConfigBag parameters) {
    -        doStop(parameters, new StopAnyProvisionedMachinesTask());
    +        doStopLatching(parameters, new StopAnyProvisionedMachinesTask());
         }
     
         /**
          * As {@link #stop} but calling {@link #suspendAnyProvisionedMachines} rather than
          * {@link #stopAnyProvisionedMachines}.
          */
         public void suspend(ConfigBag parameters) {
    -        doStop(parameters, new SuspendAnyProvisionedMachinesTask());
    +        doStopLatching(parameters, new SuspendAnyProvisionedMachinesTask());
    +    }
    +
    +    protected void doStopLatching(ConfigBag parameters, Callable<StopMachineDetails<Integer>> stopTask) {
    --- End diff --
    
    It does now after latest changes :)


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r97600990
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/sensor/ReleaseableLatch.java ---
    @@ -0,0 +1,95 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.core.sensor;
    +
    +import java.util.concurrent.Semaphore;
    +
    +import org.apache.brooklyn.api.entity.Entity;
    +import org.apache.brooklyn.util.core.task.DeferredSupplier;
    +import org.apache.brooklyn.util.core.task.ImmediateSupplier;
    +import org.apache.brooklyn.util.exceptions.Exceptions;
    +import org.apache.brooklyn.util.guava.Maybe;
    +
    +// DeferredSupplier used as a marker interface to prevent coercion. When resolved it must evaluate to {@code Boolean.TRUE}.
    +public interface ReleaseableLatch extends DeferredSupplier<Boolean>, ImmediateSupplier<Boolean> {
    +    /**
    +     * Increment usage count for the {@code caller} entity
    +     */
    +    void acquire(Entity caller);
    +
    +    /**
    +     * Decrement usage count for the {@code caller} entity
    +     */
    +    void release(Entity caller);
    +
    +    static abstract class AbstractReleaseableLatch implements ReleaseableLatch {
    +        // Instances coerce to TRUE as they are non-null.
    +        @Override public Boolean get() {return Boolean.TRUE;}
    +        @Override public Maybe<Boolean> getImmediately() {return Maybe.of(Boolean.TRUE);}
    +    }
    +
    +    ReleaseableLatch NOP = new Factory.NopLatch();
    +
    +    static class Factory {
    +        private static class NopLatch extends AbstractReleaseableLatch {
    +            @Override public void acquire(Entity caller) {}
    +            @Override public void release(Entity caller) {}
    +        }
    +
    +        private static class MaxConcurrencyLatch extends AbstractReleaseableLatch {
    +            private int permits;
    +            private transient final Semaphore sem;
    +
    +            public MaxConcurrencyLatch(int permits) {
    +                this.permits = permits;
    +                this.sem = new Semaphore(permits);
    +            }
    +
    +            @Override
    +            public void acquire(Entity caller) {
    --- End diff --
    
    Why is no use made of `caller` here and in `release` - by the description of the contract above "Increment usage count for the {@code caller} entity" surely it should?   Reading that, I wouldn't expect that if I have two entities and do `latch.acquire(entity1)`, I would then be able to do `latch.release(entity2)`, or indeed `latch.release(null)`, and still release a permit on the semaphore, but that's what will happen.  It may be worth updating the comment to indicate that implementations are free to ignore the entity if that suits them.


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r97794412
  
    --- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java ---
    @@ -1083,4 +1146,39 @@ protected void clearEntityLocationAttributes(Location machine) {
             entity().sensors().set(Attributes.SUBNET_ADDRESS, null);
         }
     
    +    public static ReleaseableLatch waitForLatch(EntityInternal entity, ConfigKey<Boolean> configKey) {
    +        Maybe<?> rawValue = entity.config().getRaw(configKey);
    +        if (rawValue.isAbsent()) {
    +            return ReleaseableLatch.NOP;
    +        } else {
    +            ValueResolverIterator<Boolean> iter = resolveLatchIterator(entity, rawValue.get(), configKey);
    +            Maybe<ReleaseableLatch> releasableLatchMaybe = iter.next(ReleaseableLatch.class);
    --- End diff --
    
    ah cool


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r96845410
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/sensor/MaxConcurrencySensor.java ---
    @@ -0,0 +1,91 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.core.sensor;
    +
    +import org.apache.brooklyn.api.mgmt.Task;
    +import org.apache.brooklyn.config.ConfigKey;
    +import org.apache.brooklyn.core.config.ConfigKeys;
    +import org.apache.brooklyn.core.effector.AddSensor;
    +import org.apache.brooklyn.core.entity.Entities;
    +import org.apache.brooklyn.util.core.config.ConfigBag;
    +import org.apache.brooklyn.util.core.task.Tasks;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * Can be used as:
    + * <pre>
    + * {@code
    + * brooklyn.initializers:
    + * - type: org.apache.brooklyn.core.sensor.StaticSensor
    --- End diff --
    
    `StaticSensor` should be `MaxConcurrencySensor`


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r97789383
  
    --- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java ---
    @@ -944,14 +1002,19 @@ protected static boolean canStop(StopMode stopMode, boolean isStopped) {
                     stopMode == StopMode.IF_NOT_STOPPED && !isStopped;
         }
     
    +    /** @deprecated since 0.11.0. Use {@link #preStopConfirmCustom(AtomicReference)} instead. */
    +    @Deprecated
    +    protected void preStopConfirmCustom() {
    +        preStopConfirmCustom(RELEASEABLE_LATCH_TL.get());
    +    }
    +
         /** 
          * Override to check whether stop can be executed.
          * Throw if stop should be aborted.
          */
    -    protected void preStopConfirmCustom() {
    +    protected void preStopConfirmCustom(AtomicReference<ReleaseableLatch> stopLatchRef) {
             // Opportunity to block stop() until other dependent components are ready for it
    -        Object val = entity().getConfig(SoftwareProcess.STOP_LATCH);
    -        if (val != null) log.debug("{} finished waiting for stop-latch {}; continuing...", entity(), val);
    +        stopLatchRef.set(waitForLatch(entity(), SoftwareProcess.STOP_LATCH));
    --- End diff --
    
    Good cleanup, will update.


---
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 #520: Limit parallelism of start/stop steps on ...

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

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


---
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 #520: Limit parallelism of start/stop steps on Softwar...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:

    https://github.com/apache/brooklyn-server/pull/520
  
    @geomacy, @sam addressed comments.


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r96848337
  
    --- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcessDriverLifecycleEffectorTasks.java ---
    @@ -169,7 +173,7 @@ protected String startProcessesAtMachine(final Supplier<MachineLocation> machine
         }
     
         @Override
    -    protected void postStartCustom() {
    +    protected void postStartCustom(AtomicReference<ReleaseableLatch> startLatchRef) {
    --- End diff --
    
    There are subclasses of `SoftwareProcessDriverLifecycleEffectorTasks` and `MachineLifecycleEffectorTasks` in brooklyn-library (e.g. https://github.com/apache/brooklyn-library/blob/master/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleLifecycleEffectorTasks.java#L138, which should really have an `@Override` annotation) and elsewhere in the wild so we should at least deprecate the no-arg methods. 


---
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 #520: Limit parallelism of start/stop steps on Softwar...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:

    https://github.com/apache/brooklyn-server/pull/520
  
    @sam Addressed above comment in
    https://github.com/apache/brooklyn-server/pull/539
    https://github.com/apache/brooklyn-docs/pull/146


---
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 #520: Limit parallelism of start/stop steps on Softwar...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:

    https://github.com/apache/brooklyn-server/pull/520
  
    Addressed comments, let's discuss API backwards compatibility.


---
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 #520: Limit parallelism of start/stop steps on Softwar...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:

    https://github.com/apache/brooklyn-server/pull/520
  
    After seeing how many classes override the methods decided to keep the original signatures and deprecate them, wrapping the call with a thread local.
    Test failures fixed.


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r96905127
  
    --- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcessDriverLifecycleEffectorTasks.java ---
    @@ -169,7 +173,7 @@ protected String startProcessesAtMachine(final Supplier<MachineLocation> machine
         }
     
         @Override
    -    protected void postStartCustom() {
    +    protected void postStartCustom(AtomicReference<ReleaseableLatch> startLatchRef) {
    --- End diff --
    
    I was wondering how to do that in a backwards way. Couldn't think of a good solution.
    If we keep the old signature it won't get called so it's actually worse than getting a compile error.
    Could do something with thread locals, calling the deprecated method and passing the argument. Do you think that's something worth doing?


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r97798251
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/sensor/ReleaseableLatch.java ---
    @@ -0,0 +1,95 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.core.sensor;
    +
    +import java.util.concurrent.Semaphore;
    +
    +import org.apache.brooklyn.api.entity.Entity;
    +import org.apache.brooklyn.util.core.task.DeferredSupplier;
    +import org.apache.brooklyn.util.core.task.ImmediateSupplier;
    +import org.apache.brooklyn.util.exceptions.Exceptions;
    +import org.apache.brooklyn.util.guava.Maybe;
    +
    +// DeferredSupplier used as a marker interface to prevent coercion. When resolved it must evaluate to {@code Boolean.TRUE}.
    +public interface ReleaseableLatch extends DeferredSupplier<Boolean>, ImmediateSupplier<Boolean> {
    +    /**
    +     * Increment usage count for the {@code caller} entity
    +     */
    +    void acquire(Entity caller);
    +
    +    /**
    +     * Decrement usage count for the {@code caller} entity
    +     */
    +    void release(Entity caller);
    +
    +    static abstract class AbstractReleaseableLatch implements ReleaseableLatch {
    +        // Instances coerce to TRUE as they are non-null.
    +        @Override public Boolean get() {return Boolean.TRUE;}
    +        @Override public Maybe<Boolean> getImmediately() {return Maybe.of(Boolean.TRUE);}
    +    }
    +
    +    ReleaseableLatch NOP = new Factory.NopLatch();
    +
    +    static class Factory {
    +        private static class NopLatch extends AbstractReleaseableLatch {
    +            @Override public void acquire(Entity caller) {}
    +            @Override public void release(Entity caller) {}
    +        }
    +
    +        private static class MaxConcurrencyLatch extends AbstractReleaseableLatch {
    +            private int permits;
    +            private transient final Semaphore sem;
    +
    +            public MaxConcurrencyLatch(int permits) {
    +                this.permits = permits;
    +                this.sem = new Semaphore(permits);
    +            }
    +
    +            @Override
    +            public void acquire(Entity caller) {
    --- End diff --
    
    Updated doc. I'd expect the entity would match on acquire/release - added info for the argument as well.


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r97337170
  
    --- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java ---
    @@ -444,15 +460,27 @@ public MachineLocation call() throws NoMachinesAvailableException {
             }
         }
     
    -    /** Wraps a call to {@link #preStartCustom(MachineLocation)}, after setting the hostname and address. */
    +    /**
    +     * Wraps a call to {@link #preStartCustom(MachineLocation)}, after setting the hostname and address.
    +     * @deprecated since 0.11.0. Use {@link #preStartAtMachineAsync(Supplier)} instead.
    --- End diff --
    
    Should this point at `preStartAtMachineAsync(Supplier, AtomicReference)`?


---
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 #520: Limit parallelism of start/stop steps on Softwar...

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the issue:

    https://github.com/apache/brooklyn-server/pull/520
  
    Jenkins is failing on `SoftwareProcessEntityLatchTest.testConcurrency` and `testReleaseableLatchBlocks` 
    
    By the way, can we get rid of `maxConcurrentChildCommands` when this is merged? I don't see any reason to keep it. It's annotated `@Beta`.


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r97757004
  
    --- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java ---
    @@ -731,15 +774,30 @@ protected void restartChildren(ConfigBag parameters) {
          * If no errors were encountered call {@link #postStopCustom()} at the end.
          */
         public void stop(ConfigBag parameters) {
    -        doStop(parameters, new StopAnyProvisionedMachinesTask());
    +        doStopLatching(parameters, new StopAnyProvisionedMachinesTask());
         }
     
         /**
          * As {@link #stop} but calling {@link #suspendAnyProvisionedMachines} rather than
          * {@link #stopAnyProvisionedMachines}.
          */
         public void suspend(ConfigBag parameters) {
    -        doStop(parameters, new SuspendAnyProvisionedMachinesTask());
    +        doStopLatching(parameters, new SuspendAnyProvisionedMachinesTask());
    +    }
    +
    +    protected void doStopLatching(ConfigBag parameters, Callable<StopMachineDetails<Integer>> stopTask) {
    --- End diff --
    
    Despite the name, this doesn't actually aquire the latch - that's left to the `doStop` - see comment on `preStopConfirm` below.


---
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 #520: Limit parallelism of start/stop steps on Softwar...

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the issue:

    https://github.com/apache/brooklyn-server/pull/520
  
    Compilation fails for me with the following error:
    ```
    [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.3:testCompile (default-testCompile) on project brooklyn-software-base: Compilation failure: Compilation failure:
    [ERROR] /Users/sam/code/brooklyn.io/brooklyn/brooklyn-server/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessEntityLatchTest.java:[135,17] reference to configure is ambiguous
    [ERROR] both method <V>configure(org.apache.brooklyn.config.ConfigKey<V>,V) in org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec and method <V>configure(org.apache.brooklyn.config.ConfigKey<V>,org.apache.brooklyn.api.mgmt.Task<? extends V>) in org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec match
    [ERROR] /Users/sam/code/brooklyn.io/brooklyn/brooklyn-server/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessEntityLatchTest.java:[134,58] incompatible types: org.apache.brooklyn.api.entity.Entity cannot be converted to org.apache.brooklyn.entity.software.base.SoftwareProcessEntityTest.MyService
    [ERROR] /Users/sam/code/brooklyn.io/brooklyn/brooklyn-server/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessEntityLatchTest.java:[169,25] reference to configure is ambiguous
    [ERROR] both method <V>configure(org.apache.brooklyn.config.ConfigKey<V>,V) in org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec and method <V>configure(org.apache.brooklyn.config.ConfigKey<V>,org.apache.brooklyn.api.mgmt.Task<? extends V>) in org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec match
    ```
    Maven is building with Java 8:
    ```
    $ mvn -v
    Apache Maven 3.3.9 (bb52d8502b132ec0a5a3f4c09453c07478323dc5; 2015-11-10T16:41:47+00:00)
    Maven home: /opt/apache-maven
    Java version: 1.8.0_05, vendor: Oracle Corporation
    Java home: /Library/Java/JavaVirtualMachines/jdk1.8.0_05.jdk/Contents/Home/jre
    Default locale: en_US, platform encoding: UTF-8
    OS name: "mac os x", version: "10.11.6", arch: "x86_64", family: "mac"
    ```
    For what it's worth IntelliJ doesn't like line 135 either.


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r97746296
  
    --- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java ---
    @@ -1083,4 +1146,39 @@ protected void clearEntityLocationAttributes(Location machine) {
             entity().sensors().set(Attributes.SUBNET_ADDRESS, null);
         }
     
    +    public static ReleaseableLatch waitForLatch(EntityInternal entity, ConfigKey<Boolean> configKey) {
    +        Maybe<?> rawValue = entity.config().getRaw(configKey);
    +        if (rawValue.isAbsent()) {
    +            return ReleaseableLatch.NOP;
    +        } else {
    +            ValueResolverIterator<Boolean> iter = resolveLatchIterator(entity, rawValue.get(), configKey);
    +            Maybe<ReleaseableLatch> releasableLatchMaybe = iter.next(ReleaseableLatch.class);
    --- End diff --
    
    Should this be a `for` loop? Given that you have your new iterator here, is is not possible for the latch to be several iterations down inside some nested expression?


---
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 #520: Limit parallelism of start/stop steps on ...

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/520#discussion_r97049449
  
    --- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcessDriverLifecycleEffectorTasks.java ---
    @@ -169,7 +173,7 @@ protected String startProcessesAtMachine(final Supplier<MachineLocation> machine
         }
     
         @Override
    -    protected void postStartCustom() {
    +    protected void postStartCustom(AtomicReference<ReleaseableLatch> startLatchRef) {
    --- End diff --
    
    We could force subclasses to error at compile time by leaving the removed methods in place and making them `final`. I'd prefer that to silently ignoring the method. Your suggestion of continuing to call the deprecated method seems like the least bad approach.


---
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 #520: Limit parallelism of start/stop steps on Softwar...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:

    https://github.com/apache/brooklyn-server/pull/520
  
    Rebased and resolved merge conflicts.


---
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.
---