From b23ee407f51a87d57616d7b531f3b0a9fcdb4fb1 Mon Sep 17 00:00:00 2001 From: Maggie Neterval Date: Tue, 6 Aug 2019 12:56:04 -0400 Subject: [PATCH] perf(google): improve performance of /serverGroups endpoint (#3906) * perf(google): improve performance of /serverGroups endpoint * fix(google): make application cache data a static class, improve var names per code review * fix(google): make GoogleApplicaitonProvider.getApplicationCacheData private --- .../view/GoogleApplicationProvider.java | 46 +++++++++---- .../view/GoogleClusterProvider.groovy | 68 ++++++++++++------- .../view/GoogleInstanceProvider.groovy | 2 +- 3 files changed, 79 insertions(+), 37 deletions(-) diff --git a/clouddriver-google/src/main/groovy/com/netflix/spinnaker/clouddriver/google/provider/view/GoogleApplicationProvider.java b/clouddriver-google/src/main/groovy/com/netflix/spinnaker/clouddriver/google/provider/view/GoogleApplicationProvider.java index 3140d55406d..a7ef7f1106f 100644 --- a/clouddriver-google/src/main/groovy/com/netflix/spinnaker/clouddriver/google/provider/view/GoogleApplicationProvider.java +++ b/clouddriver-google/src/main/groovy/com/netflix/spinnaker/clouddriver/google/provider/view/GoogleApplicationProvider.java @@ -24,7 +24,6 @@ import static java.util.stream.Collectors.toSet; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.collect.ImmutableSet; import com.netflix.spinnaker.cats.cache.Cache; import com.netflix.spinnaker.cats.cache.CacheData; import com.netflix.spinnaker.cats.cache.RelationshipCacheFilter; @@ -35,9 +34,12 @@ import com.netflix.spinnaker.clouddriver.model.Application; import com.netflix.spinnaker.clouddriver.model.ApplicationProvider; import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import lombok.Value; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @@ -66,32 +68,50 @@ public Set getApplications(boolean expand) { return data.stream().map(this::applicationFromCacheData).collect(toSet()); } - @Override - public Application getApplication(String name) { + @Value + static class ApplicationCacheData { + Map applicationAttributes; + Set clusterIdentifiers; + Set instanceIdentifiers; + } + ApplicationCacheData getApplicationCacheData(String name) { CacheData cacheData = cacheView.get( APPLICATIONS.getNs(), Keys.getApplicationKey(name), RelationshipCacheFilter.include(CLUSTERS.getNs(), INSTANCES.getNs())); - if (cacheData == null) { - return null; - } + return getApplicationCacheData(cacheData); + } - return applicationFromCacheData(cacheData); + private ApplicationCacheData getApplicationCacheData(CacheData cacheData) { + return new ApplicationCacheData( + cacheData.getAttributes(), + getRelationships(cacheData, CLUSTERS), + getRelationships(cacheData, INSTANCES)); + } + + @Override + public Application getApplication(String name) { + return applicationFromCacheData(getApplicationCacheData(name)); } private GoogleApplication.View applicationFromCacheData(CacheData cacheData) { + return applicationFromCacheData(getApplicationCacheData(cacheData)); + } + private GoogleApplication.View applicationFromCacheData( + ApplicationCacheData applicationCacheData) { GoogleApplication application = - objectMapper.convertValue(cacheData.getAttributes(), GoogleApplication.class); + objectMapper.convertValue( + applicationCacheData.getApplicationAttributes(), GoogleApplication.class); if (application == null) { return null; } GoogleApplication.View applicationView = application.getView(); - Collection clusters = getRelationships(cacheData, CLUSTERS); + Set clusters = applicationCacheData.getClusterIdentifiers(); clusters.forEach( key -> { Map parsedKey = Keys.parse(key); @@ -102,14 +122,14 @@ private GoogleApplication.View applicationFromCacheData(CacheData cacheData) { }); List> instances = - getRelationships(cacheData, INSTANCES).stream().map(Keys::parse).collect(toList()); + applicationCacheData.getInstanceIdentifiers().stream().map(Keys::parse).collect(toList()); applicationView.setInstances(instances); return applicationView; } - private Collection getRelationships(CacheData cacheData, Namespace namespace) { - Collection result = cacheData.getRelationships().get(namespace.getNs()); - return result != null ? result : ImmutableSet.of(); + private Set getRelationships(CacheData cacheData, Namespace namespace) { + Collection relationships = cacheData.getRelationships().get(namespace.getNs()); + return relationships == null ? Collections.emptySet() : new HashSet<>(relationships); } } diff --git a/clouddriver-google/src/main/groovy/com/netflix/spinnaker/clouddriver/google/provider/view/GoogleClusterProvider.groovy b/clouddriver-google/src/main/groovy/com/netflix/spinnaker/clouddriver/google/provider/view/GoogleClusterProvider.groovy index a6d2895938e..fffd9636d60 100644 --- a/clouddriver-google/src/main/groovy/com/netflix/spinnaker/clouddriver/google/provider/view/GoogleClusterProvider.groovy +++ b/clouddriver-google/src/main/groovy/com/netflix/spinnaker/clouddriver/google/provider/view/GoogleClusterProvider.groovy @@ -17,6 +17,7 @@ package com.netflix.spinnaker.clouddriver.google.provider.view import com.fasterxml.jackson.databind.ObjectMapper +import com.google.common.collect.ImmutableSet import com.netflix.spinnaker.cats.cache.Cache import com.netflix.spinnaker.cats.cache.CacheData import com.netflix.spinnaker.cats.cache.RelationshipCacheFilter @@ -77,26 +78,37 @@ class GoogleClusterProvider implements ClusterProvider { } Map> getClusters(String applicationName, boolean includeInstanceDetails) { - GoogleApplication.View application = applicationProvider.getApplication(applicationName) + GoogleApplicationProvider.ApplicationCacheData applicationCacheData = applicationProvider.getApplicationCacheData(applicationName) - def clusterKeys = [] - application?.clusterNames?.each { String accountName, Set clusterNames -> - clusterNames.each { String clusterName -> - clusterKeys << Keys.getClusterKey(accountName, applicationName, clusterName) - } - } - - // TODO(jacobkiefer): Avoid parsing instance keys into map just to re-serialize? - Set allApplicationInstanceKeys = includeInstanceDetails ? application?.instances?.collect { Keys.getInstanceKey(it.account, it.region, it.name) } : [] as Set - - List clusters = cacheView.getAll( - CLUSTERS.ns, - clusterKeys, - RelationshipCacheFilter.include(SERVER_GROUPS.ns)).collect { CacheData cacheData -> - clusterFromCacheData(cacheData, allApplicationInstanceKeys) + Set clusterIdentifiers = applicationCacheData.getClusterIdentifiers(); + Collection clusterCacheData = cacheView.getAll( + CLUSTERS.ns, + clusterIdentifiers, + RelationshipCacheFilter.include(SERVER_GROUPS.ns) + ) + + Set instanceIdentifiers = includeInstanceDetails ? + applicationCacheData.getInstanceIdentifiers() : + Collections.emptySet() + Collection instanceCacheData = instanceProvider.getInstanceCacheData(instanceIdentifiers) + + Map> clustersByAccount = new HashMap<>() + Map> securityGroupsByAccount = new HashMap<>() + + clusterCacheData.each { cacheData -> + String accountName = cacheData.getAttributes().get("accountName") + Set accountSecurityGroups = securityGroupsByAccount.computeIfAbsent( + accountName, + { a -> securityGroupProvider.getAllByAccount(false, accountName) } + ) + Set accountClusters = clustersByAccount.computeIfAbsent( + accountName, + { a -> new HashSet() } + ) + accountClusters.add(clusterFromCacheData(cacheData, instanceCacheData, accountSecurityGroups)) } - clusters?.groupBy { it.accountName } as Map> + return clustersByAccount } @Override @@ -167,10 +179,22 @@ class GoogleClusterProvider implements ClusterProvider { return false } - GoogleCluster.View clusterFromCacheData(CacheData cacheData, Set instanceKeySuperSet) { - GoogleCluster.View clusterView = objectMapper.convertValue(cacheData.attributes, GoogleCluster)?.view + GoogleCluster.View clusterFromCacheData(CacheData clusterCacheData, Set instanceKeySuperSet) { + return clusterFromCacheData( + clusterCacheData, + instanceProvider.getInstanceCacheData(instanceKeySuperSet), + securityGroupProvider.getAllByAccount(false, (String) clusterCacheData.getAttributes().get("accountName")) + ) + } + + GoogleCluster.View clusterFromCacheData( + CacheData clusterCacheData, + Collection instanceCacheDataSuperSet, + Set securityGroups) + { + GoogleCluster.View clusterView = objectMapper.convertValue(clusterCacheData.attributes, GoogleCluster)?.view - def serverGroupKeys = cacheData.relationships[SERVER_GROUPS.ns] + def serverGroupKeys = clusterCacheData.relationships[SERVER_GROUPS.ns] if (serverGroupKeys) { log.debug("Server group keys from cluster relationships: ${serverGroupKeys}") def filter = RelationshipCacheFilter.include(LOAD_BALANCERS.ns) @@ -178,9 +202,7 @@ class GoogleClusterProvider implements ClusterProvider { def serverGroupData = cacheView.getAll(SERVER_GROUPS.ns, serverGroupKeys, filter) log.debug("Retrieved cache data for server groups: ${serverGroupData?.collect { it?.attributes?.name }}") - def securityGroups = securityGroupProvider.getAllByAccount(false, clusterView.accountName) - - def instanceCacheData = instanceProvider.getInstanceCacheData(instanceKeySuperSet as List).findAll { instance -> + def instanceCacheData = instanceCacheDataSuperSet.findAll { instance -> instance.relationships.get(CLUSTERS.ns)?.collect { Keys.parse(it).cluster }?.any { it.contains(clusterView.name) } } diff --git a/clouddriver-google/src/main/groovy/com/netflix/spinnaker/clouddriver/google/provider/view/GoogleInstanceProvider.groovy b/clouddriver-google/src/main/groovy/com/netflix/spinnaker/clouddriver/google/provider/view/GoogleInstanceProvider.groovy index 497ad6d77e8..4e502e38bbe 100644 --- a/clouddriver-google/src/main/groovy/com/netflix/spinnaker/clouddriver/google/provider/view/GoogleInstanceProvider.groovy +++ b/clouddriver-google/src/main/groovy/com/netflix/spinnaker/clouddriver/google/provider/view/GoogleInstanceProvider.groovy @@ -81,7 +81,7 @@ class GoogleInstanceProvider implements InstanceProvider getInstanceCacheData(List keys) { + Collection getInstanceCacheData(Collection keys) { cacheView.getAll(INSTANCES.ns, keys, RelationshipCacheFilter.include(LOAD_BALANCERS.ns,