You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by sjcorbett <gi...@git.apache.org> on 2015/11/18 15:48:48 UTC
[GitHub] incubator-brooklyn pull request: Allows ScheduledTasks to be resub...
GitHub user sjcorbett opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/1043
Allows ScheduledTasks to be resubmitted even when the task throws an exception.
I had to spend some time last week debugging an entity that was mysteriously on fire even though its driver's `isRunning` method returned true (verified via Groovy console).
After some log searching I realised that roughly the following scenario had transpired:
* Entity running
* Network connectivity began affecting Brooklyn VM. Several entities were on fire.
* One entity's SoftwareProcessImpl.serviceProcessIsRunning feed SSHed to its VM. The SSH task threw an exception because of the network issue.
* The entity's serviceProcessIsRunning feed was cancelled.
* The network problem finished. The entity's status could not recover.
This PR adds a flag to `ScheduledTask` to indicate that its task should be resubmitted even after throwing an exception. The `Poller` class has been configured use this flag.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sjcorbett/incubator-brooklyn scheduled-poller
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/1043.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 #1043
----
commit 4ece7feff521ad260286f6ef7a9f48202ab0df2f
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-11-18T12:46:32Z
Trivial doc fix
commit ba313ec4fc720e7a704e25bdc6b67aa21f36d240
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-11-18T12:46:56Z
Closing ] in jquery selector
commit abe1bf3e51f73a2896be13a1e88a66bab36bfd99
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-11-18T12:47:44Z
Adds cancelOnException to ScheduledTask
commit c0da7603fb8e9580fb339a742d9e855bf619a840
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-11-18T14:36:32Z
Pollers set cancelOnException=false when scheduling tasks
----
---
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: Allows ScheduledTasks to be resub...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1043#discussion_r45329630
--- Diff: usage/jsgui/src/test/java/org/apache/brooklyn/rest/jsgui/BrooklynJavascriptGuiLauncher.java ---
@@ -49,7 +49,7 @@
public static void main(String[] args) throws Exception {
// NOTE: When running Brooklyn from an IDE (i.e. by launching BrooklynJavascriptGuiLauncher.main())
// you will need to ensure that the working directory is set to the jsgui folder. For IntelliJ,
- // set the 'Working directory' of the Run/Debug Configuration to $MODULE_DIR/../jsgui.
+ // set the 'Working directory' of the Run/Debug Configuration to $MODULE_DIR$/../jsgui.
--- End diff --
It's an IntelliJ specific thing: https://www.jetbrains.com/idea/help/path-variables.html
---
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: Allows ScheduledTasks to be resub...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/1043
---
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: Allows ScheduledTasks to be resub...
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/1043#discussion_r45330844
--- Diff: core/src/main/java/org/apache/brooklyn/util/core/task/ScheduledTask.java ---
@@ -121,13 +145,17 @@ public ScheduledTask maxIterations(int val) {
protected String getActiveTaskStatusString(int verbosity) {
StringBuilder rv = new StringBuilder("Scheduler");
- if (runCount>0) rv.append(", iteration "+(runCount+1));
- if (recentRun!=null) rv.append(", last run "+
- Duration.sinceUtc(recentRun.getStartTimeUtc())+" ms ago");
+ if (runCount > 0) {
+ rv.append(", iteration ").append(runCount + 1);
+ }
+ if (recentRun != null) {
+ Duration start = Duration.sinceUtc(recentRun.getStartTimeUtc());
+ rv.append(", last run ").append(start).append(" ms ago");
--- End diff --
The toString of the duration will include the units, so remove the "ms"
---
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: Allows ScheduledTasks to be resub...
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/1043#discussion_r45329049
--- Diff: usage/jsgui/src/test/java/org/apache/brooklyn/rest/jsgui/BrooklynJavascriptGuiLauncher.java ---
@@ -49,7 +49,7 @@
public static void main(String[] args) throws Exception {
// NOTE: When running Brooklyn from an IDE (i.e. by launching BrooklynJavascriptGuiLauncher.main())
// you will need to ensure that the working directory is set to the jsgui folder. For IntelliJ,
- // set the 'Working directory' of the Run/Debug Configuration to $MODULE_DIR/../jsgui.
+ // set the 'Working directory' of the Run/Debug Configuration to $MODULE_DIR$/../jsgui.
--- End diff --
Not sure I follow why you make this change. The previous syntax in the comment was assuming that MODULE_DIR was an environment variable, so why surround it with a `$` on each side?
But I don't care :-)
---
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: Allows ScheduledTasks to be resub...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1043#discussion_r45343190
--- Diff: core/src/main/java/org/apache/brooklyn/util/core/task/ScheduledTask.java ---
@@ -84,12 +99,15 @@ public ScheduledTask(Map flags, Callable<Task<?>> taskFactory) {
delay = Duration.of(elvis(flags.remove("delay"), 0));
period = Duration.of(elvis(flags.remove("period"), null));
maxIterations = elvis(flags.remove("maxIterations"), null);
+ Object cancelFlag = flags.remove("cancelOnException");
+ cancelOnException = cancelFlag == null || Boolean.TRUE.equals(cancelFlag);
--- End diff --
Yes, I assume callers are going to do the right thing. They also have the option of calling `cancelOnException(boolean)` if they like the strongly-typed side of life.
---
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: Allows ScheduledTasks to be resub...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1043#issuecomment-158038315
@sjcorbett looks really good! Only some very minor comments. Please merge once you've looked at those, and decided whether to incorporate/ignore.
---
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: Allows ScheduledTasks to be resub...
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/1043#discussion_r45330704
--- Diff: core/src/main/java/org/apache/brooklyn/util/core/task/ScheduledTask.java ---
@@ -84,12 +99,15 @@ public ScheduledTask(Map flags, Callable<Task<?>> taskFactory) {
delay = Duration.of(elvis(flags.remove("delay"), 0));
period = Duration.of(elvis(flags.remove("period"), null));
maxIterations = elvis(flags.remove("maxIterations"), null);
+ Object cancelFlag = flags.remove("cancelOnException");
+ cancelOnException = cancelFlag == null || Boolean.TRUE.equals(cancelFlag);
--- End diff --
Should we do a type coercion? No strong feelings. This deep in the code, we can assume folk are using the right types.
---
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: Allows ScheduledTasks to be resub...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1043#issuecomment-158071772
@aledsage Thanks for your comments. Will 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.
---