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

[asfgit]

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

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

[neykov]

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, 
ImmediateSupplier {
+/**
+ * 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 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[geomacy]

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 configKey) {
+Maybe rawValue = entity.config().getRaw(configKey);
+if (rawValue.isAbsent()) {
+return ReleaseableLatch.NOP;
+} else {
+ValueResolverIterator iter = 
resolveLatchIterator(entity, rawValue.get(), configKey);
+Maybe 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[geomacy]

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, 
ImmediateSupplier {
+/**
+ * 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 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[neykov]

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

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

[neykov]

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, 
ImmediateSupplier {
+/**
+ * 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 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[neykov]

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 configKey) {
+Maybe rawValue = entity.config().getRaw(configKey);
+if (rawValue.isAbsent()) {
+return ReleaseableLatch.NOP;
+} else {
+ValueResolverIterator iter = 
resolveLatchIterator(entity, rawValue.get(), configKey);
+Maybe 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[neykov]

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, 
ImmediateSupplier {
+/**
+ * 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 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[neykov]

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

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

[geomacy]

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

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

[geomacy]

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, 
ImmediateSupplier {
+/**
+ * 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 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[geomacy]

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, 
ImmediateSupplier {
+/**
+ * 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 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[geomacy]

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 configKey) {
+Maybe rawValue = entity.config().getRaw(configKey);
+if (rawValue.isAbsent()) {
+return ReleaseableLatch.NOP;
+} else {
+ValueResolverIterator iter = 
resolveLatchIterator(entity, rawValue.get(), configKey);
+Maybe 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[geomacy]

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

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

[neykov]

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

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

[sjcorbett]

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

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

[sjcorbett]

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 machine
 }
 
 @Override
-protected void postStartCustom() {
+protected void postStartCustom(AtomicReference 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[neykov]

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 machine
 }
 
 @Override
-protected void postStartCustom() {
+protected void postStartCustom(AtomicReference 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[sjcorbett]

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 machine
 }
 
 @Override
-protected void postStartCustom() {
+protected void postStartCustom(AtomicReference 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[sjcorbett]

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:
+ * 
+ * {@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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[sjcorbett]

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

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

[neykov]

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