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 2016/01/19 13:49:22 UTC
[GitHub] incubator-brooklyn pull request: Tasks cancellation
GitHub user ahgittin opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/1160
Tasks cancellation
clearer semantics for cancel, and cancel dependent submitted tasks by default
prevents leaks where dependent tasks (eg resolveValue) are submitted in the background,
then the caller is cancelled; the interruption was not propagated. now by default it is,
to children tasks and to submitted transients, with options for other (weaker and stronger) cancellations.
see TaskInternal.cancel(TaskCancellationOptions), and new tests for cancelling children in DynamicSequentialTaskTest.
also remove deprecated loose-typing ExecutionUtils.invoke,
more TRACE logging for activities,
and wrap batch config in a task so more resolutions are nested
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahgittin/incubator-brooklyn tasks-cancellation
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/1160.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 #1160
----
commit 87ad37ae46644886adb7d99848a4802ee1559000
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2016-01-19T12:48:15Z
clearer semantics for cancel, and cancel dependent submitted tasks by default
prevents leaks where dependent tasks (eg resolveValue) are submitted in the background,
then the caller is cancelled; the interruption was not propagated. now by default it is,
to children tasks and to submitted transients, with options for other (weaker and stronger) cancellations.
see TaskInternal.cancel(TaskCancellationOptions), and new tests for cancelling children in DynamicSequentialTaskTest.
also remove deprecated loose-typing ExecutionUtils.invoke,
more TRACE logging for activities,
and wrap batch config in a task so more resolutions are nested
----
---
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 cancellation
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/1160#discussion_r50317800
--- Diff: brooklyn-server/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java ---
@@ -526,19 +538,63 @@ public String toString() {
}
}
- private final static class ListenableForwardingFutureForTask<T> extends ListenableForwardingFuture<T> {
+ @SuppressWarnings("deprecation")
+ // TODO do we even need a listenable future here? possibly if someone wants to interrogate the future it might
+ // be interesting, so possibly it is useful that we implement ListenableFuture...
+ private final static class CancellingListenableForwardingFutureForTask<T> extends ListenableForwardingFuture<T> {
private final Task<T> task;
+ private BasicExecutionManager execMgmt;
- private ListenableForwardingFutureForTask(Future<T> delegate, ExecutionList list, Task<T> task) {
+ private CancellingListenableForwardingFutureForTask(BasicExecutionManager execMgmt, Future<T> delegate, ExecutionList list, Task<T> task) {
super(delegate, list);
+ this.execMgmt = execMgmt;
this.task = task;
}
@Override
- public boolean cancel(boolean mayInterruptIfRunning) {
+ public boolean cancel(TaskCancellationMode mode) {
boolean result = false;
- if (!task.isCancelled()) result |= task.cancel(mayInterruptIfRunning);
- result |= super.cancel(mayInterruptIfRunning);
+ if (log.isTraceEnabled()) {
+ log.trace("CLFFT cancelling "+task+" mode "+mode);
+ }
+ if (!task.isCancelled()) result |= ((TaskInternal<T>)task).cancel(mode);
+ result |= delegate().cancel(mode.isAllowedToInterruptTask());
+
+ if (mode.isAllowedToInterruptAllSubmittedTasks() || mode.isAllowedToInterruptDependentSubmittedTasks()) {
+ int subtasksFound=0;
+ int subtasksReallyCancelled=0;
+
+ if (task instanceof HasTaskChildren) {
+ for (Task<?> child: ((HasTaskChildren)task).getChildren()) {
+ if (log.isTraceEnabled()) {
+ log.trace("Cancelling "+child+" on recursive cancellation of "+task);
+ }
+ subtasksFound++;
+ if (((TaskInternal<?>)child).cancel(mode)) {
+ result = true;
+ subtasksReallyCancelled++;
+ }
+ }
+ }
+ for (Task<?> t: execMgmt.getAllTasks()) {
--- End diff --
have added a TODO -- i agree with your intuition, but would rather work on optimising tasks as a separate exercise
---
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 cancellation
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/1160#discussion_r50284330
--- Diff: brooklyn-server/core/src/test/java/org/apache/brooklyn/util/core/task/DynamicSequentialTaskTest.java ---
@@ -207,6 +222,85 @@ public void testCancelled() throws Exception {
// but we do _not_ get a mutex from task3 as it does not run (is not interrupted)
Assert.assertEquals(cancellations.availablePermits(), 0);
}
+
+ @Test
+ public void testCancellationModeAndSubmitted() throws Exception {
+ doTestCancellationModeAndSubmitted(true, TaskCancellationMode.DO_NOT_INTERRUPT, false, false);
+
+ doTestCancellationModeAndSubmitted(true, TaskCancellationMode.INTERRUPT_TASK_AND_ALL_SUBMITTED_TASKS, true, true);
+ doTestCancellationModeAndSubmitted(true, TaskCancellationMode.INTERRUPT_TASK_AND_DEPENDENT_SUBMITTED_TASKS, true, true);
+ doTestCancellationModeAndSubmitted(true, TaskCancellationMode.INTERRUPT_TASK_BUT_NOT_SUBMITTED_TASKS, true, false);
+
+ // if it's not transient, it should only be cancelled on "all submitted"
+ doTestCancellationModeAndSubmitted(false, TaskCancellationMode.INTERRUPT_TASK_AND_DEPENDENT_SUBMITTED_TASKS, true, false);
+ doTestCancellationModeAndSubmitted(false, TaskCancellationMode.INTERRUPT_TASK_AND_ALL_SUBMITTED_TASKS, true, true);
+
+ // cancellation mode left off should be the same as TASK_AND_DEPENDENT, i.e. don't cancel non-transient bg submitted
+ doTestCancellationModeAndSubmitted(true, null, true, true);
+ doTestCancellationModeAndSubmitted(false, null, true, false);
+ // and 'true' should be the same
+ doTestCancellationModeAndSubmitted(true, true, true, true);
+ doTestCancellationModeAndSubmitted(false, true, true, false);
+
+ // cancellation mode false should be the same as DO_NOT_INTERRUPT
+ doTestCancellationModeAndSubmitted(true, false, false, false);
+ }
+
+ public void doTestCancellationModeAndSubmitted(
+ boolean isSubtaskTransient,
+ Object cancellationMode,
+ boolean expectedTaskInterrupted,
+ boolean expectedSubtaskCancelled
+ ) throws Exception {
+ tearDown(); setUp();
+
+ final Task<String> t1 = sayTask("1-wait", Duration.minutes(10), "1-done");
+ if (isSubtaskTransient) {
+ BrooklynTaskTags.addTagDynamically(t1, BrooklynTaskTags.TRANSIENT_TASK_TAG);
+ }
+
+ final Task<List<?>> t = Tasks.parallel(
+ submitting(t1),
+ sayTask("2-wait", Duration.minutes(10), "2-done"));
+ ec.submit(t);
+
+ waitForMessages(Predicates.compose(MathPredicates.greaterThanOrEqual(2), CollectionFunctionals.sizeFunction()), TIMEOUT);
+ Asserts.assertEquals(MutableSet.copyOf(messages), MutableSet.of("1-wait", "2-wait"));
+
+ Time.sleep(Duration.millis(400));
--- End diff --
Why do we need the sleep? I thought that was what your assert for the messages was for (that the task was executing)?
---
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 cancellation
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/1160#discussion_r50317829
--- Diff: brooklyn-server/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java ---
@@ -665,9 +728,27 @@ protected void internalBeforeStart(Map<?,?> flags, Task<?> task) {
PerThreadCurrentTaskHolder.perThreadCurrentTask.set(task);
((TaskInternal<?>)task).setStartTimeUtc(System.currentTimeMillis());
}
- ExecutionUtils.invoke(flags.get("newTaskStartCallback"), task);
+ invokeCallback(flags.get("newTaskStartCallback"), task);
}
+ @SuppressWarnings({ "unchecked", "rawtypes" })
+ // not ideal, such loose typing on the callback -- should prefer Function<Task,Object>
+ // but at least it's package-private
+ static Object invokeCallback(Object callable, Task<?> task) {
+ if (callable instanceof Closure) return ((Closure<?>)callable).call(task);
+ if (callable instanceof Callable) {
+ try {
+ return ((Callable<?>)callable).call();
+ } catch (Throwable t) {
+ throw Throwables.propagate(t);
--- End diff --
good spot -- this code was moved but worth improving
---
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 cancellation
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/1160#discussion_r50282704
--- Diff: brooklyn-server/core/src/main/java/org/apache/brooklyn/util/core/task/BasicTask.java ---
@@ -288,15 +288,33 @@ public synchronized boolean uncancel() {
}
@Override
- public synchronized boolean cancel(boolean mayInterruptIfRunning) {
+ public final synchronized boolean cancel(boolean mayInterruptIfRunning) {
+ // semantics changed in 2016-01, previously "true" was INTERRUPT_TASK_BUT_NOT_SUBMITTED_TASKS
+ return cancel(mayInterruptIfRunning ? TaskCancellationMode.INTERRUPT_TASK_AND_DEPENDENT_SUBMITTED_TASKS
+ : TaskCancellationMode.DO_NOT_INTERRUPT);
+ }
+
+ public synchronized boolean cancel(TaskCancellationMode mode) {
--- End diff --
Let's mark this `@Beta` (note that `TaskCancellationMode` is already `@Beta`).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Tasks cancellation
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/1160#discussion_r50282211
--- Diff: brooklyn-server/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java ---
@@ -665,9 +728,27 @@ protected void internalBeforeStart(Map<?,?> flags, Task<?> task) {
PerThreadCurrentTaskHolder.perThreadCurrentTask.set(task);
((TaskInternal<?>)task).setStartTimeUtc(System.currentTimeMillis());
}
- ExecutionUtils.invoke(flags.get("newTaskStartCallback"), task);
+ invokeCallback(flags.get("newTaskStartCallback"), task);
}
+ @SuppressWarnings({ "unchecked", "rawtypes" })
+ // not ideal, such loose typing on the callback -- should prefer Function<Task,Object>
+ // but at least it's package-private
+ static Object invokeCallback(Object callable, Task<?> task) {
+ if (callable instanceof Closure) return ((Closure<?>)callable).call(task);
+ if (callable instanceof Callable) {
+ try {
+ return ((Callable<?>)callable).call();
+ } catch (Throwable t) {
+ throw Throwables.propagate(t);
--- End diff --
Should use `Exceptions.propagate(t)`, so we properly handle `InterruptedException`.
---
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 cancellation
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1160#issuecomment-173357726
Thanks @aledsage -- all good comments, all addressed. Merging.
---
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 cancellation
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/1160#discussion_r50317896
--- Diff: brooklyn-server/core/src/main/java/org/apache/brooklyn/util/core/task/BasicTask.java ---
@@ -288,15 +288,33 @@ public synchronized boolean uncancel() {
}
@Override
- public synchronized boolean cancel(boolean mayInterruptIfRunning) {
+ public final synchronized boolean cancel(boolean mayInterruptIfRunning) {
+ // semantics changed in 2016-01, previously "true" was INTERRUPT_TASK_BUT_NOT_SUBMITTED_TASKS
+ return cancel(mayInterruptIfRunning ? TaskCancellationMode.INTERRUPT_TASK_AND_DEPENDENT_SUBMITTED_TASKS
+ : TaskCancellationMode.DO_NOT_INTERRUPT);
+ }
+
+ public synchronized boolean cancel(TaskCancellationMode mode) {
--- End diff --
the super is marked `@Beta` -- have added `@Override @Beta`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Tasks cancellation
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1160#issuecomment-173278396
@ahgittin looks good - only very minor comments.
Happy for you to merge once you've looked over those.
---
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 cancellation
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/1160
---
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 cancellation
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/1160#discussion_r50281559
--- Diff: brooklyn-server/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java ---
@@ -526,19 +538,63 @@ public String toString() {
}
}
- private final static class ListenableForwardingFutureForTask<T> extends ListenableForwardingFuture<T> {
+ @SuppressWarnings("deprecation")
+ // TODO do we even need a listenable future here? possibly if someone wants to interrogate the future it might
+ // be interesting, so possibly it is useful that we implement ListenableFuture...
+ private final static class CancellingListenableForwardingFutureForTask<T> extends ListenableForwardingFuture<T> {
private final Task<T> task;
+ private BasicExecutionManager execMgmt;
- private ListenableForwardingFutureForTask(Future<T> delegate, ExecutionList list, Task<T> task) {
+ private CancellingListenableForwardingFutureForTask(BasicExecutionManager execMgmt, Future<T> delegate, ExecutionList list, Task<T> task) {
super(delegate, list);
+ this.execMgmt = execMgmt;
this.task = task;
}
@Override
- public boolean cancel(boolean mayInterruptIfRunning) {
+ public boolean cancel(TaskCancellationMode mode) {
boolean result = false;
- if (!task.isCancelled()) result |= task.cancel(mayInterruptIfRunning);
- result |= super.cancel(mayInterruptIfRunning);
+ if (log.isTraceEnabled()) {
+ log.trace("CLFFT cancelling "+task+" mode "+mode);
+ }
+ if (!task.isCancelled()) result |= ((TaskInternal<T>)task).cancel(mode);
+ result |= delegate().cancel(mode.isAllowedToInterruptTask());
+
+ if (mode.isAllowedToInterruptAllSubmittedTasks() || mode.isAllowedToInterruptDependentSubmittedTasks()) {
+ int subtasksFound=0;
+ int subtasksReallyCancelled=0;
+
+ if (task instanceof HasTaskChildren) {
+ for (Task<?> child: ((HasTaskChildren)task).getChildren()) {
+ if (log.isTraceEnabled()) {
+ log.trace("Cancelling "+child+" on recursive cancellation of "+task);
+ }
+ subtasksFound++;
+ if (((TaskInternal<?>)child).cancel(mode)) {
+ result = true;
+ subtasksReallyCancelled++;
+ }
+ }
+ }
+ for (Task<?> t: execMgmt.getAllTasks()) {
--- End diff --
This feels like it could get very inefficient - perhaps would be good to index the tasks by "submitted by", so can retrieve them easily.
---
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 cancellation
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/1160#discussion_r50318137
--- Diff: brooklyn-server/core/src/test/java/org/apache/brooklyn/util/core/task/DynamicSequentialTaskTest.java ---
@@ -207,6 +222,85 @@ public void testCancelled() throws Exception {
// but we do _not_ get a mutex from task3 as it does not run (is not interrupted)
Assert.assertEquals(cancellations.availablePermits(), 0);
}
+
+ @Test
+ public void testCancellationModeAndSubmitted() throws Exception {
+ doTestCancellationModeAndSubmitted(true, TaskCancellationMode.DO_NOT_INTERRUPT, false, false);
+
+ doTestCancellationModeAndSubmitted(true, TaskCancellationMode.INTERRUPT_TASK_AND_ALL_SUBMITTED_TASKS, true, true);
+ doTestCancellationModeAndSubmitted(true, TaskCancellationMode.INTERRUPT_TASK_AND_DEPENDENT_SUBMITTED_TASKS, true, true);
+ doTestCancellationModeAndSubmitted(true, TaskCancellationMode.INTERRUPT_TASK_BUT_NOT_SUBMITTED_TASKS, true, false);
+
+ // if it's not transient, it should only be cancelled on "all submitted"
+ doTestCancellationModeAndSubmitted(false, TaskCancellationMode.INTERRUPT_TASK_AND_DEPENDENT_SUBMITTED_TASKS, true, false);
+ doTestCancellationModeAndSubmitted(false, TaskCancellationMode.INTERRUPT_TASK_AND_ALL_SUBMITTED_TASKS, true, true);
+
+ // cancellation mode left off should be the same as TASK_AND_DEPENDENT, i.e. don't cancel non-transient bg submitted
+ doTestCancellationModeAndSubmitted(true, null, true, true);
+ doTestCancellationModeAndSubmitted(false, null, true, false);
+ // and 'true' should be the same
+ doTestCancellationModeAndSubmitted(true, true, true, true);
+ doTestCancellationModeAndSubmitted(false, true, true, false);
+
+ // cancellation mode false should be the same as DO_NOT_INTERRUPT
+ doTestCancellationModeAndSubmitted(true, false, false, false);
+ }
+
+ public void doTestCancellationModeAndSubmitted(
+ boolean isSubtaskTransient,
+ Object cancellationMode,
+ boolean expectedTaskInterrupted,
+ boolean expectedSubtaskCancelled
+ ) throws Exception {
+ tearDown(); setUp();
+
+ final Task<String> t1 = sayTask("1-wait", Duration.minutes(10), "1-done");
+ if (isSubtaskTransient) {
+ BrooklynTaskTags.addTagDynamically(t1, BrooklynTaskTags.TRANSIENT_TASK_TAG);
+ }
+
+ final Task<List<?>> t = Tasks.parallel(
+ submitting(t1),
+ sayTask("2-wait", Duration.minutes(10), "2-done"));
+ ec.submit(t);
+
+ waitForMessages(Predicates.compose(MathPredicates.greaterThanOrEqual(2), CollectionFunctionals.sizeFunction()), TIMEOUT);
+ Asserts.assertEquals(MutableSet.copyOf(messages), MutableSet.of("1-wait", "2-wait"));
+
+ Time.sleep(Duration.millis(400));
--- End diff --
you're right, this shouldn't have been there
---
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 cancellation
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/1160#discussion_r50284105
--- Diff: brooklyn-server/core/src/test/java/org/apache/brooklyn/util/core/task/DynamicSequentialTaskTest.java ---
@@ -207,6 +222,85 @@ public void testCancelled() throws Exception {
// but we do _not_ get a mutex from task3 as it does not run (is not interrupted)
Assert.assertEquals(cancellations.availablePermits(), 0);
}
+
+ @Test
+ public void testCancellationModeAndSubmitted() throws Exception {
+ doTestCancellationModeAndSubmitted(true, TaskCancellationMode.DO_NOT_INTERRUPT, false, false);
+
+ doTestCancellationModeAndSubmitted(true, TaskCancellationMode.INTERRUPT_TASK_AND_ALL_SUBMITTED_TASKS, true, true);
+ doTestCancellationModeAndSubmitted(true, TaskCancellationMode.INTERRUPT_TASK_AND_DEPENDENT_SUBMITTED_TASKS, true, true);
+ doTestCancellationModeAndSubmitted(true, TaskCancellationMode.INTERRUPT_TASK_BUT_NOT_SUBMITTED_TASKS, true, false);
+
+ // if it's not transient, it should only be cancelled on "all submitted"
+ doTestCancellationModeAndSubmitted(false, TaskCancellationMode.INTERRUPT_TASK_AND_DEPENDENT_SUBMITTED_TASKS, true, false);
+ doTestCancellationModeAndSubmitted(false, TaskCancellationMode.INTERRUPT_TASK_AND_ALL_SUBMITTED_TASKS, true, true);
+
+ // cancellation mode left off should be the same as TASK_AND_DEPENDENT, i.e. don't cancel non-transient bg submitted
+ doTestCancellationModeAndSubmitted(true, null, true, true);
+ doTestCancellationModeAndSubmitted(false, null, true, false);
+ // and 'true' should be the same
+ doTestCancellationModeAndSubmitted(true, true, true, true);
+ doTestCancellationModeAndSubmitted(false, true, true, false);
+
+ // cancellation mode false should be the same as DO_NOT_INTERRUPT
+ doTestCancellationModeAndSubmitted(true, false, false, false);
+ }
+
+ public void doTestCancellationModeAndSubmitted(
+ boolean isSubtaskTransient,
+ Object cancellationMode,
+ boolean expectedTaskInterrupted,
+ boolean expectedSubtaskCancelled
+ ) throws Exception {
+ tearDown(); setUp();
+
+ final Task<String> t1 = sayTask("1-wait", Duration.minutes(10), "1-done");
+ if (isSubtaskTransient) {
+ BrooklynTaskTags.addTagDynamically(t1, BrooklynTaskTags.TRANSIENT_TASK_TAG);
+ }
+
+ final Task<List<?>> t = Tasks.parallel(
+ submitting(t1),
+ sayTask("2-wait", Duration.minutes(10), "2-done"));
+ ec.submit(t);
+
+ waitForMessages(Predicates.compose(MathPredicates.greaterThanOrEqual(2), CollectionFunctionals.sizeFunction()), TIMEOUT);
+ Asserts.assertEquals(MutableSet.copyOf(messages), MutableSet.of("1-wait", "2-wait"));
+
+ Time.sleep(Duration.millis(400));
+
+ if (cancellationMode==null) {
+ ((TaskInternal<?>)t).cancel();
+ } else if (cancellationMode instanceof Boolean) {
+ t.cancel((Boolean)cancellationMode);
+ } else if (cancellationMode instanceof TaskCancellationMode) {
--- End diff --
Include an:
} else {
throw new IllegalStateException("Invalid cancellationMode: "+cancellationMode);
}
(even though it's a test, simple to add and makes clear to readers/extenders that we don't expect anything else.)
---
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 cancellation
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/1160#discussion_r50283167
--- Diff: brooklyn-server/core/src/main/java/org/apache/brooklyn/util/core/task/DynamicSequentialTask.java ---
@@ -309,7 +326,7 @@ public T call() throws Exception {
}
if (!primaryFinished && failureHandlingConfig.cancelPrimaryOnSecondaryFailure) {
- cancel(true, false, false);
+ cancel(TaskCancellationMode.INTERRUPT_TASK_BUT_NOT_SUBMITTED_TASKS, false);
--- End diff --
Wow, this code is winning a prize for one of the biggest indents, which is a bit of a code smell!
But nothing to do with this PR, so ignore here.
---
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.
---