Improve db_pools init: do not crash if DB unavailable during startup (sqlx) #2932
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
## Why?
When using `Pool::connect[_with]`, sqlx attempts to connect to the given
database immediately, and the fairing will fail if there are any
problems in that attempt (beyond obvious configuration problems that are
found before hitting the network), e.g.:
- the database is unavailable; or
- the username/password is incorrect; or
- the ssl configuration is invalid; or
- any other connection issue.
There are a few pros and cons to this approach:
Pros:
- In development, configuration errors are surfaced slightly faster
Cons:
- Databases are expected to be unavailable sometimes. It does not
normally crash a server if one becomes unavailable after startup,
so why should it prevent a server from starting at all? See
[deadpool's justification]{https://docs.rs/deadpool} for not crashing.
- In production/testing, slower to debug configuration or networking
errors as your edit-test loop now involves restarting an application
rather than refreshing a page or trying a request again.
- Causes database or configuration issues to appear as "failed
deployments" in standard deployment scenarios.
- Introduces hard ordering constraints on operator actions during
database recovery, requiring reboots to follow a functioning database
or applications not to be restarted at certain times
## Effect of change
The sqlx backend now behaves like the deadpool backend: no connection
issues are surfaced during startup. You will not see them until you
attempt to get a connection from the pool. That means rocket will launch
and you can find problems like these in smoke tests.
|
Test failures are same as master |
cormacrelf
changed the title
the10thWiz
approved these changes
May 4, 2025
There was a problem hiding this comment.
I think this is a good change. More generally, we should consider also attempting to detect database issues and report them, even if the server continues running.
This would likely take the form of spawning a task to pull a connection from the pool, and potentially run some kind of basic sql (e.g. select 1;). We could then provide an error log indicating that the db is currently inaccessible.
- Fixes testbench mtls test
|
Merged in 64ef4e9 |
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.
Fixes #2931
Why?
When using
Pool::connect[_with], sqlx attempts to connect to the givendatabase immediately, and the fairing will fail if there are any
problems in that attempt (beyond obvious configuration problems that are
found before hitting the network), e.g.:
There are a few pros and cons to this approach:
Pros:
Cons:
normally crash a server if one becomes unavailable after startup,
so why should it prevent a server from starting at all? See
[deadpool's justification]{https://docs.rs/deadpool} for not crashing.
errors as your edit-test loop now involves restarting an application
rather than refreshing a page or trying a request again.
deployments" in standard deployment scenarios.
database recovery, requiring reboots to follow a functioning database
or applications not to be restarted at certain times
Effect of change
The sqlx backend now behaves like the deadpool backend: no connection
issues are surfaced during startup. You will not see them until you
attempt to get a connection from the pool. That means rocket will launch
and you can find problems like these in smoke tests.