brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From henev...@apache.org
Subject [16/23] brooklyn-server git commit: change when cancellation is done for getImmediate - means effector invocation now works
Date Fri, 06 Oct 2017 08:06:40 GMT
change when cancellation is done for getImmediate - means effector invocation now works


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/bb26d320
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/bb26d320
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/bb26d320

Branch: refs/heads/master
Commit: bb26d320906ad86888c28cc32b2fa36709e70d1a
Parents: d7b086b
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Authored: Thu Sep 28 16:55:33 2017 +0100
Committer: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Committed: Thu Sep 28 17:29:59 2017 +0100

----------------------------------------------------------------------
 .../util/core/task/BasicExecutionContext.java     | 18 +++++++-----------
 .../brooklyn/core/effector/EffectorSayHiTest.java |  6 +++---
 2 files changed, 10 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb26d320/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
b/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
index f9f431a..2c8ddab 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
@@ -287,17 +287,6 @@ public class BasicExecutionContext extends AbstractExecutionContext {
         try {
             return runInSameThread(fakeTaskForContext, new Callable<Maybe<T>>()
{
                 public Maybe<T> call() {
-                    // could try to make this work for more types of tasks by not cancelling,
just interrupting;
-                    // however there is a danger that immediate-submission tasks are leaked
if we don't cancel.
-                    // for instance with DSTs the thread interrupt may apply only to the
main job queue.andWait blocking,
-                    // leaving other tasks leaked.
-                    //
-                    // this method is best-effort so fine if it doesn't succeed.  good if
we can expand
-                    // coverage but NOT at the expense of major leaks of course!
-                    //
-                    // see WIP test in EffectorSayHiTest
-                    fakeTaskForContext.cancel();
-                    
                     boolean wasAlreadyInterrupted = Thread.interrupted();
                     try {
                         return job.getImmediately();
@@ -305,6 +294,13 @@ public class BasicExecutionContext extends AbstractExecutionContext {
                         if (wasAlreadyInterrupted) {
                             Thread.currentThread().interrupt();
                         }
+                        // we've acknowledged that getImmediate may wreck (cancel) the task,
+                        // their first priority is to prevent them from leaking;
+                        // however previously we did the cancel before running, 
+                        // doing it after means more tasks successfully execute 
+                        // (the interrupt is sufficient to prevent them blocking); 
+                        // see test EffectorSayHiTest.testInvocationGetImmediately
+                        fakeTaskForContext.cancel();
                     }
                 } });
         } catch (Exception e) {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb26d320/core/src/test/java/org/apache/brooklyn/core/effector/EffectorSayHiTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/effector/EffectorSayHiTest.java b/core/src/test/java/org/apache/brooklyn/core/effector/EffectorSayHiTest.java
index bc41d45..e7dc626 100644
--- a/core/src/test/java/org/apache/brooklyn/core/effector/EffectorSayHiTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/effector/EffectorSayHiTest.java
@@ -61,6 +61,7 @@ public class EffectorSayHiTest extends BrooklynAppUnitTestSupport {
     //TODO test edge/error conditions
     //(missing parameters, wrong number of params, etc)
 
+    @SuppressWarnings("unused")
     private static final Logger log = LoggerFactory.getLogger(EffectorSayHiTest.class);
 
     private MyEntity e;
@@ -109,11 +110,10 @@ public class EffectorSayHiTest extends BrooklynAppUnitTestSupport {
             .get( Effectors.invocation(e, MyEntity.SAY_HI_1, ImmutableMap.of("name", "Bob",
"greeting", "hi")) ), "hi Bob");
     }
     
-    @Test(groups="WIP")  // see comments at BasicExecutionContext.getImmediately
-    // TODO this will be fixed soon by #835
+    @Test(invocationCount=100)
     public void testInvocationGetImmediately() throws Exception {
         assertEquals(((EntityInternal)e).getExecutionContext()
-            .getImmediately( Effectors.invocation(e, MyEntity.SAY_HI_1, ImmutableMap.of("name",
"Bob", "greeting", "hi")) ), "hi Bob");
+            .getImmediately( Effectors.invocation(e, MyEntity.SAY_HI_1, ImmutableMap.of("name",
"Bob", "greeting", "hi")) ).get(), "hi Bob");
     }
 
     @Test


Mime
View raw message