Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop old dialers when we get a new proxy config #1445

Merged
merged 4 commits into from
Nov 22, 2024
Merged

Stop old dialers when we get a new proxy config #1445

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c79d88f
Stop old dialers when we get a new proxy config
myleshorton Nov 20, 2024
1a623af
Comment and interface enforcement
myleshorton Nov 20, 2024
99d8a3e
Code review fixes
myleshorton Nov 21, 2024
034eed0
tweak select
myleshorton Nov 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 10 additions & 11 deletions dialer/fastconnect.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func newFastConnectDialer(opts *Options, next func(opts *Options, existing Diale
opts: opts,
next: next,
topDialer: protectedDialer{},
stopCh: make(chan struct{}),
stopCh: make(chan struct{}, 10),
}
}

Expand Down Expand Up @@ -113,21 +113,21 @@ func (fcd *fastConnectDialer) connectAll(dialers []ProxyDialer) {
return
}
log.Debugf("Dialing all dialers in parallel %#v", dialers)
outerLoop:
for {
// Loop until we're connected
if len(fcd.connected.dialers) < 2 {
fcd.parallelDial(dialers)
// Add jitter to avoid thundering herd
time.Sleep(time.Duration(rand.Intn(4000)) * time.Millisecond)
} else {
break
}
select {
case <-fcd.stopCh:
log.Debug("Stopping parallel dialing")
return
default:
// Loop until we're connected
if len(fcd.connected.dialers) < 2 {
fcd.parallelDial(dialers)
// Add jitter to avoid thundering herd
time.Sleep(time.Duration(rand.Intn(4000)) * time.Millisecond)
} else {
break outerLoop
}

}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not big deal since the goroutine running connectAll would only hang around for a few seconds, at most, but, if you add another case and use time.After instead of time.Sleep here, the goroutine will exit as soon as it's canceled and be cleaned up. Just FYI.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good point -- fixed!

}
// At this point, we've tried all of the dialers, and they've all either
Expand All @@ -138,7 +138,6 @@ outerLoop:
nextOpts := fcd.opts.Clone()
nextOpts.Dialers = fcd.connected.proxyDialers()
fcd.next(nextOpts, fcd)

}

func (fcd *fastConnectDialer) parallelDial(dialers []ProxyDialer) {
Expand Down
1 change: 0 additions & 1 deletion dialer/two_phase_dialer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ func NewTwoPhaseDialer(opts *Options, next func(opts *Options, existing Dialer)
// This is where we move to the second dialer.
nextDialer := next(dialerOpts, existing)
tpd.activeDialer.set(nextDialer)
existing.Close()
return nextDialer
})

Expand Down
Loading