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/10/25 12:28:03 UTC
[brooklyn-server] 12/15: make fewer things transient for more
runtime visibility
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 <al...@cloudsoftcorp.com>
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;
}