Optimise activation speed #119
Merged
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
added 3 commits
July 4, 2022 14:47
Prior to this change, when you tried to activate a revision we list *all* revisions to try and discover if the passed argument is valid. In our case this ends up fetching tens of thousands of revisions from s3 which is kinda slow... It turns out that the AWS `listObjects` function takes a prefix (this is already used in the code to somewhat limit the results). We can pass the entire expected filename and retrieve exactly one record (which is of course much much faster)
I guess there is the possibility that we could have retrieved information about a revision with a `Key` which *starts with* the `Prefix` we passed (rather than the entire filename being the prefix we passed). It's pretty easy to guard against this edge case so let's do so...
We were again grabbing _every_ uploaded revision in order to be able to check if you were trying to overwrite a revision. This is slow when you have many revisions uploaded to s3. Here we prefer our new `findRevision` method which uses the `Prefix` to load information about _just_ the revision we're interested in.
|
Thanks for the quick merge! Hopefully the failing build isn't my fault?
|
I doubt that is the case. @LevelbossMike any insight into this? |
This was referenced Jul 4, 2022
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.
What Changed & Why
I noticed that the activation step of our deployments was getting pretty slow. When digging into it I figured out that it's because we have 60,000+ revisions for each app deployed and the activation process uses S3's
ListObjectsno fewer than 3 times to get that list of revisions. This gets to be pretty slooooow!This PR includes the first simple changes which don't change existing functionality. In a couple of places the list of all revisions were being loaded just to figure out if the passed revision was valid. Instead we can pass a
prefixtoListObjectswhich is the entire key we are searching for. Rather than downloading them all and searching them clientside we just try to download the one we want.For one of our apps with a fair amount of history (just over 60,000 deployments) this takes the activation down from 835s to 590s (a 30% reduction on my computer and internet connection - probably a bit better from CI!).
There are still two places in the flow where we download the entire list of deployed items but the fix there is less simple...
Related issues
I discuss potential follow up tasks in #120
PR Checklist
People
All maintainers ;)