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 <[email protected]> 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")
