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/07/24 02:07:31 UTC
[GitHub] incubator-brooklyn pull request: Misc errors when errors happen
GitHub user ahgittin opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/91
Misc errors when errors happen
When things go really wrong, a lot more things also go wrong.
(NB this builds on #83 at present, so merge that first.)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahgittin/incubator-brooklyn misc-errors-when-errors-happen
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/91.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 #91
----
commit a070492a20505e2b2671e9a0b229ba6e47063f97
Author: bmwshop <bm...@gmail.com>
Date: 2014-06-23T18:01:00Z
deploy / undeploy / update on the ControlledDynamicWebAppCluster
commit 36423a546e07da32fb1e421731747afdc1600321
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-07-18T19:38:37Z
tidy initial pull request from @bmwshop, as per code review, plus some todo comments
commit e58bdba72312de47c4cdbb7d8b925b1d4df711d8
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-07-18T19:58:16Z
move deploy and redeploy bodies to DynamicWebAppCluster, with ControlledDWAC calling to that,
and with new interfaces for the shared items (deploy effectors etc) so that ControlledDWAC is not a JavaWebAppSoftwareProcess
commit 6c2a3932e6fb9df4599a93c96f1f9c22a0bbf14b
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-07-18T21:32:59Z
better tasks logic for deploying wars when target entity is ready (ie service up), failing gracefully if it's not, with some helpers in Tasks and DynamicTasks
commit 2db8fd95031d0b9c6ba9e9c45b314ccc480e1f4a
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-07-18T22:14:12Z
squash code warnings, clarify user-facing messages, fix redeployAll bug
commit ab7e3d0ff42f3c5baae04fcfff31fdfa7d6dd155
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-07-22T00:33:51Z
have at-entity tasks show up as children (in dynamic queueing context, though still submitted externally which means they will not be run as part of queue), and allow them to have children: improves traceability in GUI when effector methods call other effector methods, they show up as children rather than background tasks (or worse, not even as background tasks); and support setting flags in TaskBuilder
commit 2e124ca0c6ee89709dcf4f153c2e6f87ab3b5a66
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-07-22T00:36:53Z
parallel task should not fail if inessential child fails
commit 47488483167f9de09b014e3933b7c7b24ccc4895
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-07-22T00:37:23Z
improve display of task results, catch NPE when switching task contexts, and ensure data is cleared (nulled) for old map entries due to backbone bug (without this, sometimes blockingTask might show up as `TYPEOF undefined` when you inspect its JSON)
commit 38c15da06b718ac09b03058f6fc3058dfff90729
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-07-22T00:38:49Z
include blocking details when waiting for service up to invoke an effector on a child
commit 880c27a217b185edf2f358f6c81a03687854f6a3
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-07-22T20:18:41Z
effector invocations had been somewhat inconsistent in what failed -- the method call, the surrounding context -- this ensures that a programmatic call to invoke an effector will throw if it fails, that the effector task is added to any parent context, but as an inessential task so it does not fail the parent context. new tests for this are in BasicEffectorTest. the effect is particularly magnified when we attempt to add programmatically driven effector calls to the dynamic task context (so they show up in the gui). these are now marked inessential, which i think is right because the user is interacting programmatically, and as the caller they would typically get on the result and handle errors themselves.
this largely preserves existing behaviour, apart from with the exception that effector method calls inside other effectors could previously fail without throwing, and now they throw.
this mainly affected dynamic cluster which in some places relied on failures being silently ignored, e.g. in the resize effector call. this effector now reliably fails if the cluster does not resize. previously it would depend whether it was in a task or not. to preserve compatibility in that class as much as possible, some interface methods (non-effector) have had their signature changed. it was not clear why these were on the interface, so i have deprecated them there also.
commit cc871b5240bae7fcd3698e990a0a7e2490ac3a6a
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-07-22T20:23:31Z
map target name to right format on cluster-level undeploy
commit 9e5aaffbcfc0cccda862c76893899b8019fb0bcd
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-07-23T20:05:22Z
fix link error
commit 07a136e606c14d75cd73912cea17caa1ba82f980
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-07-23T20:05:57Z
don't block rebind just because a group can't rescan his elements; it may be referencing an entity which has gone away
commit 615738e041efa367f254c2e2921bf9824f749e46
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-07-23T20:06:25Z
fix memory leak where peeked-locations are kept by their parents
commit b33d76054b4cc857726411a409d652889d3d5fdb
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-07-23T20:21:16Z
provide some detail and better tostrings when tasks are backing up
commit 5bd99975d581cf9c86fd374d6bc939a1a339683b
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-07-23T22:24:19Z
give better error when ssh key with passphrase is used
commit a0c3dcbfcffca28d072de8e0993e79f00b7fe4a1
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-07-23T23:56:14Z
prevent NPE due to applicationName being null
commit 2952ddf4e7175d4856f2252b96b1fd25e944a86d
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-07-23T23:56:24Z
unmanage and stop rebind on failure, before publishing the new state, and cancel tasks on demotion
----
---
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: Misc errors when errors happen
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/91#issuecomment-50525757
rebased, and add one OSX test fix. 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: Misc errors when errors happen
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/91#issuecomment-50478629
Have reviewed "prevent NPE due to applicationName being null" and the subsequent 3 commits.
---
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: Misc errors when errors happen
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/91#discussion_r15544874
--- Diff: core/src/main/java/brooklyn/util/task/DynamicTasks.java ---
@@ -159,6 +159,11 @@ private boolean orSubmitInternal() {
public void andWaitForSuccess() {
task.getUnchecked();
}
+ public void orCancel() {
--- End diff --
it's usually terminal, normally there's nothing someone would do thereafter; if people are needing to inspect it we could change the api to return that, but most of the time i think they won't, so returning something would encourage the wrong use of the fluent api
---
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: Misc errors when errors happen
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/91#discussion_r15523372
--- Diff: core/src/main/java/brooklyn/util/task/DynamicTasks.java ---
@@ -159,6 +159,11 @@ private boolean orSubmitInternal() {
public void andWaitForSuccess() {
--- End diff --
Why doesn't `andWaitForSuccess()` return the result?
---
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: Misc errors when errors happen
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/91#discussion_r15544799
--- Diff: core/src/main/java/brooklyn/util/task/DynamicTasks.java ---
@@ -159,6 +159,11 @@ private boolean orSubmitInternal() {
public void andWaitForSuccess() {
--- End diff --
makes sense
---
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: Misc errors when errors happen
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/91#discussion_r15544945
--- Diff: core/src/main/java/brooklyn/util/task/ssh/SshTasks.java ---
@@ -130,9 +131,24 @@ public static SshFetchTaskFactory newSshFetchTaskFactory(SshMachineLocation mach
return result;
}
+ @Beta
+ public static enum OnFailingTask { FAIL, WARN_OR_FAIL_INESSENTIAL_IF_DYNAMIC, WARN_IN_LOG_ONLY, IGNORE }
--- End diff --
how about `WARN_OR_IF_DYNAMIC_THEN_FAIL_MARKING_INESSENTIAL` ? agree javadoc. agree, would rather a better semantics.
---
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: Misc errors when errors happen
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/91#discussion_r15523257
--- Diff: core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java ---
@@ -516,10 +522,41 @@ protected void demoteToStandby() {
}
nodeState = ManagementNodeState.STANDBY;
+ onDemotion();
+ publishDemotionFromMaster(false);
+ }
+
+ protected void onDemotion() {
managementContext.getRebindManager().stop();
for (Application app: managementContext.getApplications())
Entities.unmanage(app);
- publishDemotionFromMaster(false);
+ // let's try forcibly interrupting tasks on managed entities
+ Collection<Exception> exceptions = MutableSet.of();
+ int tasks = 0;
+ LOG.debug("Cancelling tasks on demotion");
+ try {
+ for (Entity entity: managementContext.getEntityManager().getEntities()) {
+ for (Task<?> t: managementContext.getExecutionContext(entity).getTasks()) {
+ if (!t.isDone()) {
+ tasks++;
+ try {
+ LOG.debug("Cancelling "+t+" on "+entity);
+ t.cancel(true);
+ } catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
+ exceptions.add(e);
+ }
+ }
+ }
+ }
+ } catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
+ LOG.warn("Error inspecting tasks to cancel on demotion: "+e, e);
+ }
+ if (!exceptions.isEmpty())
+ LOG.warn("Error when cancelling tasks on demotion: "+Exceptions.create(exceptions));
--- End diff --
The exception's stacktrace will never be logged. We need to log all stacktraces at least at debug, at least (and ideally) once. Could do that in the try-catch above.
---
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: Misc errors when errors happen
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/91#discussion_r15523548
--- Diff: core/src/main/java/brooklyn/util/task/ssh/SshTasks.java ---
@@ -130,9 +131,24 @@ public static SshFetchTaskFactory newSshFetchTaskFactory(SshMachineLocation mach
return result;
}
+ @Beta
+ public static enum OnFailingTask { FAIL, WARN_OR_FAIL_INESSENTIAL_IF_DYNAMIC, WARN_IN_LOG_ONLY, IGNORE }
--- End diff --
Would prefer a blank line after this enum.
Also `WARN_OR_FAIL_INESSENTIAL_IF_DYNAMIC` is a hard-to-understand name. It wasn't obvious where the grouping is: i.e. WARN IF NOT INESSENTIAL OR NOT DYNAMIC; FAIL IF INESSENTIAL AND IF DYNAMIC. At least needs javadoc.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Misc errors when errors happen
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/91#discussion_r15523396
--- Diff: core/src/main/java/brooklyn/util/task/DynamicTasks.java ---
@@ -159,6 +159,11 @@ private boolean orSubmitInternal() {
public void andWaitForSuccess() {
task.getUnchecked();
}
+ public void orCancel() {
--- End diff --
Why not return the `TaskQueueingResult<T>`?
---
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: Misc errors when errors happen
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/91#discussion_r15523211
--- Diff: core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java ---
@@ -516,10 +522,41 @@ protected void demoteToStandby() {
}
nodeState = ManagementNodeState.STANDBY;
+ onDemotion();
+ publishDemotionFromMaster(false);
+ }
+
+ protected void onDemotion() {
managementContext.getRebindManager().stop();
for (Application app: managementContext.getApplications())
Entities.unmanage(app);
- publishDemotionFromMaster(false);
+ // let's try forcibly interrupting tasks on managed entities
+ Collection<Exception> exceptions = MutableSet.of();
+ int tasks = 0;
+ LOG.debug("Cancelling tasks on demotion");
+ try {
+ for (Entity entity: managementContext.getEntityManager().getEntities()) {
+ for (Task<?> t: managementContext.getExecutionContext(entity).getTasks()) {
+ if (!t.isDone()) {
+ tasks++;
+ try {
+ LOG.debug("Cancelling "+t+" on "+entity);
+ t.cancel(true);
+ } catch (Exception e) {
--- End diff --
What exception could `t.cancel(true)` throw - is it just being cautious? No harm having the try-catch I guess.
---
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: Misc errors when errors happen
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/91
---
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.
---