shorten time we block db connections in requests#3225
Open
syphar wants to merge 1 commit intorust-lang:mainfrom
Open
shorten time we block db connections in requests#3225syphar wants to merge 1 commit intorust-lang:mainfrom
syphar wants to merge 1 commit intorust-lang:mainfrom
Conversation
Collaborator
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
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
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.
Now where the new infra migration comes more and more near, I did another test of how the webserver reacts when it sees an empty archive index directory, which is what will happen after each deploy and web server container restart in the future.
It again took the server down, but now I had a look at the logs and saw error messages that requests couldn't get a database connection (or getting the connection took long).
This lead me to the actual issue with the empty archive index:
In most handlers we just fetch a database connection from the pool for the whole duration of the request. But: in typical rustdoc requests, most of the time is spend in the request to S3. And when the archive index cache is empty, the time to fetch the index comes on top (and some internal locking).
This PR updates all handlers that use storage, trying to hand back the connection earlier, so it gets freed for other requests.
So in the situation where S3 is slow / we don't have a cache, the DB pool won't be a problem any more.
Having this fixed also opens up the possibility to clean up index files when the disk is full, which is helpful for the web containers.