You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by an...@apache.org on 2017/06/09 10:54:15 UTC

[2/4] brooklyn-server git commit: replace custom caching with computeIfAbsent

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 <ro...@cloudsoftcorp.com>
Authored: Thu Jun 8 16:15:15 2017 +0100
Committer: Robert Moss <ro...@cloudsoftcorp.com>
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<Module> 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<ComputeService> computeServiceSupplier = allowReuse
-                ? new ReusableComputeServiceSupplier(conf, modules, properties)
-                : new ComputeServiceSupplierImpl(conf, modules, properties);
+        Properties properties = propertiesBuilder
+                .setJCloudsProperties()
+                .setEndpointProperty()
+                .build();
 
+        Supplier<ComputeService> 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<ComputeService> {
+    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<String, Object> 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<ComputeService> {
 
         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<? extends Module> modules, Properties properties) {
-            super(conf, modules, properties);
-        }
-    }
-
-    public class ReusableComputeServiceSupplier extends ComputeServiceSupplier {
-
-        private Map<?, ?> cacheKey;
-
-        public ReusableComputeServiceSupplier(ConfigBag conf, Iterable<? extends Module> 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<String, Object> 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<Credentials> makeCredentials(ConfigBag conf);
 
     /**