You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by neykov <gi...@git.apache.org> on 2016/10/14 17:21:21 UTC
[GitHub] brooklyn-server pull request #385: Fix/task context
GitHub user neykov opened a pull request:
https://github.com/apache/brooklyn-server/pull/385
Fix/task context
When executing a task which is already tagged with a context entity in the execution context of another task, the task ends up with two context entities in its tags. It's indeterministic which one will be available through BrooklynTaskTags.getContextEntity.
The context entity should be set by internal code at a single place. Remove all places where we are setting the context additionally and add a warning at runtime if detected. To be changed to an exception after a release or two, currently a warning in the logs.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/neykov/brooklyn-server fix/task-context
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/385.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #385
----
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #385: Don't allow multiple context entities in ...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/385#discussion_r87398136
--- Diff: core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java ---
@@ -203,11 +211,46 @@ public Void apply(Task<?> it) {
throw new IllegalArgumentException("Unhandled task type: task="+task+"; type="+(task!=null ? task.getClass() : "null"));
}
}
-
+
private void registerPerThreadExecutionContext() { perThreadExecutionContext.set(this); }
private void clearPerThreadExecutionContext() { perThreadExecutionContext.remove(); }
+ private void checkUserSuppliedContext(Object task, Collection<Object> taskTags) {
+ Entity taskContext = BrooklynTaskTags.getWrappedEntityOfType(taskTags, BrooklynTaskTags.CONTEXT_ENTITY);
+ Entity defaultContext = BrooklynTaskTags.getWrappedEntityOfType(tags, BrooklynTaskTags.CONTEXT_ENTITY);
+ if (taskContext != null) {
+ if (log.isWarnEnabled()) {
+ String msg = "Deprecated since 0.10.0. Task " + task + " is submitted for execution but has context " +
+ "entity (" + taskContext + ") tag set by the caller. ";
+ if (taskContext != defaultContext) {
+ msg += "The context entity of the execution context (" + this + ") the task is submitted on is " +
+ defaultContext + " which is different. This will cause any of them to be used at random at " +
+ "runtime. ";
+ if (task instanceof BasicTask) {
+ msg += "Fixing the context entity to the latter. ";
+ }
+ }
+ msg += "Setting the context entity by the caller is not allowed. See the documentation on " +
+ "BrooklynTaskTags.tagForContextEntity(Entity) method for more details. Future Apache Brooklyn " +
+ "releases will throw an exception instead of logging a warning.";
+
+ /**
+ * @deprecated since 0.10.0
+ */
+ // Should we rate limit?
+ log.warn(msg);
--- End diff --
Let's monitor (including grep'ing the output logs from the mvn build), and rate limit if we're seeing this a lot.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #385: Don't allow multiple context entities in a task'...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/385
@neykov the possible explanation of "entity got on fire and that aborted the wait for the sensor" makes sense. I fixed a similar underlying problem in https://github.com/apache/brooklyn-server/commit/0070c6b417963018bbb1e4a86d1cedd1f249fc22 for `AbstractApplication`, but it looks like this `attributeWhenReady` has been called on `TestEntity` so not sure what would have caused that.
Merging now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #385: Don't allow multiple context entities in a task'...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/385
changes look good to me -- nice work @neykov
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #385: Don't allow multiple context entities in a task'...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/385
I'm not able to reproduce the exception and the build is now gone for deeper inspection.
The only explanation I have is that somehow `entity` got on fire and that aborted the wait for the sensor. Could be some interplay between the `newEnricherForServiceUpIfNotUpIndicatorsEmpty` enricher and manually setting `SERVICE_UP` value.
Resolved merge conflicts - let's see what this build brings us.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #385: Don't allow multiple context entities in a task'...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/385
The jenkins test failure was `EntityConfigUsageTest.testGetAttributeWhenReadyConfigBlocksUntilSet`:
```
java.lang.reflect.UndeclaredThrowableException: null
at java.util.concurrent.FutureTask.report(FutureTask.java:122)
at java.util.concurrent.FutureTask.get(FutureTask.java:188)
at com.google.common.util.concurrent.ForwardingFuture.get(ForwardingFuture.java:63)
at org.apache.brooklyn.util.core.task.BasicTask.get(BasicTask.java:361)
at org.apache.brooklyn.util.core.task.ValueResolver.getMaybeInternal(ValueResolver.java:354)
at org.apache.brooklyn.util.core.task.ValueResolver.getMaybe(ValueResolver.java:280)
at org.apache.brooklyn.util.core.task.ValueResolver.get(ValueResolver.java:273)
at org.apache.brooklyn.util.core.task.Tasks.resolveValue(Tasks.java:147)
at org.apache.brooklyn.core.config.BasicConfigKey.resolveValue(BasicConfigKey.java:405)
at org.apache.brooklyn.core.config.BasicConfigKey.extractValue(BasicConfigKey.java:390)
at org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl.resolveRawValueFromContainer(AbstractConfigMapImpl.java:308)
at org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl$3.apply(AbstractConfigMapImpl.java:360)
at org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl$3.apply(AbstractConfigMapImpl.java:357)
at org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl.getConfigImpl(AbstractConfigMapImpl.java:372)
at org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl.getConfig(AbstractConfigMapImpl.java:107)
at org.apache.brooklyn.core.objs.AbstractConfigurationSupportInternal.get(AbstractConfigurationSupportInternal.java:169)
at org.apache.brooklyn.core.entity.AbstractEntity.getConfig(AbstractEntity.java:1262)
at sun.reflect.GeneratedMethodAccessor69.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at org.apache.brooklyn.core.objs.proxy.EntityProxyImpl.invoke(EntityProxyImpl.java:188)
at com.sun.proxy.$Proxy2428.getConfig(Unknown Source)
at org.apache.brooklyn.core.entity.EntityConfigUsageTest.testGetAttributeWhenReadyConfigBlocksUntilSet(EntityConfigUsageTest.java:303)
Caused by: org.apache.brooklyn.util.exceptions.CompoundRuntimeException: Aborted waiting for ready from TestEntityImpl{id=twbvmkjwhe} Sensor: test.name (java.lang.String)
at org.apache.brooklyn.core.sensor.DependentConfiguration$WaitInTaskForAttributeReady.call(DependentConfiguration.java:353)
at org.apache.brooklyn.util.core.task.BasicExecutionManager$SubmissionCallable.call(BasicExecutionManager.java:522)
at java.util.concurrent.FutureTask.run(FutureTask.java:262)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.Exception: Abort due to TestEntityImpl{id=twbvmkjwhe} -> Sensor: service.state (org.apache.brooklyn.core.entity.lifecycle.Lifecycle)
at org.apache.brooklyn.core.sensor.DependentConfiguration$WaitInTaskForAttributeReady$2.onEvent(DependentConfiguration.java:293)
at org.apache.brooklyn.core.mgmt.internal.LocalSubscriptionManager$2.run(LocalSubscriptionManager.java:255)
at org.apache.brooklyn.util.concurrent.CallableFromRunnable.call(CallableFromRunnable.java:43)
at org.apache.brooklyn.util.core.task.BasicExecutionManager$SubmissionCallable.call(BasicExecutionManager.java:522)
at org.apache.brooklyn.util.core.task.SingleThreadedScheduler$1.call(SingleThreadedScheduler.java:116)
at java.util.concurrent.FutureTask.run(FutureTask.java:262)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
at java.lang.Thread.run(Thread.java:745)
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #385: Don't allow multiple context entities in ...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/385#discussion_r87817025
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java ---
@@ -320,7 +320,6 @@ private void validateDescendantConfig(Entity e) {
* TODO It would be nice if these schedule tasks were grouped in a bucket!
*/
((EntityInternal)entity).getExecutionContext().submit(Tasks.builder().dynamic(false).displayName("Entity initialization")
- .tag(BrooklynTaskTags.tagForContextEntity(entity))
--- End diff --
this looks right, but i guess redundant because of the `submit`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #385: Don't allow multiple context entities in ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/385
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #385: Don't allow multiple context entities in ...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/385#discussion_r85510265
--- Diff: core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java ---
@@ -53,10 +58,18 @@ public LocationConfigMap(AbstractLocation loc, Map<ConfigKey<?>, Object> storage
@Override
protected ExecutionContext getExecutionContext(BrooklynObject bo) {
- if (bo==null) return null;
- ManagementContext mgmt = ((AbstractLocation)bo).getManagementContext();
- if (mgmt==null) return null;
- return mgmt.getServerExecutionContext();
+ // Support DSL in location config. DSL expects to be resolved in the execution context of an entity.
+ // Since there's no location-entity relation try to infer it from the context of the caller.
+ Entity contextEntity = BrooklynTaskTags.getTargetOrContextEntity(Tasks.current());
+ if (contextEntity != null) {
+ return ((EntityInternal)contextEntity).getExecutionContext();
--- End diff --
@ahgittin Any thoughts on this change? It's retrieving the context entity's execution context if there's one, to be used as the location's execution context.
It's actually the same as what we currently do, just cleaning it up a bit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #385: Don't allow multiple context entities in a task'...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/385
Resolved merger conflicts.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---