fix(pool): bound the entire connection return operation - #4407
Open
KimSoungRyoul wants to merge 1 commit into
Open
KimSoungRyoul wants to merge 1 commit into
KimSoungRyoul wants to merge 1 commit into
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
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.
Hi @abonander, I'm using SQLx 0.9.0 and have encountered persistent connection pool timeouts in production. Restarting the affected instance restored service, which led me to investigate #4349.
A local PostgreSQL regression test reproduces the permit leak in #4349: when a connection stops responding,
return_to_pool()can wait indefinitely while holding its pool permit. Repeated failures exhaust the pool, and subsequent acquisitions time out because the permits are never released.Following the review on #4350, this bounds the entire return operation with one five-second timeout, including callbacks, ping, and shutdown. On expiry, the return future is dropped, discarding the connection and releasing its permit so the pool can open a replacement.
min_connectionsmaintenance still runs. Long-runningafter_releasecallbacks are also subject to this deadline.Would you consider this fix for a 0.9.x patch release before #3582 lands?
Pool exhaustion without recovery is a serious problem for a prod service, and I'm proposing this PR to help address it. I can also contribute the change to #4350 if preferred.
Tests verify permit recovery after stalled pings and callbacks, connection reuse, and minimum-connection replenishment. All pass; the timeout cases fail without the fix. Shutdown stalls aren't separately tested.
Thanks for maintaining sqlx!