Skip to content

Commit

Permalink
perf(titus): Optimize out the per-security group lookups
Browse files Browse the repository at this point in the history 
This PR determines the security group id -> name mapping by parsing
identifiers rather than fetching entire security group objects.

Looks to shave a few seconds locally (~30-40%) off the agent execution
time.
  • Loading branch information
@ajordens @tomaslin
ajordens authored and tomaslin committed Mar 26, 2018
1 parent d350c33 commit c4043f7
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 59 deletions.
109 changes: 63 additions & 46 deletions ...vy/com/netflix/spinnaker/clouddriver/titus/caching/agents/TitusClusterCachingAgent.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,22 +278,24 @@ class TitusClusterCachingAgent implements CachingAgent, CustomScheduledAgent, On
List cacheablePolicyStates = [ScalingPolicyState.Pending, ScalingPolicyState.Applied, ScalingPolicyState.Deleting]
Map<String, TitusSecurityGroup> titusSecurityGroupCache = [:]

for (Job job : jobs) {
try {
List<ScalingPolicyData> scalingPolicies = allScalingPolicies.findResults {
it.jobId == job.id && cacheablePolicyStates.contains(it.policyState.state) ?
new ScalingPolicyData(id: it.id.id, policy: it.scalingPolicy, status: it.policyState) :
null
}
ServerGroupData data = new ServerGroupData(job, scalingPolicies, account.name, region, account.stack)
cacheApplication(data, applications)
cacheCluster(data, clusters)
cacheServerGroup(data, serverGroups, instances, titusSecurityGroupCache)
} catch (Exception ex) {
log.error("Failed to cache ${job.name} in ${account.name}", ex)
def serverGroupDatas = jobs.collect { job ->
List<ScalingPolicyData> scalingPolicies = allScalingPolicies.findResults {
it.jobId == job.id && cacheablePolicyStates.contains(it.policyState.state) ?
new ScalingPolicyData(id: it.id.id, policy: it.scalingPolicy, status: it.policyState) :
null
}

return new ServerGroupData(job, scalingPolicies, account.name, region, account.stack)
}

serverGroupDatas.each { data ->
cacheApplication(data, applications)
cacheCluster(data, clusters)
}

// caching _all_ jobs at once allows us to optimize the security group lookups
cacheServerGroups(serverGroupDatas, serverGroups, instances, titusSecurityGroupCache)

new DefaultCacheResult(
[(APPLICATIONS.ns) : applications.values(),
(CLUSTERS.ns) : clusters.values(),
Expand Down Expand Up @@ -321,34 +323,45 @@ class TitusClusterCachingAgent implements CachingAgent, CustomScheduledAgent, On
}
}

private void cacheServerGroup(ServerGroupData data, Map<String, CacheData> serverGroups, Map<String, CacheData> instances, Map titusSecurityGroupCache) {
serverGroups[data.serverGroup].with {
Job job = objectMapper.convertValue(data.job, Job.class)
resolveAwsDetails(titusSecurityGroupCache, job)
List<Map> policies = data.scalingPolicies ? data.scalingPolicies.collect {
// There is probably a better way to convert a protobuf to a Map, but I don't know what it is
[
id: it.id,
status: [ state: it.status.state.name(), reason: it.status.pendingReason ],
policy: objectMapper.readValue(JsonFormat.printer().print(it.policy), Map)
]
} : []

// tasks are cached independently as instances so avoid the overhead of also storing on the serialized job
def jobTasks = job.tasks
job.tasks = []

attributes.job = job
attributes.scalingPolicies = policies
attributes.tasks = jobTasks.collect { [ id: it.id, instanceId: it.instanceId ] }
attributes.region = region
attributes.account = account.name
relationships[APPLICATIONS.ns].add(data.appName)
relationships[CLUSTERS.ns].add(data.cluster)
relationships[INSTANCES.ns].addAll(data.instanceIds)
for (Job.TaskSummary task : jobTasks) {
def instanceData = new InstanceData(job, task, account.name, region, account.stack)
cacheInstance(instanceData, instances)
private void cacheServerGroups(List<ServerGroupData> datas,
Map<String, CacheData> serverGroups,
Map<String, CacheData> instances,
Map titusSecurityGroupCache) {
def allJobs = datas*.job
resolveAwsDetails(titusSecurityGroupCache, allJobs)

datas.each { data ->
serverGroups[data.serverGroup].with {
try {
Job job = objectMapper.convertValue(data.job, Job.class)
List<Map> policies = data.scalingPolicies ? data.scalingPolicies.collect {
// There is probably a better way to convert a protobuf to a Map, but I don't know what it is
[
id : it.id,
status: [state: it.status.state.name(), reason: it.status.pendingReason],
policy: objectMapper.readValue(JsonFormat.printer().print(it.policy), Map)
]
} : []

// tasks are cached independently as instances so avoid the overhead of also storing on the serialized job
def jobTasks = job.tasks
job.tasks = []

attributes.job = job
attributes.scalingPolicies = policies
attributes.tasks = jobTasks.collect { [id: it.id, instanceId: it.instanceId] }
attributes.region = region
attributes.account = account.name
relationships[APPLICATIONS.ns].add(data.appName)
relationships[CLUSTERS.ns].add(data.cluster)
relationships[INSTANCES.ns].addAll(data.instanceIds)
for (Job.TaskSummary task : jobTasks) {
def instanceData = new InstanceData(job, task, account.name, region, account.stack)
cacheInstance(instanceData, instances)
}
} catch (Exception e) {
log.error("Failed to cache ${data.job.name} in ${account.name}", e)
}
}
}
}
Expand Down Expand Up @@ -419,11 +432,15 @@ class TitusClusterCachingAgent implements CachingAgent, CustomScheduledAgent, On
}

private void resolveAwsDetails(Map<String, TitusSecurityGroup> titusSecurityGroupCache,
Job job) {
Set<TitusSecurityGroup> securityGroups = awsLookupUtil.get().lookupSecurityGroupNames(
titusSecurityGroupCache, account.name, region, job.securityGroups
)
job.securityGroupDetails = securityGroups
List<Job> jobs) {
def allSecurityGroupIds = jobs*.securityGroups.flatten() as List<String>
def allSecurityGroupsById = awsLookupUtil.get().lookupSecurityGroupNames(
titusSecurityGroupCache, account.name, region, allSecurityGroupIds
).collectEntries { [it.groupId, it] }

jobs.each {
it.securityGroupDetails = it.securityGroups.collect { allSecurityGroupsById[it] } as Set<TitusSecurityGroup>
}
}

private String getAwsAccountId(String account) {
Expand Down
47 changes: 34 additions & 13 deletions src/main/groovy/com/netflix/spinnaker/clouddriver/titus/caching/utils/AwsLookupUtil.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,22 @@

package com.netflix.spinnaker.clouddriver.titus.caching.utils

import com.netflix.spinnaker.clouddriver.aws.cache.Keys
import com.netflix.spinnaker.clouddriver.aws.model.AmazonVpc
import com.netflix.spinnaker.clouddriver.aws.provider.view.AmazonSecurityGroupProvider
import com.netflix.spinnaker.clouddriver.aws.provider.view.AmazonVpcProvider
import com.netflix.spinnaker.clouddriver.aws.security.AmazonCredentials
import com.netflix.spinnaker.clouddriver.aws.services.RegionScopedProviderFactory
import com.netflix.spinnaker.clouddriver.security.AccountCredentialsProvider
import com.netflix.spinnaker.clouddriver.titus.client.security.TitusCredentials
import com.netflix.spinnaker.clouddriver.titus.credentials.NetflixTitusCredentials
import com.netflix.spinnaker.clouddriver.titus.model.TitusSecurityGroup
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.stereotype.Component

import javax.annotation.PostConstruct

import static com.netflix.spinnaker.clouddriver.aws.cache.Keys.Namespace.SECURITY_GROUPS

@Component
class AwsLookupUtil {

Expand All @@ -53,25 +55,44 @@ class AwsLookupUtil {
String region,
List<String> securityGroups) {
Set<TitusSecurityGroup> expandedGroups = new LinkedHashSet<TitusSecurityGroup>()
Set<String> missingSecurityGroupIds = []

securityGroups.each { securityGroupId ->
def titusSecurityGroupLookupCacheId = "${account}-${region}-${securityGroupId}".toString()
TitusSecurityGroup titusSecurityGroup = titusSecurityGroupLookupCache.get(titusSecurityGroupLookupCacheId)

try {
if (!titusSecurityGroup) {
titusSecurityGroup = new TitusSecurityGroup(groupId: securityGroupId)
Map details = getSecurityGroupDetails(account, region, securityGroupId)
if (details) {
titusSecurityGroup.groupName = details.name
titusSecurityGroup.awsAccount = details.awsAccount
titusSecurityGroup.awsVpcId = details.vpcId
}
titusSecurityGroupLookupCache.put(titusSecurityGroupLookupCacheId, titusSecurityGroup)
}
if (!titusSecurityGroup) {
missingSecurityGroupIds << securityGroupId
} else {
expandedGroups << titusSecurityGroup
} catch (Exception ignored) {
}
}

if (missingSecurityGroupIds) {
def securityGroupNamesByIdentifier = awsSecurityGroupProvider.cacheView.getIdentifiers(
SECURITY_GROUPS.ns
).collectEntries {
def key = Keys.parse(it)
[key.id, key.name]
}

Map awsDetails = awsAccountLookup.find {
it.titusAccount == account && it.region == region
}

expandedGroups.addAll(missingSecurityGroupIds.collect {
def titusSecurityGroup = new TitusSecurityGroup(groupId: it)
titusSecurityGroup.groupName = securityGroupNamesByIdentifier[it]
titusSecurityGroup.awsAccount = awsDetails.awsAccount
titusSecurityGroup.awsVpcId = awsDetails.vpcId

def titusSecurityGroupLookupCacheId = "${account}-${region}-${it}".toString()
titusSecurityGroupLookupCache.put(titusSecurityGroupLookupCacheId, titusSecurityGroup)

return titusSecurityGroup
})
}

expandedGroups
}

Expand Down

0 comments on commit c4043f7

Please sign in to comment.