You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2014/10/16 17:29:09 UTC
[GitHub] incubator-brooklyn pull request: Tasks stop on unmanaged
GitHub user ahgittin opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/252
Tasks stop on unmanaged
stop tasks when entities are unmanaged, and better bookkeeping for scheduled tasks.
(also fixes eclipse IDE problems caused by checkstyle.)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahgittin/incubator-brooklyn tasks-stop-on-unmanaged
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/252.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 #252
----
commit 0bb2e325f3c613906f67e3ad02d3480c3f4e671d
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-10-15T22:11:24Z
exclude maven check-style in eclipse, to suppress eclipse errors
commit 2a0de8f282db187b3ab41a74f332d76a322c9946
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-10-15T22:18:03Z
support changing mode to disabled, ensuring things really are purged from memory; and allow HA tests based on RebindTestUtils/Fixture
commit a22adfd5a7a5ba7040cafc1208c94541dc2b5126
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-10-16T09:49:11Z
better bookkeeping for scheduled tasks (counts go to 0 now), and set tags on scheduled tasks
commit 6ea763151d8f7424c6e1e772f872a2f6c84d6da3
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-10-16T10:52:11Z
switch incomplete tasks to be tracked by ID, rather than a simple count, so we can GC those which are interrupted (and so their afterEnd method is never called),
ensuring incomplete counts are accurate modulo interrupted-but-not-garbage-collected tasks; and log if tasks are removed while active
commit df0f307983b36e43ea1d456a46d4070f91f39ea5
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-10-15T22:19:05Z
ensure feeds (and all entity tasks) are cancelled when entities are unmanaged, including in-memory test and an integration test with nginx; now it will GC entity tasks *after* fully unmanaged, but will cancel entity tasks *during* unmanage. also fixes nginx issue where it tries to reload conf immediately on rebind
commit e56ff2d0ffa3a57213c9f03ac21a20de51fc6d7c
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-10-16T15:23:07Z
a few minor tidies for GC'ing tasks and alerting on nginx reload/update failure
commit 3889a49b29316194b0fbf88971f34555dff8b059
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-10-16T15:27:46Z
Merge remote branch 'tasks-stop-on-unmanaged` into tasks-stop-on-unmanaged
----
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r19007830
--- Diff: core/src/test/java/brooklyn/entity/group/DynamicFabricTest.java ---
@@ -226,7 +226,7 @@ public void testDynamicFabricStartsEntitiesInParallel() throws Exception {
@Override public Boolean call() {
return latches.size() == locs.size();
}})
- .run();
+ .runRequiringTrue();
--- End diff --
This particular instance I just fixed the `runRequiringTrue` bit but elsewhere I added some `Repeater` calls. It's just such a nice API :) but agree `Asserts.eventually` is more appropriate and allows external control of timings.
(One minor point, if I'm *debugging* tests, it's nice to have control of timings, so perhaps a `Asserts.DEFAULT_EVENTUALLY_TIMEOUT` field would help.)
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r19008375
--- Diff: core/src/main/java/brooklyn/util/task/BasicExecutionManager.java ---
@@ -195,10 +204,21 @@ public void deleteTask(Task<?> task) {
protected boolean deleteTaskNonRecursive(Task<?> task) {
Set<?> tags = checkNotNull(task, "task").getTags();
for (Object tag : tags) {
- Set<Task<?>> tasks = tasksWithTagLiveOrNull(tag);
- if (tasks != null) tasks.remove(task);
+ synchronized (tasksByTag) {
+ Set<Task<?>> tasks = tasksWithTagLiveOrNull(tag);
+ if (tasks != null) {
+ tasks.remove(task);
+ if (tasks.isEmpty()) {
+ tasksByTag.remove(tag);
+ }
+ }
+ }
}
Task<?> removed = tasksById.remove(task.getId());
+ incompleteTaskIds.remove(task.getId());
+ if (removed!=null && removed.isSubmitted() && !removed.isDone()) {
+ log.warn("Deleting submitted but incomplete task (cancel it first): "+removed);
--- End diff --
gone with:
log.warn("Deleting submitted task before completion: "+removed+"; this task will continue to run in the background outwith "+this+", but perhaps it should have been cancelled?");
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r19008096
--- Diff: core/src/test/java/brooklyn/entity/rebind/RebindFeedTest.java ---
@@ -100,8 +119,33 @@ public void testHttpFeedRegisteredInInitIsPersisted() throws Exception {
newEntity.setAttribute(SENSOR_STRING, null);
EntityTestUtils.assertAttributeEqualsEventually(newEntity, SENSOR_INT, (Integer)200);
EntityTestUtils.assertAttributeEqualsEventually(newEntity, SENSOR_STRING, "{\"foo\":\"myfoo\"}");
+
+ // Now test that everything in the origApp stops, including feeds
+ Entities.unmanage(origApp);
+ origApp = null;
+ origManagementContext.getRebindManager().stop();
+ Repeater.create().every(Duration.millis(20)).limitTimeTo(Duration.TEN_SECONDS).until(new Callable<Boolean>() {
--- End diff --
actually, with `Asserts.succeedsEventually(...)` I need to do an extra `throw` inside the body.
especially for `Callable` I think we want
* `Asserts.eventually(Callable<T>, Predicate<T>)`
* `Asserts.eventuallyEquals(Callable<T>, T)`
* `Asserts.eventuallyTrue(Callable<Boolean>)`
The name `succeedsEventually` (which given the preponderance of conditions I am not thinking we reverse to `eventuallyXxxxx`) is misleading with callable. I'd have just `eventually(Callable<T>)` and `eventually(Runnable)`.
Worth looking at the `assertj` library too. I like the pattern `Asserts.that(X).eventually().verb(...)` which I think it follows.
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r18990207
--- Diff: core/src/main/java/brooklyn/management/internal/LocalLocationManager.java ---
@@ -358,7 +369,7 @@ private synchronized boolean manageNonRecursive(Location loc, ManagementTransiti
* Returns true if the location has been removed from management; if it was not previously managed (anything else throws exception)
*/
private synchronized boolean unmanageNonRecursive(Location loc) {
- loc.setParent(null);
+ ((AbstractLocation)loc).setParent(null, false);
--- End diff --
I can see why we'd want to do this when shutting down (or clearing state when transitioning to "failed"), but is this the right thing to do in normal situation when location is being unmanaged? Will the parent still have a reference to the unmanaged location in its `getChildren()`?
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r18994063
--- Diff: core/src/main/java/brooklyn/util/task/BasicExecutionManager.java ---
@@ -101,24 +95,36 @@
// TODO Could have a set of all knownTasks; but instead we're having a separate set per tag,
// so the same task could be listed multiple times if it has multiple tags...
- //access to the below is synchronized in code in this class, to allow us to preserve order while guaranteeing thread-safe
+ //access to this field AND to members in this field is synchronized,
+ //to allow us to preserve order while guaranteeing thread-safe
//(but more testing is needed before we are sure it is thread-safe!)
//synch blocks are as finely grained as possible for efficiency
//Not using a CopyOnWriteArraySet for each, because profiling showed this being a massive perf bottleneck.
- private ConcurrentMap<Object,Set<Task<?>>> tasksByTag = new ConcurrentHashMap<Object,Set<Task<?>>>();
+ private Map<Object,Set<Task<?>>> tasksByTag = new HashMap<Object,Set<Task<?>>>();
--- End diff --
Just to check... is the reason to use a map we synchronize on (rather than a `ConcurrentHashMap`) so that we can safely delete the entry when the set of tasks for that key is empty?
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r19008176
--- Diff: software/webapp/src/test/java/brooklyn/entity/proxy/nginx/NginxRebindWithHaIntegrationTest.java ---
@@ -0,0 +1,174 @@
+/*
+ * 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 brooklyn.entity.proxy.nginx;
+
+import java.net.URL;
+import java.util.Collection;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import brooklyn.entity.Feed;
+import brooklyn.entity.basic.Attributes;
+import brooklyn.entity.basic.Entities;
+import brooklyn.entity.basic.EntityInternal;
+import brooklyn.entity.basic.Lifecycle;
+import brooklyn.entity.group.DynamicCluster;
+import brooklyn.entity.proxying.EntitySpec;
+import brooklyn.entity.rebind.RebindTestFixtureWithApp;
+import brooklyn.entity.rebind.RebindTestUtils;
+import brooklyn.entity.webapp.tomcat.TomcatServer;
+import brooklyn.internal.BrooklynFeatureEnablement;
+import brooklyn.location.LocationSpec;
+import brooklyn.location.basic.LocalhostMachineProvisioningLocation;
+import brooklyn.location.basic.SshMachineLocationReuseIntegrationTest.RecordingSshjTool;
+import brooklyn.management.Task;
+import brooklyn.management.ha.HighAvailabilityMode;
+import brooklyn.management.internal.LocalManagementContext;
+import brooklyn.test.EntityTestUtils;
+import brooklyn.test.WebAppMonitor;
+import brooklyn.test.entity.TestApplication;
+import brooklyn.util.internal.ssh.SshTool;
+import brooklyn.util.net.Networking;
+import brooklyn.util.repeat.Repeater;
+import brooklyn.util.task.BasicExecutionManager;
+import brooklyn.util.time.Duration;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+
+/**
+ * Test the operation of the {@link NginxController} class.
+ */
+public class NginxRebindWithHaIntegrationTest extends RebindTestFixtureWithApp {
+
+ private static final Logger LOG = LoggerFactory.getLogger(NginxRebindWithHaIntegrationTest.class);
+
+ private URL warUrl;
+ private List<WebAppMonitor> webAppMonitors = new CopyOnWriteArrayList<WebAppMonitor>();
+ private ExecutorService executor;
+ private LocalhostMachineProvisioningLocation loc;
+
+ private Boolean feedRegistration;
+
+ @Override
+ protected boolean useLiveManagementContext() {
+ // For Aled, the test failed without own ~/.brooklyn/brooklyn.properties.
+ // Suspect that was caused by local environment, with custom brooklyn.ssh.config.scriptHeader
+ // to set things like correct Java on path.
+ return true;
+ }
+
+ @BeforeMethod(groups = "Integration")
+ public void setUp() throws Exception {
+ super.setUp();
+ warUrl = getClass().getClassLoader().getResource("hello-world.war");
+ loc = origManagementContext.getLocationManager().createLocation(LocationSpec.create(LocalhostMachineProvisioningLocation.class)
+ .configure("address", Networking.getLocalHost())
+ .configure(SshTool.PROP_TOOL_CLASS, RecordingSshjTool.class.getName()));
+ executor = Executors.newCachedThreadPool();
+
+ feedRegistration = BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_FEED_REGISTRATION_PROPERTY);
+ BrooklynFeatureEnablement.setEnablement(BrooklynFeatureEnablement.FEATURE_FEED_REGISTRATION_PROPERTY, true);
+ }
+
+ @AfterMethod(groups = "Integration", alwaysRun=true)
+ public void tearDown() throws Exception {
+ if (feedRegistration!=null)
+ BrooklynFeatureEnablement.setEnablement(BrooklynFeatureEnablement.FEATURE_FEED_REGISTRATION_PROPERTY, feedRegistration);
+
+ for (WebAppMonitor monitor : webAppMonitors) {
+ monitor.terminate();
+ }
--- End diff --
changed in both old and new test
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r18992921
--- Diff: core/src/test/java/brooklyn/entity/group/DynamicFabricTest.java ---
@@ -226,7 +226,7 @@ public void testDynamicFabricStartsEntitiesInParallel() throws Exception {
@Override public Boolean call() {
return latches.size() == locs.size();
}})
- .run();
+ .runRequiringTrue();
--- End diff --
Instead of this whole repeater call, can just do:
Asserts.eventually(Suppliers.ofInstance(latches), CollectionFunctionals.sizeEquals(locs.size()));
In general, I don't like us using `Repeater` in tests. There are almost no situations where one actually wants to specify the times in the test class (i.e. just rely on default of 30 seconds, unless it really is a performance test or expected to be really long). Where there isn't a nice way of using `Asserts.eventually` as above, then I'd prefer if we defaulted to using things like:
Asserts.succeedsEventually(new Runnable() {
public void run() {
assertEquals(latches.size(), locs.size());
}});
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r18992322
--- Diff: core/src/main/java/brooklyn/management/internal/BrooklynGarbageCollector.java ---
@@ -226,6 +235,12 @@ public void shutdownNow() {
}
public void onUnmanaged(Entity entity) {
+ synchronized (unmanagedEntitiesNeedingGc) {
--- End diff --
Worth a comment to say why deferring the deletion of its tasks. e.g. something like `don't delete record of its tasks yet; these will be needed to cancel the tasks`.
Was about to ask what deletes the entry from `unmanagedEntitiesNeedingGc`, but see you've added that in a later commit :-)
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r18993415
--- Diff: software/webapp/src/test/java/brooklyn/entity/proxy/nginx/NginxRebindWithHaIntegrationTest.java ---
@@ -0,0 +1,174 @@
+/*
+ * 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 brooklyn.entity.proxy.nginx;
+
+import java.net.URL;
+import java.util.Collection;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import brooklyn.entity.Feed;
+import brooklyn.entity.basic.Attributes;
+import brooklyn.entity.basic.Entities;
+import brooklyn.entity.basic.EntityInternal;
+import brooklyn.entity.basic.Lifecycle;
+import brooklyn.entity.group.DynamicCluster;
+import brooklyn.entity.proxying.EntitySpec;
+import brooklyn.entity.rebind.RebindTestFixtureWithApp;
+import brooklyn.entity.rebind.RebindTestUtils;
+import brooklyn.entity.webapp.tomcat.TomcatServer;
+import brooklyn.internal.BrooklynFeatureEnablement;
+import brooklyn.location.LocationSpec;
+import brooklyn.location.basic.LocalhostMachineProvisioningLocation;
+import brooklyn.location.basic.SshMachineLocationReuseIntegrationTest.RecordingSshjTool;
+import brooklyn.management.Task;
+import brooklyn.management.ha.HighAvailabilityMode;
+import brooklyn.management.internal.LocalManagementContext;
+import brooklyn.test.EntityTestUtils;
+import brooklyn.test.WebAppMonitor;
+import brooklyn.test.entity.TestApplication;
+import brooklyn.util.internal.ssh.SshTool;
+import brooklyn.util.net.Networking;
+import brooklyn.util.repeat.Repeater;
+import brooklyn.util.task.BasicExecutionManager;
+import brooklyn.util.time.Duration;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+
+/**
+ * Test the operation of the {@link NginxController} class.
+ */
+public class NginxRebindWithHaIntegrationTest extends RebindTestFixtureWithApp {
+
+ private static final Logger LOG = LoggerFactory.getLogger(NginxRebindWithHaIntegrationTest.class);
+
+ private URL warUrl;
+ private List<WebAppMonitor> webAppMonitors = new CopyOnWriteArrayList<WebAppMonitor>();
+ private ExecutorService executor;
+ private LocalhostMachineProvisioningLocation loc;
+
+ private Boolean feedRegistration;
+
+ @Override
+ protected boolean useLiveManagementContext() {
+ // For Aled, the test failed without own ~/.brooklyn/brooklyn.properties.
+ // Suspect that was caused by local environment, with custom brooklyn.ssh.config.scriptHeader
+ // to set things like correct Java on path.
+ return true;
+ }
+
+ @BeforeMethod(groups = "Integration")
--- End diff --
Always use `@BeforeMethod(alwaysRun=true)`. Otherwise, if someone adds another test into this class that is not integration group, it will pass in IDE and fail at command line because the setUp won't be run.
Same for `@AfterMethod`: don't include `groups="Integration"`
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/252
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r19008143
--- Diff: software/webapp/src/test/java/brooklyn/entity/proxy/nginx/NginxRebindWithHaIntegrationTest.java ---
@@ -0,0 +1,174 @@
+/*
+ * 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 brooklyn.entity.proxy.nginx;
+
+import java.net.URL;
+import java.util.Collection;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import brooklyn.entity.Feed;
+import brooklyn.entity.basic.Attributes;
+import brooklyn.entity.basic.Entities;
+import brooklyn.entity.basic.EntityInternal;
+import brooklyn.entity.basic.Lifecycle;
+import brooklyn.entity.group.DynamicCluster;
+import brooklyn.entity.proxying.EntitySpec;
+import brooklyn.entity.rebind.RebindTestFixtureWithApp;
+import brooklyn.entity.rebind.RebindTestUtils;
+import brooklyn.entity.webapp.tomcat.TomcatServer;
+import brooklyn.internal.BrooklynFeatureEnablement;
+import brooklyn.location.LocationSpec;
+import brooklyn.location.basic.LocalhostMachineProvisioningLocation;
+import brooklyn.location.basic.SshMachineLocationReuseIntegrationTest.RecordingSshjTool;
+import brooklyn.management.Task;
+import brooklyn.management.ha.HighAvailabilityMode;
+import brooklyn.management.internal.LocalManagementContext;
+import brooklyn.test.EntityTestUtils;
+import brooklyn.test.WebAppMonitor;
+import brooklyn.test.entity.TestApplication;
+import brooklyn.util.internal.ssh.SshTool;
+import brooklyn.util.net.Networking;
+import brooklyn.util.repeat.Repeater;
+import brooklyn.util.task.BasicExecutionManager;
+import brooklyn.util.time.Duration;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+
+/**
+ * Test the operation of the {@link NginxController} class.
+ */
+public class NginxRebindWithHaIntegrationTest extends RebindTestFixtureWithApp {
+
+ private static final Logger LOG = LoggerFactory.getLogger(NginxRebindWithHaIntegrationTest.class);
+
+ private URL warUrl;
+ private List<WebAppMonitor> webAppMonitors = new CopyOnWriteArrayList<WebAppMonitor>();
+ private ExecutorService executor;
+ private LocalhostMachineProvisioningLocation loc;
+
+ private Boolean feedRegistration;
+
+ @Override
+ protected boolean useLiveManagementContext() {
+ // For Aled, the test failed without own ~/.brooklyn/brooklyn.properties.
+ // Suspect that was caused by local environment, with custom brooklyn.ssh.config.scriptHeader
+ // to set things like correct Java on path.
+ return true;
+ }
+
+ @BeforeMethod(groups = "Integration")
--- End diff --
agree - i just copied `NginxRebindIntegrationTest` which i've also changed
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r19008650
--- Diff: core/src/test/java/brooklyn/management/internal/EntityExecutionManagerTest.java ---
@@ -336,8 +337,8 @@ public void testUnmanagedEntityGcedOnUnmanageEvenIfEffectorInvoked() throws Exce
BasicAttributeSensor<Object> byteArrayAttrib = new BasicAttributeSensor<Object>(Object.class, "test.byteArray", "");
- for (int i = 0; i < 1000; i++) {
- if (i%100==0) LOG.info(JavaClassNames.niceClassAndMethod()+": iteration "+i);
+ for (int i = 0; i < 200; i++) {
--- End diff --
you're right, works fine without the extra `gc` calls, have reverted to 1000 and commented out that line w explanation
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r18990314
--- Diff: core/src/test/java/brooklyn/management/ha/HotStandbyTest.java ---
@@ -560,5 +562,36 @@ public void testChangeMode() throws Exception {
public void testChangeModeManyTimes() throws Exception {
testChangeMode();
}
+
+ @Test
+ public void testChangeModeToDisabledAndBack() throws Exception {
+ HaMgmtNode n1 = createMaster(Duration.PRACTICALLY_FOREVER);
+ n1.mgmt.getLocationManager().createLocation(LocationSpec.create(LocalhostMachine.class));
+ @SuppressWarnings("unused")
+ TestApplication app = createFirstAppAndPersist(n1);
+
+ HaMgmtNode n2 = createHotStandby(Duration.PRACTICALLY_FOREVER);
+
+ // disabled n1 allows n2 to become master when next we tell it to check
+ n1.ha.changeMode(HighAvailabilityMode.DISABLED);
+ n2.ha.changeMode(HighAvailabilityMode.AUTO);
+ assertMaster(n2);
+ assertEquals(n1.ha.getNodeState(), ManagementNodeState.FAILED);
+ Assert.assertTrue(n1.mgmt.getApplications().isEmpty(), "n1 should have had no apps; instead had: "+n1.mgmt.getApplications());
+ Assert.assertTrue(n1.mgmt.getEntityManager().getEntities().isEmpty(), "n1 should have had no entities; instead had: "+n1.mgmt.getEntityManager().getEntities());
+ Assert.assertTrue(n1.mgmt.getLocationManager().getLocations().isEmpty(), "n1 should have had no locations; instead had: "+n1.mgmt.getLocationManager().getLocations());
+
+ // we can now change n1 back to hot_standby
+ n1.ha.changeMode(HighAvailabilityMode.HOT_STANDBY);
+ assertHotStandby(n1);
+ // and it sees apps
+ Assert.assertFalse(n1.mgmt.getApplications().isEmpty(), "n1 should have had apps now");
+ Assert.assertFalse(n1.mgmt.getApplications().isEmpty(), "n1 should have had locations now");
--- End diff --
Should be `n1.mgmt.getLocationManager().getLocations().isEmpty()`
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r19010100
--- Diff: software/webapp/src/test/java/brooklyn/entity/proxy/nginx/NginxRebindWithHaIntegrationTest.java ---
@@ -0,0 +1,174 @@
+/*
+ * 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 brooklyn.entity.proxy.nginx;
+
+import java.net.URL;
+import java.util.Collection;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import brooklyn.entity.Feed;
+import brooklyn.entity.basic.Attributes;
+import brooklyn.entity.basic.Entities;
+import brooklyn.entity.basic.EntityInternal;
+import brooklyn.entity.basic.Lifecycle;
+import brooklyn.entity.group.DynamicCluster;
+import brooklyn.entity.proxying.EntitySpec;
+import brooklyn.entity.rebind.RebindTestFixtureWithApp;
+import brooklyn.entity.rebind.RebindTestUtils;
+import brooklyn.entity.webapp.tomcat.TomcatServer;
+import brooklyn.internal.BrooklynFeatureEnablement;
+import brooklyn.location.LocationSpec;
+import brooklyn.location.basic.LocalhostMachineProvisioningLocation;
+import brooklyn.location.basic.SshMachineLocationReuseIntegrationTest.RecordingSshjTool;
+import brooklyn.management.Task;
+import brooklyn.management.ha.HighAvailabilityMode;
+import brooklyn.management.internal.LocalManagementContext;
+import brooklyn.test.EntityTestUtils;
+import brooklyn.test.WebAppMonitor;
+import brooklyn.test.entity.TestApplication;
+import brooklyn.util.internal.ssh.SshTool;
+import brooklyn.util.net.Networking;
+import brooklyn.util.repeat.Repeater;
+import brooklyn.util.task.BasicExecutionManager;
+import brooklyn.util.time.Duration;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+
+/**
+ * Test the operation of the {@link NginxController} class.
+ */
+public class NginxRebindWithHaIntegrationTest extends RebindTestFixtureWithApp {
+
+ private static final Logger LOG = LoggerFactory.getLogger(NginxRebindWithHaIntegrationTest.class);
+
+ private URL warUrl;
+ private List<WebAppMonitor> webAppMonitors = new CopyOnWriteArrayList<WebAppMonitor>();
+ private ExecutorService executor;
+ private LocalhostMachineProvisioningLocation loc;
+
+ private Boolean feedRegistration;
+
+ @Override
+ protected boolean useLiveManagementContext() {
+ // For Aled, the test failed without own ~/.brooklyn/brooklyn.properties.
+ // Suspect that was caused by local environment, with custom brooklyn.ssh.config.scriptHeader
+ // to set things like correct Java on path.
+ return true;
+ }
+
+ @BeforeMethod(groups = "Integration")
+ public void setUp() throws Exception {
+ super.setUp();
+ warUrl = getClass().getClassLoader().getResource("hello-world.war");
+ loc = origManagementContext.getLocationManager().createLocation(LocationSpec.create(LocalhostMachineProvisioningLocation.class)
+ .configure("address", Networking.getLocalHost())
+ .configure(SshTool.PROP_TOOL_CLASS, RecordingSshjTool.class.getName()));
+ executor = Executors.newCachedThreadPool();
+
+ feedRegistration = BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_FEED_REGISTRATION_PROPERTY);
+ BrooklynFeatureEnablement.setEnablement(BrooklynFeatureEnablement.FEATURE_FEED_REGISTRATION_PROPERTY, true);
+ }
+
+ @AfterMethod(groups = "Integration", alwaysRun=true)
+ public void tearDown() throws Exception {
+ if (feedRegistration!=null)
+ BrooklynFeatureEnablement.setEnablement(BrooklynFeatureEnablement.FEATURE_FEED_REGISTRATION_PROPERTY, feedRegistration);
+
+ for (WebAppMonitor monitor : webAppMonitors) {
+ monitor.terminate();
+ }
+ if (executor != null) executor.shutdownNow();
+ super.tearDown();
+ RecordingSshjTool.reset();
+ }
+
+ @Override
+ protected TestApplication createApp() {
+ origManagementContext.getHighAvailabilityManager().changeMode(HighAvailabilityMode.MASTER);
+ return super.createApp();
+ }
+
+ @Test(groups = "Integration")
+ public void testChangeModeFailureStopsTasksButHappyUponResumption() throws Exception {
+ DynamicCluster origServerPool = origApp.createAndManageChild(EntitySpec.create(DynamicCluster.class)
+ .configure(DynamicCluster.MEMBER_SPEC, EntitySpec.create(TomcatServer.class).configure("war", warUrl.toString()))
--- End diff --
we do a lot of tests w JBoss -- there were reports tomcat might have ssh issues on startup (but it didn't). no strong feelings, but i figure both are worth stressing a bit more.
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#issuecomment-59441812
completely agree re `ScheduledTask` API being poor. we don't actually use it in that many places. but i think it's low priority (and likely to be quite time consuming, knowing how those things go)
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r18993103
--- Diff: core/src/test/java/brooklyn/entity/rebind/RebindFeedTest.java ---
@@ -100,8 +119,33 @@ public void testHttpFeedRegisteredInInitIsPersisted() throws Exception {
newEntity.setAttribute(SENSOR_STRING, null);
EntityTestUtils.assertAttributeEqualsEventually(newEntity, SENSOR_INT, (Integer)200);
EntityTestUtils.assertAttributeEqualsEventually(newEntity, SENSOR_STRING, "{\"foo\":\"myfoo\"}");
+
+ // Now test that everything in the origApp stops, including feeds
+ Entities.unmanage(origApp);
+ origApp = null;
+ origManagementContext.getRebindManager().stop();
+ Repeater.create().every(Duration.millis(20)).limitTimeTo(Duration.TEN_SECONDS).until(new Callable<Boolean>() {
--- End diff --
Again, I'd prefer `Asserts.succeedsEventually(new Runnable() {...})` with appropriate `assertEquals` etc in the runnable impl. I see no reason to specify ten second timeout, or to not just rely on the nice backoff behaviour we have in our `Asserts` code (except to limit logging to at most once every 20ms).
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r18993739
--- Diff: core/src/main/java/brooklyn/util/task/BasicExecutionManager.java ---
@@ -195,10 +204,21 @@ public void deleteTask(Task<?> task) {
protected boolean deleteTaskNonRecursive(Task<?> task) {
Set<?> tags = checkNotNull(task, "task").getTags();
for (Object tag : tags) {
- Set<Task<?>> tasks = tasksWithTagLiveOrNull(tag);
- if (tasks != null) tasks.remove(task);
+ synchronized (tasksByTag) {
+ Set<Task<?>> tasks = tasksWithTagLiveOrNull(tag);
+ if (tasks != null) {
+ tasks.remove(task);
+ if (tasks.isEmpty()) {
+ tasksByTag.remove(tag);
+ }
+ }
+ }
}
Task<?> removed = tasksById.remove(task.getId());
+ incompleteTaskIds.remove(task.getId());
+ if (removed!=null && removed.isSubmitted() && !removed.isDone()) {
+ log.warn("Deleting submitted but incomplete task (cancel it first): "+removed);
--- End diff --
Perhaps word as `"Deleting submitted but incomplete task (without automatically cancelling it first): "+removed`?
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r19008511
--- Diff: core/src/main/java/brooklyn/util/task/BasicExecutionManager.java ---
@@ -101,24 +95,36 @@
// TODO Could have a set of all knownTasks; but instead we're having a separate set per tag,
// so the same task could be listed multiple times if it has multiple tags...
- //access to the below is synchronized in code in this class, to allow us to preserve order while guaranteeing thread-safe
+ //access to this field AND to members in this field is synchronized,
+ //to allow us to preserve order while guaranteeing thread-safe
//(but more testing is needed before we are sure it is thread-safe!)
//synch blocks are as finely grained as possible for efficiency
//Not using a CopyOnWriteArraySet for each, because profiling showed this being a massive perf bottleneck.
- private ConcurrentMap<Object,Set<Task<?>>> tasksByTag = new ConcurrentHashMap<Object,Set<Task<?>>>();
+ private Map<Object,Set<Task<?>>> tasksByTag = new HashMap<Object,Set<Task<?>>>();
--- End diff --
yes. i noticed a leak where we might ask for tasks with tags after a tag is no longer in use.
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r18993473
--- Diff: software/webapp/src/test/java/brooklyn/entity/proxy/nginx/NginxRebindWithHaIntegrationTest.java ---
@@ -0,0 +1,174 @@
+/*
+ * 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 brooklyn.entity.proxy.nginx;
+
+import java.net.URL;
+import java.util.Collection;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import brooklyn.entity.Feed;
+import brooklyn.entity.basic.Attributes;
+import brooklyn.entity.basic.Entities;
+import brooklyn.entity.basic.EntityInternal;
+import brooklyn.entity.basic.Lifecycle;
+import brooklyn.entity.group.DynamicCluster;
+import brooklyn.entity.proxying.EntitySpec;
+import brooklyn.entity.rebind.RebindTestFixtureWithApp;
+import brooklyn.entity.rebind.RebindTestUtils;
+import brooklyn.entity.webapp.tomcat.TomcatServer;
+import brooklyn.internal.BrooklynFeatureEnablement;
+import brooklyn.location.LocationSpec;
+import brooklyn.location.basic.LocalhostMachineProvisioningLocation;
+import brooklyn.location.basic.SshMachineLocationReuseIntegrationTest.RecordingSshjTool;
+import brooklyn.management.Task;
+import brooklyn.management.ha.HighAvailabilityMode;
+import brooklyn.management.internal.LocalManagementContext;
+import brooklyn.test.EntityTestUtils;
+import brooklyn.test.WebAppMonitor;
+import brooklyn.test.entity.TestApplication;
+import brooklyn.util.internal.ssh.SshTool;
+import brooklyn.util.net.Networking;
+import brooklyn.util.repeat.Repeater;
+import brooklyn.util.task.BasicExecutionManager;
+import brooklyn.util.time.Duration;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+
+/**
+ * Test the operation of the {@link NginxController} class.
+ */
+public class NginxRebindWithHaIntegrationTest extends RebindTestFixtureWithApp {
+
+ private static final Logger LOG = LoggerFactory.getLogger(NginxRebindWithHaIntegrationTest.class);
+
+ private URL warUrl;
+ private List<WebAppMonitor> webAppMonitors = new CopyOnWriteArrayList<WebAppMonitor>();
+ private ExecutorService executor;
+ private LocalhostMachineProvisioningLocation loc;
+
+ private Boolean feedRegistration;
+
+ @Override
+ protected boolean useLiveManagementContext() {
+ // For Aled, the test failed without own ~/.brooklyn/brooklyn.properties.
+ // Suspect that was caused by local environment, with custom brooklyn.ssh.config.scriptHeader
+ // to set things like correct Java on path.
+ return true;
+ }
+
+ @BeforeMethod(groups = "Integration")
+ public void setUp() throws Exception {
+ super.setUp();
+ warUrl = getClass().getClassLoader().getResource("hello-world.war");
+ loc = origManagementContext.getLocationManager().createLocation(LocationSpec.create(LocalhostMachineProvisioningLocation.class)
+ .configure("address", Networking.getLocalHost())
+ .configure(SshTool.PROP_TOOL_CLASS, RecordingSshjTool.class.getName()));
+ executor = Executors.newCachedThreadPool();
+
+ feedRegistration = BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_FEED_REGISTRATION_PROPERTY);
+ BrooklynFeatureEnablement.setEnablement(BrooklynFeatureEnablement.FEATURE_FEED_REGISTRATION_PROPERTY, true);
+ }
+
+ @AfterMethod(groups = "Integration", alwaysRun=true)
+ public void tearDown() throws Exception {
+ if (feedRegistration!=null)
+ BrooklynFeatureEnablement.setEnablement(BrooklynFeatureEnablement.FEATURE_FEED_REGISTRATION_PROPERTY, feedRegistration);
+
+ for (WebAppMonitor monitor : webAppMonitors) {
+ monitor.terminate();
+ }
--- End diff --
I'd include `webAppMonitors.clear()` after the for loop. But given `terminate` is idempotent (I hope!) then not necessary.
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r18993538
--- Diff: software/webapp/src/test/java/brooklyn/entity/proxy/nginx/NginxRebindWithHaIntegrationTest.java ---
@@ -0,0 +1,174 @@
+/*
+ * 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 brooklyn.entity.proxy.nginx;
+
+import java.net.URL;
+import java.util.Collection;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import brooklyn.entity.Feed;
+import brooklyn.entity.basic.Attributes;
+import brooklyn.entity.basic.Entities;
+import brooklyn.entity.basic.EntityInternal;
+import brooklyn.entity.basic.Lifecycle;
+import brooklyn.entity.group.DynamicCluster;
+import brooklyn.entity.proxying.EntitySpec;
+import brooklyn.entity.rebind.RebindTestFixtureWithApp;
+import brooklyn.entity.rebind.RebindTestUtils;
+import brooklyn.entity.webapp.tomcat.TomcatServer;
+import brooklyn.internal.BrooklynFeatureEnablement;
+import brooklyn.location.LocationSpec;
+import brooklyn.location.basic.LocalhostMachineProvisioningLocation;
+import brooklyn.location.basic.SshMachineLocationReuseIntegrationTest.RecordingSshjTool;
+import brooklyn.management.Task;
+import brooklyn.management.ha.HighAvailabilityMode;
+import brooklyn.management.internal.LocalManagementContext;
+import brooklyn.test.EntityTestUtils;
+import brooklyn.test.WebAppMonitor;
+import brooklyn.test.entity.TestApplication;
+import brooklyn.util.internal.ssh.SshTool;
+import brooklyn.util.net.Networking;
+import brooklyn.util.repeat.Repeater;
+import brooklyn.util.task.BasicExecutionManager;
+import brooklyn.util.time.Duration;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+
+/**
+ * Test the operation of the {@link NginxController} class.
+ */
+public class NginxRebindWithHaIntegrationTest extends RebindTestFixtureWithApp {
+
+ private static final Logger LOG = LoggerFactory.getLogger(NginxRebindWithHaIntegrationTest.class);
+
+ private URL warUrl;
+ private List<WebAppMonitor> webAppMonitors = new CopyOnWriteArrayList<WebAppMonitor>();
+ private ExecutorService executor;
+ private LocalhostMachineProvisioningLocation loc;
+
+ private Boolean feedRegistration;
+
+ @Override
+ protected boolean useLiveManagementContext() {
+ // For Aled, the test failed without own ~/.brooklyn/brooklyn.properties.
+ // Suspect that was caused by local environment, with custom brooklyn.ssh.config.scriptHeader
+ // to set things like correct Java on path.
+ return true;
+ }
+
+ @BeforeMethod(groups = "Integration")
+ public void setUp() throws Exception {
+ super.setUp();
+ warUrl = getClass().getClassLoader().getResource("hello-world.war");
+ loc = origManagementContext.getLocationManager().createLocation(LocationSpec.create(LocalhostMachineProvisioningLocation.class)
+ .configure("address", Networking.getLocalHost())
+ .configure(SshTool.PROP_TOOL_CLASS, RecordingSshjTool.class.getName()));
+ executor = Executors.newCachedThreadPool();
+
+ feedRegistration = BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_FEED_REGISTRATION_PROPERTY);
+ BrooklynFeatureEnablement.setEnablement(BrooklynFeatureEnablement.FEATURE_FEED_REGISTRATION_PROPERTY, true);
+ }
+
+ @AfterMethod(groups = "Integration", alwaysRun=true)
+ public void tearDown() throws Exception {
+ if (feedRegistration!=null)
+ BrooklynFeatureEnablement.setEnablement(BrooklynFeatureEnablement.FEATURE_FEED_REGISTRATION_PROPERTY, feedRegistration);
+
+ for (WebAppMonitor monitor : webAppMonitors) {
+ monitor.terminate();
+ }
+ if (executor != null) executor.shutdownNow();
+ super.tearDown();
+ RecordingSshjTool.reset();
--- End diff --
probably worth doing this in `finally` block. Otherwise a failure in this class' tearDown could break subsequent tests in jenkins in strange ways.
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#discussion_r18990800
--- Diff: core/src/main/java/brooklyn/util/task/BasicExecutionManager.java ---
@@ -559,21 +640,49 @@ protected void beforeStart(Map<?,?> flags, Task<?> task) {
ExecutionUtils.invoke(flags.get("newTaskStartCallback"), task);
}
- protected void afterEnd(Map<?,?> flags, Task<?> task) {
- activeTaskCount.decrementAndGet();
- incompleteTaskCount.decrementAndGet();
-
+ /** normally (if not interrupted) called once for each call to {@link #beforeSubmitScheduledTaskAllIterations(Map, Task)} */
+ protected void afterEndScheduledTaskAllIterations(Map<?,?> flags, Task<?> task) {
+ _afterEnd(flags, task, false, true);
+ }
+ /** called once for each call to {@link #beforeStartScheduledTaskSubmissionIteration(Map, Task)},
+ * with a per-iteration task generated by the surrounding scheduled task */
+ protected void afterEndScheduledTaskSubmissionIteration(Map<?,?> flags, Task<?> scheduledTask, Task<?> taskIteration) {
+ _afterEnd(flags, scheduledTask, true, false);
+ }
+ /** called once for each task on which {@link #beforeStartAtomicTask(Map, Task)} is invoked,
+ * and normally (if not interrupted prior to start)
+ * called once for each task on which {@link #beforeSubmitAtomicTask(Map, Task)} */
+ protected void afterEndAtomicTask(Map<?,?> flags, Task<?> task) {
+ _afterEnd(flags, task, true, true);
+ }
+ /** normally (if not interrupted) called once for each call to {@link #beforeSubmit(Map, Task)},
+ * and, for atomic tasks and scheduled-task submission iterations where
+ * always called once if {@link #beforeStart(Map, Task)} is invoked and in the same thread as that method */
+ protected void _afterEnd(Map<?,?> flags, Task<?> task, boolean startedInThisThread, boolean isEndingAllIterations) {
--- End diff --
Personal preference: not a fan of starting methods with "_". But not sure what a better name would be.
---
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] incubator-brooklyn pull request: Tasks stop on unmanaged
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/252#issuecomment-59493099
thanks @aledsage - all addressed, will run tests then merge
---
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.
---