Remove crate_downloads
#1693
Merged
Remove crate_downloads
#1693
Conversation
This file contains hidden or 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
sgrif
force-pushed
the
sg-remove-crate-downloads
branch
from
510fee6 to
c1606d6
Compare
|
☔ The latest upstream changes (presumably #1699) made this pull request unmergeable. Please resolve the merge conflicts. |
jtgeibel
approved these changes
Apr 1, 2019
The only place we're ever using `crate_downloads` directly is in a place that we should have been using `recent_crate_downloads`. Once we make that change, `crate_downloads` is only ever used to populate `recent_crate_downloads`, which can just as easily be populated from `version_downloads` instead. Populating it from `version_downloads` is a bit slower since the table is so much larger, and we don't have an index on it which we can use for this query. However, this doesn't really matter since we run this asynchronously. Now that we're on PG 11, I also intend to partition `version_downloads` on date which will make populating the view much faster once again. Note that I have not removed `crate_downloads` yet. That needs to be deployed separately, as it's a destructive change which would break running code, so we can't run that migration until the code which is no longer relying on `crate_downloads` is deployed
No part of our code base relies on this table any longer, so we can just remove it.
|
@bors r=jtgeibel |
sgrif
force-pushed
the
sg-remove-crate-downloads
branch
from
c1606d6 to
f6702c6
Compare
|
📌 Commit f6702c6 has been approved by |
bors
added a commit
that referenced
this pull request
Apr 1, 2019
Remove `crate_downloads`⚠️ This PR contains destructive migrations. The commits in this PR must be deployed separately⚠️ I've been looking through `pg:diagnose` on our new database, and we have two indexes on `crate_downloads` which are either unused or low scan, high write. I'd looked into these indexes previously and determined that we needed them (presumably this was before we introduced `recent_crate_downloads`), but seeing one flagged as unused was surprising so I gave this another look. It turns out the only place we're still using `crate_downloads` directly is a place that should have been using `recent_crate_downloads` anyway. With that code changed to use `recent_crate_downloads`, the only place that `crate_downloads` is used is to populate `recent_crate_downloads`. We can just as easily populate that view from `version_downloads` instead. It turns out that `crate_downloads` and its indexes accounts for 23% (!!!!) of our total database size, so there's a huge incentive to get rid of it. The only downside to doing this is that refreshing `recent_crate_downloads` takes longer if we do it on `version_downloads`. This is both due to the larger table size, and the lack of an index we can use for this (we could add an index to make it faster, but that's not worth it). When based on `version_downloads`, it takes 6 seconds instead of 600ms to populate. Ultimately this doesn't matter, since we do this off the web server anyway. Now that we're on PG 11, I also intend to partition the `version_downloads` table which will get our performance faster than 600ms when done.
|
☀️ Test successful - checks-travis |
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.
I've been looking through
pg:diagnoseon our new database, and we have two indexes oncrate_downloadswhich are either unused or low scan, high write. I'd looked into these indexes previously and determined that we needed them (presumably this was before we introducedrecent_crate_downloads), but seeing one flagged as unused was surprising so I gave this another look.It turns out the only place we're still using
crate_downloadsdirectly is a place that should have been usingrecent_crate_downloadsanyway. With that code changed to userecent_crate_downloads, the only place thatcrate_downloadsis used is to populaterecent_crate_downloads. We can just as easily populate that view fromversion_downloadsinstead.It turns out that
crate_downloadsand its indexes accounts for 23% (!!!!) of our total database size, so there's a huge incentive to get rid of it. The only downside to doing this is that refreshingrecent_crate_downloadstakes longer if we do it onversion_downloads. This is both due to the larger table size, and the lack of an index we can use for this (we could add an index to make it faster, but that's not worth it). When based onversion_downloads, it takes 6 seconds instead of 600ms to populate. Ultimately this doesn't matter, since we do this off the web server anyway. Now that we're on PG 11, I also intend to partition theversion_downloadstable which will get our performance faster than 600ms when done.