perf(amazon): optimize target group loading for /loadBalancers #2610
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jrsquared
reviewed
May 8, 2018
| @@ -378,6 +377,10 @@ class AmazonApplicationLoadBalancerCachingAgent extends AbstractAmazonLoadBalanc | |||
| Map<String, CacheData> loadBalancers = CacheHelpers.cache() | |||
| Map<String, CacheData> targetGroups = CacheHelpers.cache() | |||
|
|
|||
| Map<String, String> targetGroupNameToType = allTargetGroups.collectEntries { | |||
jrsquared
approved these changes
May 8, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Incremental update to the
/{cloudProvider}/loadBalancersendpoint for AWS. By adding the target type to the target group key, we can fetch them all once and parse them instead of going to Redis once for each ALB and fetching the full target group details.This is not going to be a major performance improvement on its own, I don't think: it seems like we're cutting the number of entries we get from Redis by about 33% (although that number would grow larger as ALBs supplant CLBs). The structure of the response to the call is much more complex than the comparable one to fetch all security groups - in addition to target groups, we include security group IDs associate with the load balancer for some reason, and I'm not sure who is using that data.
There are additional improvements we're considering making in Deck - mostly around not calling this endpoint unless we need to. At Netflix, over 99% of our clusters follow the established naming conventions, and use load balancers and target groups that are named after the application, so we'll have that data available when configuring a deployment cluster. We might as well just show that list and provide a link to "Show all Load Balancers" to enable the < 1% of users to pick load balancers from different applications.