Return-Path: X-Original-To: apmail-brooklyn-commits-archive@minotaur.apache.org Delivered-To: apmail-brooklyn-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id AC8EB18723 for ; Sat, 30 Jan 2016 03:45:47 +0000 (UTC) Received: (qmail 47693 invoked by uid 500); 30 Jan 2016 03:45:47 -0000 Delivered-To: apmail-brooklyn-commits-archive@brooklyn.apache.org Received: (qmail 47599 invoked by uid 500); 30 Jan 2016 03:45:47 -0000 Mailing-List: contact commits-help@brooklyn.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@brooklyn.apache.org Delivered-To: mailing list commits@brooklyn.apache.org Received: (qmail 47500 invoked by uid 99); 30 Jan 2016 03:45:47 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 30 Jan 2016 03:45:47 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 0FDFDDFFF4; Sat, 30 Jan 2016 03:45:47 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: heneveld@apache.org To: commits@brooklyn.apache.org Date: Sat, 30 Jan 2016 03:45:47 -0000 Message-Id: <108b67118da145dfb3430c62c6c31c5a@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [01/10] incubator-brooklyn git commit: ensure install-label-salt-inferencing does not block Repository: incubator-brooklyn Updated Branches: refs/heads/master a6f32193d -> 5b37751dc ensure install-label-salt-inferencing does not block otherwise setting a shell env var to include a port will cause it to block prematurely. also clean up javadoc around `getNonBlocking` behaviour. Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/5e9012b2 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/5e9012b2 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/5e9012b2 Branch: refs/heads/master Commit: 5e9012b23e2bf9ad6423a5e0ef13c7ea8153947d Parents: b28ba02 Author: Alex Heneveld Authored: Thu Jan 21 13:22:21 2016 +0000 Committer: Alex Heneveld Committed: Thu Jan 21 13:25:36 2016 +0000 ---------------------------------------------------------------------- .../AbstractConfigurationSupportInternal.java | 5 ++-- .../core/objs/BrooklynObjectInternal.java | 11 ++++++++ .../brooklyn/util/core/flags/TypeCoercions.java | 4 +-- .../brooklyn/util/core/task/ValueResolver.java | 12 +++++++++ ...wareProcessDriverLifecycleEffectorTasks.java | 3 ++- .../base/VanillaSoftwareProcessSshDriver.java | 28 +++++++++++++++----- 6 files changed, 51 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/5e9012b2/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java ---------------------------------------------------------------------- diff --git a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java index 6cb8bb4..1d4cfea 100644 --- a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java +++ b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java @@ -19,7 +19,6 @@ package org.apache.brooklyn.core.objs; -import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; import org.apache.brooklyn.api.mgmt.ExecutionContext; @@ -28,8 +27,8 @@ import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.config.ConfigKey.HasConfigKey; import org.apache.brooklyn.util.core.flags.TypeCoercions; import org.apache.brooklyn.util.core.task.Tasks; +import org.apache.brooklyn.util.core.task.ValueResolver; import org.apache.brooklyn.util.guava.Maybe; -import org.apache.brooklyn.util.time.Duration; public abstract class AbstractConfigurationSupportInternal implements BrooklynObjectInternal.ConfigurationSupportInternal { @@ -63,7 +62,7 @@ public abstract class AbstractConfigurationSupportInternal implements BrooklynOb Object resolved = Tasks.resolving(unresolved) .as(Object.class) .defaultValue(marker) - .timeout(Duration.of(5, TimeUnit.MILLISECONDS)) + .timeout(ValueResolver.REAL_REAL_QUICK_WAIT) .context(getContext()) .swallowExceptions() .get(); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/5e9012b2/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java ---------------------------------------------------------------------- diff --git a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java index 2da7463..89c4e32 100644 --- a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java +++ b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java @@ -69,6 +69,9 @@ public interface BrooklynObjectInternal extends BrooklynObject, Rebindable { /** * Returns the uncoerced value for this config key, if available, not taking any default. * If there is no local value and there is an explicit inherited value, will return the inherited. + * Returns {@link Maybe#absent()} if the key is not explicitly set on this object or an ancestor. + *

+ * See also {@link #getLocalRaw(ConfigKey). */ @Beta Maybe getRaw(ConfigKey key); @@ -82,6 +85,9 @@ public interface BrooklynObjectInternal extends BrooklynObject, Rebindable { /** * Returns the uncoerced value for this config key, if available, * not following any inheritance chains and not taking any default. + * Returns {@link Maybe#absent()} if the key is not explicitly set on this object. + *

+ * See also {@link #getRaw(ConfigKey). */ @Beta Maybe getLocalRaw(ConfigKey key); @@ -96,6 +102,11 @@ public interface BrooklynObjectInternal extends BrooklynObject, Rebindable { * Attempts to coerce the value for this config key, if available, * taking a default and {@link Maybe#absent absent} if the uncoerced * cannot be resolved within a short timeframe. + *

+ * Note: if no value for the key is available, not even as a default, + * this returns a {@link Maybe#isPresent()} containing null + * (following the semantics of {@link #get(ConfigKey)} + * rather than {@link #getRaw(ConfigKey)}). */ @Beta Maybe getNonBlocking(ConfigKey key); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/5e9012b2/brooklyn-server/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java ---------------------------------------------------------------------- diff --git a/brooklyn-server/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java b/brooklyn-server/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java index f850fbd..872e713 100644 --- a/brooklyn-server/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java +++ b/brooklyn-server/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java @@ -122,10 +122,10 @@ public class TypeCoercions { return coerce(value, TypeToken.of(targetType)); } - /** @see #coerce(Object, Class) */ + /** @see #coerce(Object, Class); allows a null value in the contents of the Maybe */ public static Maybe tryCoerce(Object value, TypeToken targetTypeToken) { try { - return Maybe.of( coerce(value, targetTypeToken) ); + return Maybe.ofAllowingNull( coerce(value, targetTypeToken) ); } catch (Throwable t) { Exceptions.propagateIfFatal(t); return Maybe.absent(t); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/5e9012b2/brooklyn-server/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java ---------------------------------------------------------------------- diff --git a/brooklyn-server/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java b/brooklyn-server/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java index 2809482..0f85c3f 100644 --- a/brooklyn-server/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java +++ b/brooklyn-server/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java @@ -42,6 +42,7 @@ import org.apache.brooklyn.util.time.Durations; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.annotations.Beta; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -57,14 +58,25 @@ import com.google.common.reflect.TypeToken; */ public class ValueResolver implements DeferredSupplier { + // TODO most of these usages should be removed when we have + // an ability to run resolution in a non-blocking mode + // (i.e. running resolution tasks in the same thread, + // or in a context where they can only wait on subtasks + // which are guaranteed to have the same constraint) /** * Period to wait if we're expected to return real quick * but we want fast things to have time to finish. *

* Timings are always somewhat arbitrary but this at least * allows some intention to be captured in code rather than arbitrary values. */ + @Beta public static Duration REAL_QUICK_WAIT = Duration.millis(50); /** + * Like {@link #REAL_QUICK_WAIT} but even smaller, for use when potentially + * resolving multiple items in sequence. */ + @Beta + public static Duration REAL_REAL_QUICK_WAIT = Duration.millis(5); + /** * Period to wait if we're expected to return quickly * but we want to be a bit more generous for things to finish, * without letting a caller get annoyed. http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/5e9012b2/brooklyn-server/software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcessDriverLifecycleEffectorTasks.java ---------------------------------------------------------------------- diff --git a/brooklyn-server/software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcessDriverLifecycleEffectorTasks.java b/brooklyn-server/software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcessDriverLifecycleEffectorTasks.java index 91aa294..cb4149a 100644 --- a/brooklyn-server/software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcessDriverLifecycleEffectorTasks.java +++ b/brooklyn-server/software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcessDriverLifecycleEffectorTasks.java @@ -118,7 +118,8 @@ public class SoftwareProcessDriverLifecycleEffectorTasks extends MachineLifecycl entity().initDriver(machine); // Note: must only apply config-sensors after adding to locations and creating driver; - // otherwise can't do things like acquire free port from location, or allowing driver to set up ports + // otherwise can't do things like acquire free port from location + // or allowing driver to set up ports; but must be careful in init not to block on these! super.preStartCustom(machine); entity().preStart(); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/5e9012b2/brooklyn-server/software/base/src/main/java/org/apache/brooklyn/entity/software/base/VanillaSoftwareProcessSshDriver.java ---------------------------------------------------------------------- diff --git a/brooklyn-server/software/base/src/main/java/org/apache/brooklyn/entity/software/base/VanillaSoftwareProcessSshDriver.java b/brooklyn-server/software/base/src/main/java/org/apache/brooklyn/entity/software/base/VanillaSoftwareProcessSshDriver.java index dd01a69..9c955ed 100644 --- a/brooklyn-server/software/base/src/main/java/org/apache/brooklyn/entity/software/base/VanillaSoftwareProcessSshDriver.java +++ b/brooklyn-server/software/base/src/main/java/org/apache/brooklyn/entity/software/base/VanillaSoftwareProcessSshDriver.java @@ -21,11 +21,12 @@ package org.apache.brooklyn.entity.software.base; import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.Objects; import org.apache.brooklyn.api.entity.EntityLocal; import org.apache.brooklyn.api.entity.drivers.downloads.DownloadResolver; +import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.entity.Entities; +import org.apache.brooklyn.core.objs.BrooklynObjectInternal.ConfigurationSupportInternal; import org.apache.brooklyn.entity.software.base.lifecycle.ScriptHelper; import org.apache.brooklyn.location.ssh.SshMachineLocation; import org.apache.brooklyn.util.collections.MutableMap; @@ -54,15 +55,30 @@ public class VanillaSoftwareProcessSshDriver extends AbstractSoftwareProcessSshD */ @Override protected String getInstallLabelExtraSalt() { + // run non-blocking in case a value set later is used (e.g. a port) + Integer hash = hashCodeIfResolved(SoftwareProcess.DOWNLOAD_URL.getConfigKey(), + VanillaSoftwareProcess.INSTALL_COMMAND, SoftwareProcess.SHELL_ENVIRONMENT); - Maybe url = getEntity().getConfigRaw(SoftwareProcess.DOWNLOAD_URL, true); - String installCommand = getEntity().getConfig(VanillaSoftwareProcess.INSTALL_COMMAND); - Map shellEnv = getEntity().getConfig(VanillaSoftwareProcess.SHELL_ENVIRONMENT); + // if any of the above blocked then we must make a unique install label, + // as other yet-unknown config is involved + if (hash==null) return Identifiers.makeRandomId(8); - // TODO a user-friendly hash would be nice, but tricky since we don't want it to be too long or contain path chars - return Identifiers.makeIdFromHash(Objects.hash(url.or(null), installCommand, shellEnv)); + // a user-friendly hash is nice, but tricky since it would have to be short; + // go with a random one unless it's totally blank + if (hash==0) return "default"; + return Identifiers.makeIdFromHash(hash); } + private Integer hashCodeIfResolved(ConfigKey ...keys) { + int hash = 0; + for (ConfigKey k: keys) { + Maybe value = ((ConfigurationSupportInternal)getEntity().config()).getNonBlocking(k); + if (value.isAbsent()) return null; + hash = hash*31 + (value.get()==null ? 0 : value.get().hashCode()); + } + return hash; + } + @Override public void install() { Maybe url = getEntity().getConfigRaw(SoftwareProcess.DOWNLOAD_URL, true);