fix(pool): two fixes for closed connection handling#3764
Merged
Conversation
Member
|
@cxljs other than the notes from the CI, I don't have anything against this. We haven't tested the new metrics collector with sentinel (where this reason will be used), have you tested it? |
Contributor
Author
I saw the CI comment. Since these past few days are a special holiday in my country, I haven’t had time to address it yet. I’ll take care of it as soon as possible.
Thanks for the reminder. I’ll test it. |
03cf31f to
efc92ea
Compare
1f9f39f to
8f52534
Compare
ndyakov
previously approved these changes
Apr 20, 2026
7bdd70a to
63846e9
Compare
7f955e7 to
fb7aa18
Compare
97d30b4 to
e56de38
Compare
e5b8407 to
d529d4b
Compare
d9e99f4 to
d978e6d
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Reviewed by Cursor Bugbot for commit d978e6d. Configure here.
a76d7fb to
9ecde4d
Compare
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
2 participants
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.
Closes #3703
In putConn(): prevent CLOSED state conns from being added to idleConns
In Filter(): ensure closed conns are removed from idleConns during failover
Note
Medium Risk
Changes connection pool close/removal behavior and metrics emission paths, which can affect connection lifecycle correctness under concurrency (especially during failover). Risk is moderated by added targeted sentinel failover tests and mostly localized changes to pooling/metrics codepaths.
Overview
Improves closed-connection handling in the pool, especially around failover and concurrent closes.
Conn.Close()is now idempotent and ensuresonCloseruns at most once. The pool introduces a per-conn atomiccloseReason(plus newCloseReasonFailover) so an in-use conn closed by another goroutine can later emit the correctConnectionClosedmetric reason when it’s returned.ConnPool.Filter()is updated to properly distinguish idle vs in-use conns: idle conns are removed fromidleConns/pool and closed immediately with failover metrics, while in-use conns are marked withcloseReasonand closed without corrupting idle lists. Metrics emission forCloseConnis refactored intorecordConnectionMetrics().Adds a new serial/ordered sentinel failover test that asserts exactly one
failoverclose metric across differentMaxActiveConnssettings, and marks theUniversalClienttest suite asSerialto reduce inter-test interference.Reviewed by Cursor Bugbot for commit 7500d29. Bugbot is set up for automated code reviews on this repo. Configure here.