Skip to content

Commit

Permalink
perf(google): improve performance of /serverGroups endpoint (#3906)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@maggieneterval
maggieneterval committed Aug 6, 2019
1 parent ca4943c commit b840208
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 37 deletions.
46 changes: 33 additions & 13 deletions ...ovy/com/netflix/spinnaker/clouddriver/google/provider/view/GoogleApplicationProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -66,32 +68,50 @@ public Set<? extends Application> getApplications(boolean expand) {
return data.stream().map(this::applicationFromCacheData).collect(toSet());
}

@Override
public Application getApplication(String name) {
@Value
static class ApplicationCacheData {
Map<String, Object> applicationAttributes;
Set<String> clusterIdentifiers;
Set<String> 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<String> clusters = getRelationships(cacheData, CLUSTERS);
Set<String> clusters = applicationCacheData.getClusterIdentifiers();
clusters.forEach(
key -> {
Map<String, String> parsedKey = Keys.parse(key);
Expand All @@ -102,14 +122,14 @@ private GoogleApplication.View applicationFromCacheData(CacheData cacheData) {
});

List<Map<String, String>> 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<String> getRelationships(CacheData cacheData, Namespace namespace) {
Collection<String> result = cacheData.getRelationships().get(namespace.getNs());
return result != null ? result : ImmutableSet.of();
private Set<String> getRelationships(CacheData cacheData, Namespace namespace) {
Collection<String> relationships = cacheData.getRelationships().get(namespace.getNs());
return relationships == null ? Collections.emptySet() : new HashSet<>(relationships);
}
}
68 changes: 45 additions & 23 deletions ...roovy/com/netflix/spinnaker/clouddriver/google/provider/view/GoogleClusterProvider.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -77,26 +78,37 @@ class GoogleClusterProvider implements ClusterProvider<GoogleCluster.View> {
}

Map<String, Set<GoogleCluster.View>> 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<String> clusterNames ->
clusterNames.each { String clusterName ->
clusterKeys << Keys.getClusterKey(accountName, applicationName, clusterName)
}
}

// TODO(jacobkiefer): Avoid parsing instance keys into map just to re-serialize?
Set<String> allApplicationInstanceKeys = includeInstanceDetails ? application?.instances?.collect { Keys.getInstanceKey(it.account, it.region, it.name) } : [] as Set

List<GoogleCluster.View> clusters = cacheView.getAll(
CLUSTERS.ns,
clusterKeys,
RelationshipCacheFilter.include(SERVER_GROUPS.ns)).collect { CacheData cacheData ->
clusterFromCacheData(cacheData, allApplicationInstanceKeys)
Set<String> clusterIdentifiers = applicationCacheData.getClusterIdentifiers();
Collection<CacheData> clusterCacheData = cacheView.getAll(
CLUSTERS.ns,
clusterIdentifiers,
RelationshipCacheFilter.include(SERVER_GROUPS.ns)
)

Set<String> instanceIdentifiers = includeInstanceDetails ?
applicationCacheData.getInstanceIdentifiers() :
Collections.<String>emptySet()
Collection<CacheData> instanceCacheData = instanceProvider.getInstanceCacheData(instanceIdentifiers)

Map<String, Set<GoogleCluster.View>> clustersByAccount = new HashMap<>()
Map<String, Set<GoogleSecurityGroup>> securityGroupsByAccount = new HashMap<>()

clusterCacheData.each { cacheData ->
String accountName = cacheData.getAttributes().get("accountName")
Set<GoogleSecurityGroup> accountSecurityGroups = securityGroupsByAccount.computeIfAbsent(
accountName,
{ a -> securityGroupProvider.getAllByAccount(false, accountName) }
)
Set<GoogleCluster.View> accountClusters = clustersByAccount.computeIfAbsent(
accountName,
{ a -> new HashSet<GoogleCluster.View>() }
)
accountClusters.add(clusterFromCacheData(cacheData, instanceCacheData, accountSecurityGroups))
}

clusters?.groupBy { it.accountName } as Map<String, Set<GoogleCluster.View>>
return clustersByAccount
}

@Override
Expand Down Expand Up @@ -167,20 +179,30 @@ class GoogleClusterProvider implements ClusterProvider<GoogleCluster.View> {
return false
}

GoogleCluster.View clusterFromCacheData(CacheData cacheData, Set<String> instanceKeySuperSet) {
GoogleCluster.View clusterView = objectMapper.convertValue(cacheData.attributes, GoogleCluster)?.view
GoogleCluster.View clusterFromCacheData(CacheData clusterCacheData, Set<String> instanceKeySuperSet) {
return clusterFromCacheData(
clusterCacheData,
instanceProvider.getInstanceCacheData(instanceKeySuperSet),
securityGroupProvider.getAllByAccount(false, (String) clusterCacheData.getAttributes().get("accountName"))
)
}

GoogleCluster.View clusterFromCacheData(
CacheData clusterCacheData,
Collection<CacheData> instanceCacheDataSuperSet,
Set<GoogleSecurityGroup> 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)

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) }
}

Expand Down
2 changes: 1 addition & 1 deletion ...oovy/com/netflix/spinnaker/clouddriver/google/provider/view/GoogleInstanceProvider.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class GoogleInstanceProvider implements InstanceProvider<GoogleInstance.View, St
}
}

Collection<CacheData> getInstanceCacheData(List<String> keys) {
Collection<CacheData> getInstanceCacheData(Collection<String> keys) {
cacheView.getAll(INSTANCES.ns,
keys,
RelationshipCacheFilter.include(LOAD_BALANCERS.ns,
Expand Down

0 comments on commit b840208

Please sign in to comment.