Update version_downloads.counted last to minimize lock contention
#2154
Conversation
If we defer the update to `version_downloads.counted` until immediately before the commit, it should eliminate the contention between counting new downloads and propagating previous downloads. Refs: rust-lang#2153 See also: rust-lang#2153 (comment)
|
(rust_highfive has picked a reviewer for you, use r? to override) |
|
Lets do r? @sgrif |
|
I expect this PR will completely eliminate the occurrence of slow download requests. Here's a timeline showing the progress so far:
|
|
I'd be really surprised that this was specifically causing lock contention, since the database will log any locks that are held for long enough to cause a slow request. That said, this change seems completely reasonable on its own |
The example I found in the logs from the 19th (#2153 (comment)) seem to be the only occurrence of the database complaining about this loudly. Postges appears to have a 1 second threshold for generating log output about slow locks. Searching the logs for
Overall, the events logged here are far fewer than the number of slow download requests we've observed. The only explanation I have for that at the moment is that the download endpoint also issues a query joining |
|
I think this will only have a positive impact no matter what
…On Mon, Jan 27, 2020, 6:06 PM Justin Geibel ***@***.***> wrote:
I'd be really surprised that this was specifically causing lock
contention, since the database will log any locks that are held for long
enough to cause a slow request.
The example I found in the logs from the 19th (#2153 (comment)
<#2153 (comment)>)
seem to be the only occurrence of the database complaining about this
loudly. Postges appears to have a 1 second threshold for generating log
output about slow locks. Searching the logs for program:app/postgres
waiting shows a few other interesting bits of log output:
- program:app/postgres.7060, program:app/postgres.11521, and
program:app/postgres.5638 - slow at inserting into version_downloads
- program:app/postgres.10814 - slow at inserting into emails 😕
- program:app/postgres.19904 - several interesting messages at once,
but this appears to align with a deploy so maybe a migration locked
something for a moment.
Overall, the events logged here are far fewer than the number of slow
download requests we've observed. The only explanation I have for that at
the moment is that the download endpoint also issues a query joining
versions and crates. Maybe slow requests end up spending some extra time
in each query, but not enough to log either query as >1 second by postgres.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#2154?email_source=notifications&email_token=AALVMK3FNKQPIHHAHU4CQADQ76AINA5CNFSM4KKQ37VKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKBVEAQ#issuecomment-579031554>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AALVMK44FMUZG7JVJ45BUKTQ76AINANCNFSM4KKQ37VA>
.
|
|
I should be able to deploy this shortly. @bors r=sgrif |
|
📌 Commit 97e9997 has been approved by |
Update `version_downloads.counted` last to minimize lock contention If we defer the update to `version_downloads.counted` until immediately before the commit, it should eliminate the contention between counting new downloads and propagating previous downloads. Refs: #2153 See also: #2153 (comment)
|
☀️ Test successful - checks-travis |
If we defer the update to
version_downloads.counteduntil immediatelybefore the commit, it should eliminate the contention between counting
new downloads and propagating previous downloads.
Refs: #2153
See also: #2153 (comment)