Repository navigation
fix(connpool): back off between failed dials in OneTimePool.Run - #849
Open
dwisiswant0 wants to merge 1 commit into
Open
dwisiswant0 wants to merge 1 commit into
dwisiswant0 wants to merge 1 commit into
Conversation
Run discarded the dial error and redialed immediately, so a target that refuses connections made the loop spin at tens of millions of dials per second and pin a CPU core. Callers start Run in a goroutine and ignore or only log its result (crypto/jarm, tlsx), so returning the first error would instead leave every Acquire blocked until its own deadline and turn one transient failure into a dead pool. Retry failed dials with an exponential wait from 50ms up to 1s, reset by the next successful dial. The wait selects on the pool context, so Close ends it immediately. The same loop had three related defects. The dialer was invoked while holding the read lock, so Close blocked on the write lock for the whole dial, and it was re-read after the lock was released and could be nil by then. The dialer is now read once and called without the lock. The default dial used net.Dial and ignored the pool context; it now uses DialContext. Finally, a connection that finished dialing after Close had swept the in-flight set was never closed; Run now closes a connection it adds to a done pool. Run now returns the context error wrapped as "connpool: run stopped: ..." instead of bare; the in-repo and known downstream callers ignore it or match it with errors.Is. Fixes: #839 Signed-off-by: Dwi Siswanto <git@dw1.io>
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.
Run discarded the dial error and redialed immediately, so a target that
refuses connections made the loop spin at tens of millions of dials per
second and pin a CPU core. Callers start Run in a goroutine and ignore
or only log its result (crypto/jarm, tlsx), so returning the first
error would instead leave every Acquire blocked until its own deadline
and turn one transient failure into a dead pool.
Retry failed dials with an exponential wait from 50ms up to 1s, reset by
the next successful dial. The wait selects on the pool context, so
Close ends it immediately.
The same loop had three related defects. The dialer was invoked while
holding the read lock, so Close blocked on the write lock for the whole
dial, and it was re-read after the lock was released and could be nil
by then. The dialer is now read once and called without the lock. The
default dial used net.Dial and ignored the pool context; it now uses
DialContext. Finally, a connection that finished dialing after Close had
swept the in-flight set was never closed; Run now closes a connection it
adds to a done pool.
Run now returns the context error wrapped as "connpool: run stopped:
..." instead of bare; the in-repo and known downstream callers ignore it
or match it with errors.Is.
Fixes: #839