Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 542D8200CB1 for ; Fri, 9 Jun 2017 12:54:17 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 52EB6160B9C; Fri, 9 Jun 2017 10:54:17 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 276D2160BC8 for ; Fri, 9 Jun 2017 12:54:16 +0200 (CEST) Received: (qmail 63645 invoked by uid 500); 9 Jun 2017 10:54:15 -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 63627 invoked by uid 99); 9 Jun 2017 10:54:15 -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; Fri, 09 Jun 2017 10:54:15 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 9D675E0230; Fri, 9 Jun 2017 10:54:14 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: andreaturli@apache.org To: commits@brooklyn.apache.org Date: Fri, 09 Jun 2017 10:54:15 -0000 Message-Id: In-Reply-To: <57104a54954b4251853568be97ed3c47@git.apache.org> References: <57104a54954b4251853568be97ed3c47@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [2/4] brooklyn-server git commit: replace custom caching with computeIfAbsent archived-at: Fri, 09 Jun 2017 10:54:17 -0000 replace custom caching with computeIfAbsent Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/a4d8a95f Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/a4d8a95f Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/a4d8a95f Branch: refs/heads/master Commit: a4d8a95f0a2062fd278228d58cb31f9423ae90d3 Parents: e6ca01c Author: Robert Moss Authored: Thu Jun 8 16:15:15 2017 +0100 Committer: Robert Moss Committed: Thu Jun 8 16:15:15 2017 +0100 ---------------------------------------------------------------------- .../jclouds/AbstractComputeServiceRegistry.java | 303 +++++++++---------- 1 file changed, 137 insertions(+), 166 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a4d8a95f/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/AbstractComputeServiceRegistry.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/AbstractComputeServiceRegistry.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/AbstractComputeServiceRegistry.java index 076d442..8bc3388 100644 --- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/AbstractComputeServiceRegistry.java +++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/AbstractComputeServiceRegistry.java @@ -62,8 +62,8 @@ public abstract class AbstractComputeServiceRegistry implements ComputeServiceRe @Override public ComputeService findComputeService(ConfigBag conf, boolean allowReuse) { - Properties properties = new Properties(); - setCommonProperties(conf, properties); + PropertiesBuilder propertiesBuilder = new PropertiesBuilder(conf) + .setCommonProperties(); Iterable modules = getCommonModules(); @@ -71,9 +71,9 @@ public abstract class AbstractComputeServiceRegistry implements ComputeServiceRe // See https://issues.apache.org/jira/browse/WHIRR-416 String provider = getProviderFromConfig(conf); if ("aws-ec2".equals(provider)) { - setAWSEC2Properties(conf, properties); + propertiesBuilder.setAWSEC2Properties(); } else if ("azurecompute-arm".equals(provider)) { - setAzureComputeArmProperties(conf, properties); + propertiesBuilder.setAzureComputeArmProperties(); // jclouds 2.0.0 does not include the rate limit module for Azure ARM. This quick fix enables this which will // avoid provisioning to fail due to rate limit exceeded // See https://issues.apache.org/jira/browse/JCLOUDS-1229 @@ -83,17 +83,143 @@ public abstract class AbstractComputeServiceRegistry implements ComputeServiceRe .build(); } - addJCloudsProperties(conf, properties); - addEndpointProperty(conf, properties); - - Supplier computeServiceSupplier = allowReuse - ? new ReusableComputeServiceSupplier(conf, modules, properties) - : new ComputeServiceSupplierImpl(conf, modules, properties); + Properties properties = propertiesBuilder + .setJCloudsProperties() + .setEndpointProperty() + .build(); + Supplier computeServiceSupplier = new ComputeServiceSupplier(conf, modules, properties); + if (allowReuse) { + return cachedComputeServices.computeIfAbsent(makeCacheKey(conf, properties), key -> computeServiceSupplier.get()); + } return computeServiceSupplier.get(); } - public abstract class ComputeServiceSupplier implements Supplier { + private Map makeCacheKey(ConfigBag conf, Properties properties) { + String provider = getProviderFromConfig(conf); + String identity = checkNotNull(conf.get(CloudLocationConfig.ACCESS_IDENTITY), "identity must not be null"); + String credential = checkNotNull(conf.get(CloudLocationConfig.ACCESS_CREDENTIAL), "credential must not be null"); + String endpoint = properties.getProperty(Constants.PROPERTY_ENDPOINT); + return MutableMap.builder() + .putAll(properties) + .put("provider", provider) + .put("identity", identity) + .put("credential", credential) + .putIfNotNull("endpoint", endpoint) + .build() + .asUnmodifiable(); + } + + public class PropertiesBuilder { + private ConfigBag conf; + private Properties properties = new Properties(); + + public PropertiesBuilder(ConfigBag conf) { + this.conf = conf; + } + + public PropertiesBuilder setCommonProperties() { + properties.setProperty(Constants.PROPERTY_TRUST_ALL_CERTS, Boolean.toString(true)); + properties.setProperty(Constants.PROPERTY_RELAX_HOSTNAME, Boolean.toString(true)); + properties.setProperty("jclouds.ssh.max-retries", conf.getStringKey("jclouds.ssh.max-retries") != null ? + conf.getStringKey("jclouds.ssh.max-retries").toString() : "50"); + + if (conf.get(OAUTH_ENDPOINT) != null) + properties.setProperty(OAUTH_ENDPOINT.getName(), conf.get(OAUTH_ENDPOINT)); + + // See https://issues.apache.org/jira/browse/BROOKLYN-394 + // For retries, the backoff times are: + // Math.min(2^failureCount * retryDelayStart, retryDelayStart * 10) + random(10%) + // Therefore the backoff times will be: 500ms, 1s, 2s, 4s, 5s, 5s. + // The defaults (if not overridden here) are 50ms and 5 retires. This gives backoff + // times of 50ms, 100ms, 200ms, 400ms, 500ms (so a total backoff time of 1.25s), + // which is not long when you're being rate-limited and there are multiple thread all + // retrying their API calls. + properties.setProperty(Constants.PROPERTY_RETRY_DELAY_START, "500"); + properties.setProperty(Constants.PROPERTY_MAX_RETRIES, "6"); + return this; + } + + public PropertiesBuilder setAWSEC2Properties() { + // TODO convert AWS-only flags to config keys + if (groovyTruth(conf.get(IMAGE_ID))) { + properties.setProperty(PROPERTY_EC2_AMI_QUERY, ""); + properties.setProperty(PROPERTY_EC2_CC_AMI_QUERY, ""); + } else if (groovyTruth(conf.getStringKey("imageOwner"))) { + properties.setProperty(PROPERTY_EC2_AMI_QUERY, "owner-id=" + conf.getStringKey("imageOwner") + ";state=available;image-type=machine"); + } else if (groovyTruth(conf.getStringKey("anyOwner"))) { + // set `anyOwner: true` to override the default query (which is restricted to certain owners as per below), + // allowing the AMI query to bind to any machine + // (note however, we sometimes pick defaults in JcloudsLocationFactory); + // (and be careful, this can give a LOT of data back, taking several minutes, + // and requiring extra memory allocated on the command-line) + properties.setProperty(PROPERTY_EC2_AMI_QUERY, "state=available;image-type=machine"); + /* + * by default the following filters are applied: + * Filter.1.Name=owner-id&Filter.1.Value.1=137112412989& + * Filter.1.Value.2=063491364108& + * Filter.1.Value.3=099720109477& + * Filter.1.Value.4=411009282317& + * Filter.2.Name=state&Filter.2.Value.1=available& + * Filter.3.Name=image-type&Filter.3.Value.1=machine& + */ + } + + // See https://issues.apache.org/jira/browse/BROOKLYN-399 + String region = conf.get(CLOUD_REGION_ID); + if (Strings.isNonBlank(region)) { + /* + * Drop availability zone suffixes. Without this deployments to regions like us-east-1b fail + * because jclouds throws an IllegalStateException complaining that: location id us-east-1b + * not found in: [{scope=PROVIDER, id=aws-ec2, description=https://ec2.us-east-1.amazonaws.com, + * iso3166Codes=[US-VA, US-CA, US-OR, BR-SP, IE, DE-HE, SG, AU-NSW, JP-13]}]. The exception is + * thrown by org.jclouds.compute.domain.internal.TemplateBuilderImpl#locationId(String). + */ + if (Character.isLetter(region.charAt(region.length() - 1))) { + region = region.substring(0, region.length() - 1); + } + properties.setProperty(LocationConstants.PROPERTY_REGIONS, region); + } + + // occasionally can get com.google.common.util.concurrent.UncheckedExecutionException: java.lang.RuntimeException: + // security group eu-central-1/jclouds#brooklyn-bxza-alex-eu-central-shoul-u2jy-nginx-ielm is not available after creating + // the default timeout was 500ms so let's raise it in case that helps + properties.setProperty(EC2Constants.PROPERTY_EC2_TIMEOUT_SECURITYGROUP_PRESENT, "" + Duration.seconds(30).toMilliseconds()); + return this; + } + + private PropertiesBuilder setAzureComputeArmProperties() { + String region = conf.get(CLOUD_REGION_ID); + if (Strings.isNonBlank(region)) { + properties.setProperty(LocationConstants.PROPERTY_REGIONS, region); + } + return this; + } + + private PropertiesBuilder setJCloudsProperties() { + // Add extra jclouds-specific configuration + Map extra = Maps.filterKeys(conf.getAllConfig(), Predicates.containsPattern("^jclouds\\.")); + if (extra.size() > 0) { + String provider = getProviderFromConfig(conf); + LOG.debug("Configuring custom jclouds property overrides for {}: {}", provider, Sanitizer.sanitize(extra)); + } + properties.putAll(Maps.filterValues(extra, Predicates.notNull())); + return this; + } + + private PropertiesBuilder setEndpointProperty() { + String endpoint = conf.get(CloudLocationConfig.CLOUD_ENDPOINT); + if (!groovyTruth(endpoint)) endpoint = getDeprecatedProperty(conf, Constants.PROPERTY_ENDPOINT); + if (groovyTruth(endpoint)) properties.setProperty(Constants.PROPERTY_ENDPOINT, endpoint); + return this; + } + + public Properties build() { + return properties; + } + } + + public class ComputeServiceSupplier implements Supplier { private final String provider; private final ConfigBag conf; @@ -121,69 +247,6 @@ public abstract class AbstractComputeServiceRegistry implements ComputeServiceRe return computeServiceContext.getComputeService(); } } - - protected ConfigBag getConf() { - return conf; - } - - protected Properties getProperties() { - return properties; - } - } - - public class ComputeServiceSupplierImpl extends ComputeServiceSupplier { - - public ComputeServiceSupplierImpl(ConfigBag conf, Iterable modules, Properties properties) { - super(conf, modules, properties); - } - } - - public class ReusableComputeServiceSupplier extends ComputeServiceSupplier { - - private Map cacheKey; - - public ReusableComputeServiceSupplier(ConfigBag conf, Iterable modules, Properties properties) { - super(conf, modules, properties); - this.cacheKey = makeCacheKey(); - } - - @Override - public ComputeService get() { - ComputeService result = cachedComputeServices.get(cacheKey); - if (result != null) { - LOG.trace("jclouds ComputeService cache hit for compute service, for " + Sanitizer.sanitize(getProperties())); - return result; - } - LOG.debug("jclouds ComputeService cache miss for compute service, creating, for " + Sanitizer.sanitize(getProperties())); - final ComputeService computeService = super.get(); - synchronized (cachedComputeServices) { - result = cachedComputeServices.get(cacheKey); - if (result != null) { - LOG.debug("jclouds ComputeService cache recovery for compute service, for " + Sanitizer.sanitize(cacheKey)); - //keep the old one, discard the new one - computeService.getContext().close(); - return result; - } - LOG.debug("jclouds ComputeService created " + computeService + ", adding to cache, for " + Sanitizer.sanitize(getProperties())); - cachedComputeServices.put(cacheKey, computeService); - } - return result; - } - - private Map makeCacheKey() { - String provider = getProviderFromConfig(getConf()); - String identity = checkNotNull(getConf().get(CloudLocationConfig.ACCESS_IDENTITY), "identity must not be null"); - String credential = checkNotNull(getConf().get(CloudLocationConfig.ACCESS_CREDENTIAL), "credential must not be null"); - String endpoint = getProperties().getProperty(Constants.PROPERTY_ENDPOINT); - return MutableMap.builder() - .putAll(getProperties()) - .put("provider", provider) - .put("identity", identity) - .put("credential", credential) - .putIfNotNull("endpoint", endpoint) - .build() - .asUnmodifiable(); - } } protected String getProviderFromConfig(ConfigBag conf) { @@ -191,98 +254,6 @@ public abstract class AbstractComputeServiceRegistry implements ComputeServiceRe return DeserializingJcloudsRenamesProvider.INSTANCE.applyJcloudsRenames(rawProvider); } - private String addEndpointProperty(ConfigBag conf, Properties properties) { - String endpoint = conf.get(CloudLocationConfig.CLOUD_ENDPOINT); - if (!groovyTruth(endpoint)) endpoint = getDeprecatedProperty(conf, Constants.PROPERTY_ENDPOINT); - if (groovyTruth(endpoint)) properties.setProperty(Constants.PROPERTY_ENDPOINT, endpoint); - return endpoint; - } - - private void setCommonProperties(ConfigBag conf, Properties properties) { - properties.setProperty(Constants.PROPERTY_TRUST_ALL_CERTS, Boolean.toString(true)); - properties.setProperty(Constants.PROPERTY_RELAX_HOSTNAME, Boolean.toString(true)); - properties.setProperty("jclouds.ssh.max-retries", conf.getStringKey("jclouds.ssh.max-retries") != null ? - conf.getStringKey("jclouds.ssh.max-retries").toString() : "50"); - - if (conf.get(OAUTH_ENDPOINT) != null) - properties.setProperty(OAUTH_ENDPOINT.getName(), conf.get(OAUTH_ENDPOINT)); - - // See https://issues.apache.org/jira/browse/BROOKLYN-394 - // For retries, the backoff times are: - // Math.min(2^failureCount * retryDelayStart, retryDelayStart * 10) + random(10%) - // Therefore the backoff times will be: 500ms, 1s, 2s, 4s, 5s, 5s. - // The defaults (if not overridden here) are 50ms and 5 retires. This gives backoff - // times of 50ms, 100ms, 200ms, 400ms, 500ms (so a total backoff time of 1.25s), - // which is not long when you're being rate-limited and there are multiple thread all - // retrying their API calls. - properties.setProperty(Constants.PROPERTY_RETRY_DELAY_START, "500"); - properties.setProperty(Constants.PROPERTY_MAX_RETRIES, "6"); - } - - private void addJCloudsProperties(ConfigBag conf, Properties properties) { - // Add extra jclouds-specific configuration - Map extra = Maps.filterKeys(conf.getAllConfig(), Predicates.containsPattern("^jclouds\\.")); - if (extra.size() > 0) { - String provider = getProviderFromConfig(conf); - LOG.debug("Configuring custom jclouds property overrides for {}: {}", provider, Sanitizer.sanitize(extra)); - } - properties.putAll(Maps.filterValues(extra, Predicates.notNull())); - } - - private void setAzureComputeArmProperties(ConfigBag conf, Properties properties) { - String region = conf.get(CLOUD_REGION_ID); - if (Strings.isNonBlank(region)) { - properties.setProperty(LocationConstants.PROPERTY_REGIONS, region); - } - } - - private void setAWSEC2Properties(ConfigBag conf, Properties properties) { - // TODO convert AWS-only flags to config keys - if (groovyTruth(conf.get(IMAGE_ID))) { - properties.setProperty(PROPERTY_EC2_AMI_QUERY, ""); - properties.setProperty(PROPERTY_EC2_CC_AMI_QUERY, ""); - } else if (groovyTruth(conf.getStringKey("imageOwner"))) { - properties.setProperty(PROPERTY_EC2_AMI_QUERY, "owner-id=" + conf.getStringKey("imageOwner") + ";state=available;image-type=machine"); - } else if (groovyTruth(conf.getStringKey("anyOwner"))) { - // set `anyOwner: true` to override the default query (which is restricted to certain owners as per below), - // allowing the AMI query to bind to any machine - // (note however, we sometimes pick defaults in JcloudsLocationFactory); - // (and be careful, this can give a LOT of data back, taking several minutes, - // and requiring extra memory allocated on the command-line) - properties.setProperty(PROPERTY_EC2_AMI_QUERY, "state=available;image-type=machine"); - /* - * by default the following filters are applied: - * Filter.1.Name=owner-id&Filter.1.Value.1=137112412989& - * Filter.1.Value.2=063491364108& - * Filter.1.Value.3=099720109477& - * Filter.1.Value.4=411009282317& - * Filter.2.Name=state&Filter.2.Value.1=available& - * Filter.3.Name=image-type&Filter.3.Value.1=machine& - */ - } - - // See https://issues.apache.org/jira/browse/BROOKLYN-399 - String region = conf.get(CLOUD_REGION_ID); - if (Strings.isNonBlank(region)) { - /* - * Drop availability zone suffixes. Without this deployments to regions like us-east-1b fail - * because jclouds throws an IllegalStateException complaining that: location id us-east-1b - * not found in: [{scope=PROVIDER, id=aws-ec2, description=https://ec2.us-east-1.amazonaws.com, - * iso3166Codes=[US-VA, US-CA, US-OR, BR-SP, IE, DE-HE, SG, AU-NSW, JP-13]}]. The exception is - * thrown by org.jclouds.compute.domain.internal.TemplateBuilderImpl#locationId(String). - */ - if (Character.isLetter(region.charAt(region.length() - 1))) { - region = region.substring(0, region.length() - 1); - } - properties.setProperty(LocationConstants.PROPERTY_REGIONS, region); - } - - // occasionally can get com.google.common.util.concurrent.UncheckedExecutionException: java.lang.RuntimeException: - // security group eu-central-1/jclouds#brooklyn-bxza-alex-eu-central-shoul-u2jy-nginx-ielm is not available after creating - // the default timeout was 500ms so let's raise it in case that helps - properties.setProperty(EC2Constants.PROPERTY_EC2_TIMEOUT_SECURITYGROUP_PRESENT, "" + Duration.seconds(30).toMilliseconds()); - } - protected abstract Supplier makeCredentials(ConfigBag conf); /**