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 6b1e2b20a4703379a765520168393155b0e2c1e5 Author: Alex Heneveld <[email protected]> AuthorDate: Thu Oct 21 22:02:56 2021 +0100 make fewer things transient for more runtime visibility now that GC and structure is better, framework tasks aren't as much clutter so don't need to be transient; however those which are transient have nicer API and are propagated for same-thread execution as well as submission --- .../brooklyn/core/config/ConfigConstraints.java | 4 ++-- .../java/org/apache/brooklyn/core/feed/Poller.java | 4 ++-- .../apache/brooklyn/core/mgmt/BrooklynTaskTags.java | 4 ++-- .../core/mgmt/internal/EntityManagementSupport.java | 4 ++-- .../core/objs/proxy/InternalEntityFactory.java | 20 ++++---------------- .../stock/aggregator/DashboardAggregator.java | 6 ++++-- .../util/core/task/BasicExecutionContext.java | 11 +++++++++-- .../org/apache/brooklyn/util/core/task/TaskTags.java | 3 ++- .../base/AbstractSoftwareProcessSshDriver.java | 2 +- .../entity/software/base/lifecycle/ScriptHelper.java | 4 ++-- 10 files changed, 30 insertions(+), 32 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java b/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java index 2d92c45..5a87311 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java +++ b/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java @@ -145,8 +145,8 @@ public abstract class ConfigConstraints<T> { ExecutionContext exec = getExecutionContext(); if (exec!=null) { return exec.get( - Tasks.<Map<ConfigKey<?>,Throwable>>builder().dynamic(false).displayName("Validating config").body( - () -> validateAll() ).build() ); + BrooklynTaskTags.setTransient( // set transient so gets GC and doesn't pollute the "top-level" view + Tasks.<Map<ConfigKey<?>,Throwable>>builder().dynamic(false).displayName("Validating config").body( () -> validateAll() ).build() )); } else { return validateAll(); } diff --git a/core/src/main/java/org/apache/brooklyn/core/feed/Poller.java b/core/src/main/java/org/apache/brooklyn/core/feed/Poller.java index b870110..6049ecf 100644 --- a/core/src/main/java/org/apache/brooklyn/core/feed/Poller.java +++ b/core/src/main/java/org/apache/brooklyn/core/feed/Poller.java @@ -156,8 +156,8 @@ public class Poller<V> { pollJob.wrappedJob.run(); return null; } } ); - // don't set transient -- it is good to be able to see these in the UI - //BrooklynTaskTags.setTransient(task); + // explicitly make non-transient -- we want to see its execution, even if parent is transient + BrooklynTaskTags.addTagDynamically(task, BrooklynTaskTags.NON_TRANSIENT_TASK_TAG); return task; }) .displayName("scheduled:" + scheduleName) diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java index ea2dd84..d8d5733 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java @@ -386,8 +386,8 @@ public class BrooklynTaskTags extends TaskTags { // ------ misc - public static void setInessential(Task<?> task) { addTagDynamically(task, INESSENTIAL_TASK); } - public static void setTransient(Task<?> task) { addTagDynamically(task, TRANSIENT_TASK_TAG); } + public static <TR,T extends Task<TR>> T setInessential(T task) { return addTagDynamically(task, INESSENTIAL_TASK); } + public static <TR,T extends Task<TR>> T setTransient(T task) { return addTagDynamically(task, TRANSIENT_TASK_TAG); } public static boolean isTransient(Task<?> task) { if (hasTag(task, TRANSIENT_TASK_TAG)) return true; if (hasTag(task, NON_TRANSIENT_TASK_TAG)) return false; diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EntityManagementSupport.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EntityManagementSupport.java index 7dd074e..ea32e92 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EntityManagementSupport.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EntityManagementSupport.java @@ -175,7 +175,7 @@ public class EntityManagementSupport { public void onManagementStarting(ManagementTransitionInfo info) { info.getManagementContext().getExecutionContext(entity).get( Tasks.builder().displayName("Management starting") .dynamic(false) - .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG) +// .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG) .body(() -> { try { synchronized (this) { boolean alreadyManaging = isDeployed(); @@ -234,7 +234,7 @@ public class EntityManagementSupport { public void onManagementStarted(ManagementTransitionInfo info) { info.getManagementContext().getExecutionContext(entity).get( Tasks.builder().displayName("Management started") .dynamic(false) - .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG) +// .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG) .body(() -> { try { synchronized (this) { boolean alreadyManaged = isFullyManaged(); diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java b/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java index aedd617..df01680 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java @@ -292,7 +292,7 @@ public class InternalEntityFactory extends InternalFactory { protected <T extends Entity> T loadUnitializedEntity(final T entity, final EntitySpec<T> spec, EntityManager. EntityCreationOptions options) { try { - Task<T> initialize = Tasks.create("initialize", () -> { + Task<T> initialize = Tasks.create("Initialize model classes", () -> { final AbstractEntity theEntity = (AbstractEntity) entity; if (spec.getDisplayName() != null) theEntity.setDisplayName(spec.getDisplayName()); @@ -321,7 +321,7 @@ public class InternalEntityFactory extends InternalFactory { } return entity; }); - BrooklynTaskTags.setTransient(initialize); +// BrooklynTaskTags.setTransient(initialize); // don't set this transient; we might want to be able to see what it does, eg adding scheduled tasks return ((AbstractEntity) entity).getExecutionContext().get(initialize); } catch (Exception e) { @@ -381,21 +381,9 @@ public class InternalEntityFactory extends InternalFactory { // than in manageRecursive so that rebind is unaffected. validateDescendantConfig(entity, options); - /* Marked transient so that the task is not needlessly kept around at the highest level. - * Note that the task is not normally visible in the GUI, because - * (a) while it is running, the entity is often parentless (and so not in the tree); - * and (b) when it is completed it is GC'd, as it is transient. - * However task info is available via the API if you know its ID, - * and if better subtask querying is available it will be picked up as a background task - * of the parent entity creating this child entity - * (note however such subtasks are currently filtered based on parent entity so is excluded). - * <p> - * Some of these (initializers and enrichers) submit background scheduled tasks, - * which currently show up at the top level once the initializer task completes. - * TODO It would be nice if these schedule tasks were grouped in a bucket! - */ ((EntityInternal)entity).getExecutionContext().get(Tasks.builder().dynamic(false).displayName("Entity initialization") - .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG) + // no longer transient because the UI groups these more nicely now +// .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG) .body(new Runnable() { @Override public void run() { diff --git a/core/src/main/java/org/apache/brooklyn/enricher/stock/aggregator/DashboardAggregator.java b/core/src/main/java/org/apache/brooklyn/enricher/stock/aggregator/DashboardAggregator.java index 8fe05f0..2f46339 100644 --- a/core/src/main/java/org/apache/brooklyn/enricher/stock/aggregator/DashboardAggregator.java +++ b/core/src/main/java/org/apache/brooklyn/enricher/stock/aggregator/DashboardAggregator.java @@ -67,11 +67,13 @@ public class DashboardAggregator extends AbstractEnricher { .dynamic(false) .body(new AggregationJob(entity)) .displayName("DashboardAggregator task") - .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG) +// .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG) .description("Retrieves and aggregates sensor values") .build(); - task = ScheduledTask.builder(taskFactory).period(duration).displayName("scheduled:[DashboardAggregator task]").tagTransient().build(); + task = ScheduledTask.builder(taskFactory).period(duration).displayName("scheduled:[DashboardAggregator task]") + //.tagTransient() + .build(); this.getManagementContext().getExecutionManager().submit(task); } 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 9d329e5..dc69347 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 @@ -219,8 +219,15 @@ public class BasicExecutionContext extends AbstractExecutionContext { * seeing how the two work (as opposed to this method being designed as something * more generally useful). */ private <T> Maybe<T> runInSameThread(final Task<T> task, Callable<Maybe<T>> job) { - ((TaskInternal<T>)task).getMutableTags().addAll(tags); - + Set<Object> mutableTags = ((TaskInternal<T>) task).getMutableTags(); + mutableTags.addAll(tags); + + if (Tasks.current()!=null && BrooklynTaskTags.isTransient(Tasks.current()) + && !mutableTags.contains(BrooklynTaskTags.NON_TRANSIENT_TASK_TAG) && !mutableTags.contains(BrooklynTaskTags.TRANSIENT_TASK_TAG)) { + // tag as transient if submitter is transient, unless explicitly tagged as non-transient + mutableTags.add(BrooklynTaskTags.TRANSIENT_TASK_TAG); + } + Task<?> previousTask = BasicExecutionManager.getPerThreadCurrentTask().get(); BasicExecutionContext oldExecutionContext = getCurrentExecutionContext(); registerPerThreadExecutionContext(); diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/TaskTags.java b/core/src/main/java/org/apache/brooklyn/util/core/task/TaskTags.java index c62ace4..a899e03 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/task/TaskTags.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/task/TaskTags.java @@ -36,7 +36,7 @@ public class TaskTags { /** marks a task which is a subtask of another */ public static final String SUB_TASK_TAG = "SUB-TASK"; - public static void addTagDynamically(TaskAdaptable<?> task, final Object tag) { + public static <TT, TA extends TaskAdaptable<TT>> TA addTagDynamically(TA task, final Object tag) { ((BasicTask<?>)task.asTask()).applyTagModifier(new Function<Set<Object>, Void>() { @Override public Void apply(@Nullable Set<Object> input) { @@ -44,6 +44,7 @@ public class TaskTags { return null; } }); + return task; } public static void addTagsDynamically(TaskAdaptable<?> task, final Object tag1, final Object ...tags) { diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java index fc58de5..acf59bf 100644 --- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java +++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java @@ -515,7 +515,7 @@ public abstract class AbstractSoftwareProcessSshDriver extends AbstractSoftwareP } if (phase.equalsIgnoreCase(CHECK_RUNNING)) { s.setInessential(); - s.setTransient(); + //s.setTransient(); s.setFlag(SshTool.PROP_CONNECT_TIMEOUT, Duration.TEN_SECONDS.toMilliseconds()); s.setFlag(SshTool.PROP_SESSION_TIMEOUT, Duration.THIRTY_SECONDS.toMilliseconds()); s.setFlag(SshTool.PROP_SSH_TRIES, 1); diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/ScriptHelper.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/ScriptHelper.java index 9898d48..975d42c 100644 --- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/ScriptHelper.java +++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/ScriptHelper.java @@ -270,8 +270,8 @@ public class ScriptHelper { return this; } - /** indicates explicitly that the task can be safely forgotten about after it runs; useful for things like - * check_running which run repeatedly */ + /** indicates explicitly that the task can be safely forgotten about after it runs; + * possibly useful for things like check_running which run repeatedly, though improved task GC heuristics (name-based limits) mean this is often unnecessary */ public void setTransient() { isTransient = true; }
