You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2021/09/14 21:56:43 UTC

[brooklyn-server] 24/27: make sure task GC doesn't kill new entity tasks

This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit 698b2d38397134fca4d68ae2e19942e621b39c80
Author: Alex Heneveld <al...@cloudsoftcorp.com>
AuthorDate: Tue Sep 14 19:48:08 2021 +0100

    make sure task GC doesn't kill new entity tasks
    
    previously when replacing an entity (eg on promotion) the new live entity's tasks could get deleted accidentally
---
 .../mgmt/internal/BrooklynGarbageCollector.java    |  8 +++---
 .../util/core/task/BasicExecutionManager.java      | 30 ++++++++++++++++++++++
 .../mgmt/internal/EntityExecutionManagerTest.java  | 23 +++++++++--------
 3 files changed, 46 insertions(+), 15 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/BrooklynGarbageCollector.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/BrooklynGarbageCollector.java
index 74f89a4..f780b5c 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/BrooklynGarbageCollector.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/BrooklynGarbageCollector.java
@@ -280,10 +280,10 @@ public class BrooklynGarbageCollector {
     public void deleteTasksForEntity(Entity entity) {
         // remove all references to this entity from tasks
         // (note that cancellation for most tasks will have been done by LocalEntityManager.stopTasks)
-        executionManager.deleteTag(entity);
-        executionManager.deleteTag(BrooklynTaskTags.tagForContextEntity(entity));
-        executionManager.deleteTag(BrooklynTaskTags.tagForCallerEntity(entity));
-        executionManager.deleteTag(BrooklynTaskTags.tagForTargetEntity(entity));
+        executionManager.deleteDoneInTag(entity);
+        executionManager.deleteDoneInTag(BrooklynTaskTags.tagForContextEntity(entity));
+        executionManager.deleteDoneInTag(BrooklynTaskTags.tagForCallerEntity(entity));
+        executionManager.deleteDoneInTag(BrooklynTaskTags.tagForTargetEntity(entity));
     }
     
     public void onUnmanaged(Location loc) {
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java b/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java
index 0498c4f..5f94d40 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java
@@ -69,6 +69,7 @@ import org.apache.brooklyn.core.mgmt.entitlement.Entitlements;
 import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.collections.MutableSet;
+import org.apache.brooklyn.util.core.task.BasicExecutionManager.BrooklynTaskLoggingMdc;
 import org.apache.brooklyn.util.core.task.TaskInternal.TaskCancellationMode;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.exceptions.RuntimeInterruptedException;
@@ -373,7 +374,9 @@ public class BasicExecutionManager implements ExecutionManager {
      * <p>
      * Useful, for example, if an entity is being expunged so that we don't keep holding
      * a reference to it as a tag.
+     * @deprecated since 1.1 use deleteDoneInTag
      */
+    @Deprecated
     public void deleteTag(Object tag) {
         Set<Task<?>> tasks;
         synchronized (tasksByTag) {
@@ -386,6 +389,33 @@ public class BasicExecutionManager implements ExecutionManager {
         }
     }
 
+    /**
+     * Deletes all truly done tasks in the given tag, and the tag also if empty when completed.
+     * @return if all tasks were done and the tag has been deleted
+     */
+    public boolean deleteDoneInTag(Object tag) {
+        Set<Task<?>> tasks;
+        boolean tagEmpty = true;
+        synchronized (tasksByTag) {
+            tasks = MutableSet.copyOf(tasksByTag.get(tag));
+        }
+        if (tasks != null) {
+            for (Task<?> task : tasks) {
+                if (task.isDone(true)) {
+                    deleteTask(task);
+                } else {
+                    tagEmpty = false;
+                }
+            }
+        }
+        if (tagEmpty) {
+            if (!tasksByTag.containsKey(tag)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     public void deleteTask(Task<?> task) {
         boolean removed = deleteTaskNonRecursive(task);
         if (!removed) return;
diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/EntityExecutionManagerTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/EntityExecutionManagerTest.java
index 4f581f5..6168942 100644
--- a/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/EntityExecutionManagerTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/EntityExecutionManagerTest.java
@@ -314,18 +314,19 @@ public class EntityExecutionManagerTest extends BrooklynAppUnitTestSupport {
         
         Entities.destroy(e);
         forceGc();
-        
-        Set<Object> tags2 = app.getManagementContext().getExecutionManager().getTaskTags();
-        for (Object tag : tags2) {
-            if (tag instanceof Entity && ((Entity)tag).getId().equals(eId)) {
-                fail("tags contains unmanaged entity "+tag);
-            }
-            if ((tag instanceof WrappedEntity) && ((WrappedEntity)tag).unwrap().getId().equals(eId) 
-                    && ((WrappedItem<?>)tag).getWrappingType().equals(BrooklynTaskTags.CONTEXT_ENTITY)) {
-                fail("tags contains unmanaged entity (wrapped) "+tag);
+
+        Asserts.succeedsEventually(() -> {
+            Set<Object> tags2 = app.getManagementContext().getExecutionManager().getTaskTags();
+            for (Object tag : tags2) {
+                if (tag instanceof Entity && ((Entity) tag).getId().equals(eId)) {
+                    fail("tags contains unmanaged entity " + tag + "; tasks: " + app.getManagementContext().getExecutionManager().getTasksWithTag(tag));
+                }
+                if ((tag instanceof WrappedEntity) && ((WrappedEntity) tag).unwrap().getId().equals(eId)
+                        && ((WrappedItem<?>) tag).getWrappingType().equals(BrooklynTaskTags.CONTEXT_ENTITY)) {
+                    fail("tags contains unmanaged entity (wrapped) " + tag + "; tasks: " + app.getManagementContext().getExecutionManager().getTasksWithTag(tag));
+                }
             }
-        }
-        return;
+        });
     }
 
     @Test(groups="Integration")