Allow database timeouts to be configured #1344
Merged
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
We're consistently seeing the endpoint for crate downloads H12. The logs are showing that we are *eventually* finishing without error, it's just taking >30s, so Heroku is killing the connection. Unfortunately, this means that we don't have visibility into *why* it's timing out. It's either waiting for a long time for a DB connection, updating the `version_downloads` table (my guess), or talking to S3. If it's database related, this will turn those requests into actual errors. This setting will increase our error rates, as we're seeing the 99th percentile push up to 5s at some points, and we do have requests that take 15s to complete (but are completing). Eventually I think we should be able to consider 10s response times unacceptable, but for now I'm only going to leave this at 10s for long enough for me to see *what* is failing, at which point I will set it back to 30s until we no longer see requests pushing 10s regularly.
ashleygwilliams
approved these changes
Apr 13, 2018
| @@ -71,11 +72,19 @@ impl App { | |||
| _ => 1, | |||
| }; | |||
|
|
|||
| let db_connection_timeout = match (env::var("DB_TIMEOUT"), config.env) { | |||
| (Ok(num), _) => num.parse().expect("couldn't parse DB_TIMEOUT"), | |||
There was a problem hiding this comment.
in the long run we might want to make this error message a little better but it's fine for now
|
bors: r+ |
bors-voyager bot
added a commit
that referenced
this pull request
Apr 13, 2018
1344: Allow database timeouts to be configured r=ashleygwilliams We're consistently seeing the endpoint for crate downloads H12. The logs are showing that we are *eventually* finishing without error, it's just taking >30s, so Heroku is killing the connection. Unfortunately, this means that we don't have visibility into *why* it's timing out. It's either waiting for a long time for a DB connection, updating the `version_downloads` table (my guess), or talking to S3. If it's database related, this will turn those requests into actual errors. This setting will increase our error rates, as we're seeing the 99th percentile push up to 5s at some points, and we do have requests that take 15s to complete (but are completing). Eventually I think we should be able to consider 10s response times unacceptable, but for now I'm only going to leave this at 10s for long enough for me to see *what* is failing, at which point I will set it back to 30s until we no longer see requests pushing 10s regularly.
Build succeeded |
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.
We're consistently seeing the endpoint for crate downloads H12. The logs
are showing that we are eventually finishing without error, it's just
taking >30s, so Heroku is killing the connection.
Unfortunately, this means that we don't have visibility into why it's
timing out. It's either waiting for a long time for a DB connection,
updating the
version_downloadstable (my guess), or talking to S3. Ifit's database related, this will turn those requests into actual errors.
This setting will increase our error rates, as we're seeing the 99th
percentile push up to 5s at some points, and we do have requests that
take 15s to complete (but are completing). Eventually I think we should
be able to consider 10s response times unacceptable, but for now I'm
only going to leave this at 10s for long enough for me to see what is
failing, at which point I will set it back to 30s until we no longer see
requests pushing 10s regularly.