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.

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

fix(enginenetx): compute zero time when we start dialing #1594

Merged
merged 135 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
135 commits
Select commit Hold shift + click to select a range
80b5e1f
doc(enginenetx): improve documentation
bassosimone Apr 15, 2024
62c3917
fix(enginenetx): mix bridges and DNS tactics
bassosimone Apr 15, 2024
0d2b0f2
fix: update test name and add comment
bassosimone Apr 15, 2024
aee17cf
feat: test for the mixed policies case
bassosimone Apr 15, 2024
7576fc7
feat(enginenetx): add support for filtering tactics
bassosimone Apr 15, 2024
8120c06
refactor: rename function
bassosimone Apr 15, 2024
3207255
feat: improve TCP connect statistics
bassosimone Apr 15, 2024
5618b72
fix previous
bassosimone Apr 15, 2024
2153e36
feat: start to prepare for filtering endpoints
bassosimone Apr 16, 2024
e5cdbb0
ongoing work while documenting and clarifying
bassosimone Apr 16, 2024
089f70b
x
bassosimone Apr 16, 2024
20e71e8
[ci skip]
bassosimone Apr 16, 2024
b5b2e49
[ci skip]
bassosimone Apr 16, 2024
94eb284
[ci skip]
bassosimone Apr 16, 2024
90601c6
[ci skip]
bassosimone Apr 16, 2024
ef8fdfe
[ci skip]
bassosimone Apr 16, 2024
aa65cb7
[ci skip]
bassosimone Apr 16, 2024
28a6265
[ci skip]
bassosimone Apr 16, 2024
f452208
[ci skip]
bassosimone Apr 16, 2024
119e610
[ci skip]
bassosimone Apr 16, 2024
f3fb1dd
[ci skip]
bassosimone Apr 16, 2024
ce6ec84
[ci skip]
bassosimone Apr 16, 2024
4f63b60
[ci skip]
bassosimone Apr 16, 2024
45e655c
x
bassosimone Apr 16, 2024
8e2a1f3
x
bassosimone Apr 16, 2024
e2aed07
x
bassosimone Apr 16, 2024
08fbf48
[ci skip]
bassosimone Apr 16, 2024
7c6ab4b
[ci skip]
bassosimone Apr 16, 2024
c843241
[ci skip]
bassosimone Apr 16, 2024
7f16577
[ci skip]
bassosimone Apr 16, 2024
4b0c768
[ci skip]
bassosimone Apr 16, 2024
7ea130d
[ci skip]
bassosimone Apr 16, 2024
1cbc109
[ci skip]
bassosimone Apr 16, 2024
7edfbb8
[ci skip]
bassosimone Apr 16, 2024
21f9b90
[ci skip]
bassosimone Apr 16, 2024
8e5fee9
[ci skip]
bassosimone Apr 16, 2024
4f8cf91
[ci skip]
bassosimone Apr 16, 2024
436fb50
[ci skip]
bassosimone Apr 16, 2024
b6aebc2
[ci skip]
bassosimone Apr 16, 2024
77b03bd
[ci skip]
bassosimone Apr 16, 2024
f565e68
[ci skip]
bassosimone Apr 16, 2024
e9eee04
[ci skip]
bassosimone Apr 16, 2024
393968d
[ci skip]
bassosimone Apr 16, 2024
665b961
[ci skip]
bassosimone Apr 16, 2024
fb651c7
[ci skip]
bassosimone Apr 16, 2024
cb0dbfc
[ci skip]
bassosimone Apr 16, 2024
bfc0a1d
[ci skip]
bassosimone Apr 16, 2024
02660de
[ci skip]
bassosimone Apr 16, 2024
f707616
[ci skip]
bassosimone Apr 16, 2024
492ab69
[ci skip]
bassosimone Apr 16, 2024
3b63fbd
[ci skip]
bassosimone Apr 16, 2024
0c06b53
[ci skip]
bassosimone Apr 16, 2024
8691606
[ci skip]
bassosimone Apr 16, 2024
20f800a
[ci skip]
bassosimone Apr 16, 2024
e02c5d4
[ci skip]
bassosimone Apr 16, 2024
dd128d8
[ci skip]
bassosimone Apr 16, 2024
8c5bc60
[ci skip]
bassosimone Apr 16, 2024
6e47258
[ci skip]
bassosimone Apr 16, 2024
018fec4
[ci skip]
bassosimone Apr 16, 2024
c834599
[ci skip]
bassosimone Apr 16, 2024
ccd26c4
[ci skip]
bassosimone Apr 16, 2024
0b32b47
[ci skip]
bassosimone Apr 16, 2024
6066a7f
[ci skip]
bassosimone Apr 16, 2024
6c83c25
[ci skip]
bassosimone Apr 16, 2024
19fc4b5
[ci skip]
bassosimone Apr 16, 2024
856d261
[ci skip]
bassosimone Apr 16, 2024
2b7a881
[ci skip]
bassosimone Apr 16, 2024
b7327e2
[ci skip]
bassosimone Apr 16, 2024
3399fec
[ci skip]
bassosimone Apr 16, 2024
baef14f
[ci skip]
bassosimone Apr 16, 2024
15d28f2
[ci skip]
bassosimone Apr 16, 2024
0baaf9b
[ci skip]
bassosimone Apr 16, 2024
49bbf25
[ci skip]
bassosimone Apr 16, 2024
d739ddd
[ci skip]
bassosimone Apr 16, 2024
4896f68
[ci skip]
bassosimone Apr 16, 2024
8bdbbaf
x
bassosimone Apr 16, 2024
4e3a8af
the design document should now be good
bassosimone Apr 16, 2024
fd81cf7
x
bassosimone Apr 16, 2024
8a93533
x
bassosimone Apr 16, 2024
2949035
remove more code that we probably don't need
bassosimone Apr 16, 2024
c075625
[ci skip]
bassosimone Apr 17, 2024
8d14587
[ci skip]
bassosimone Apr 17, 2024
7a2a360
[ci skip]
bassosimone Apr 17, 2024
dfed510
[ci skip]
bassosimone Apr 17, 2024
be41fa9
[ci skip]
bassosimone Apr 17, 2024
5f7302a
[ci skip]
bassosimone Apr 17, 2024
7443710
[ci skip]
bassosimone Apr 17, 2024
2944ea0
[ci skip]
bassosimone Apr 17, 2024
43c1e7c
[ci skip]
bassosimone Apr 17, 2024
d6159d6
[ci skip]
bassosimone Apr 17, 2024
dcf4a03
[ci skip]
bassosimone Apr 17, 2024
f037eb3
[ci skip]
bassosimone Apr 17, 2024
586c450
[ci skip]
bassosimone Apr 17, 2024
de31fd5
[ci skip]
bassosimone Apr 17, 2024
8a3eef7
[ci skip]
bassosimone Apr 17, 2024
697f6b3
[ci skip]
bassosimone Apr 17, 2024
e652147
[ci skip]
bassosimone Apr 17, 2024
4baca70
[ci skip]
bassosimone Apr 17, 2024
fa1c237
[ci skip]
bassosimone Apr 17, 2024
75eb2df
Merge branch 'master' into issue/2704
bassosimone Apr 17, 2024
fd9c568
Merge branch 'master' into issue/2704
bassosimone Apr 17, 2024
d0f721a
x
bassosimone Apr 17, 2024
7058b7d
x
bassosimone Apr 17, 2024
4e628db
x
bassosimone Apr 17, 2024
3b0e110
x
bassosimone Apr 17, 2024
72e1336
Merge branch 'master' into issue/2704
bassosimone Apr 17, 2024
32178c6
x
bassosimone Apr 17, 2024
e7c7541
x
bassosimone Apr 17, 2024
f16784c
x
bassosimone Apr 17, 2024
28bcd5f
[ci skip]
bassosimone Apr 17, 2024
da06bbb
[ci skip]
bassosimone Apr 17, 2024
67ae4d9
[ci skip]
bassosimone Apr 17, 2024
02ee95d
[ci skip]
bassosimone Apr 17, 2024
3877575
fix: adapt test after remix change
bassosimone Apr 18, 2024
af87c6e
doc: add table of contents
bassosimone Apr 18, 2024
1a50266
doc: rename the goals section to be more clear
bassosimone Apr 18, 2024
139f10c
Update internal/enginenetx/DESIGN.md
bassosimone Apr 26, 2024
1d8a786
Update internal/enginenetx/DESIGN.md
bassosimone Apr 26, 2024
a161fa1
Merge branch 'master' into issue/2704
bassosimone Apr 26, 2024
bffc811
Apply suggestions from @hellais
bassosimone May 7, 2024
e9765df
Merge branch 'master' into issue/2704
bassosimone May 7, 2024
9d2c47f
feat: allow chaining policies after the DNS
bassosimone May 8, 2024
f7509d9
feat: implement the mix-with-interleaving policy
bassosimone May 8, 2024
ab3926e
fix: we actually need to get rid of fallbacks
bassosimone May 8, 2024
d4f60b1
refactor(bridgespolicy.go): use free functions
bassosimone May 8, 2024
b0f88bd
feat: introduce fallback-less v2 policies
bassosimone May 8, 2024
35d667f
Shove all the changes I have together
bassosimone May 8, 2024
2582614
chore: add more debugging info
bassosimone May 9, 2024
ba05f4c
x
bassosimone May 9, 2024
1e818f4
x
bassosimone May 9, 2024
f61e96b
Merge branch 'master' into issue/2704
bassosimone May 9, 2024
93954bd
Merge branch 'master' into issue/2704
bassosimone May 10, 2024
25e2eb8
x
bassosimone May 10, 2024
83ec4da
fix(enginenetx): compute zero time when we start dialing
bassosimone May 10, 2024
8a5d0b7
x
bassosimone May 10, 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
3 changes: 3 additions & 0 deletions internal/enginenetx/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ func filterAssignInitialDelays(input <-chan *httpsDialerTactic) <-chan *httpsDia

index := 0
for tx := range input {
// TODO(bassosimone): what do we do now about the user configured
// initial delays? Should we declare them as deprecated?

// rewrite the delays
tx.InitialDelay = happyEyeballsDelay(index)
index++
Expand Down
51 changes: 40 additions & 11 deletions internal/enginenetx/httpsdialer.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"errors"
"fmt"
"net"
"sync"
"sync/atomic"
"time"

Expand Down Expand Up @@ -206,24 +207,22 @@ func (hd *httpsDialer) DialTLSContext(ctx context.Context, network string, endpo
// pattern used by `./internal/httpclientx` to perform attempts faster
// in case there is an initial early failure.

// TODO(bassosimone): the algorithm to filter and assign initial
// delays is broken because, if the DNS runs for more than one
// second, then several policies will immediately be due. We should
// probably use a better strategy that takes as the zero the time
// when the first dialing policy becomes available.

// We need a cancellable context to interrupt the tactics emitter early when we
// immediately get a valid response and we don't need to use other tactics.
ctx, cancel := context.WithCancel(ctx)
defer cancel()

// Create structure for computing the zero dialing time once during
// the first dial, so that subsequent attempts use happy eyeballs based
// on the moment in which we tried the first dial.
t0 := &httpsDialerWorkerZeroTime{}

// The emitter will emit tactics and then close the channel when done. We spawn 16 workers
// that handle tactics in parallel and post results on the collector channel.
emitter := httpsDialerFilterTactics(hd.policy.LookupTactics(ctx, hostname, port))
collector := make(chan *httpsDialerErrorOrConn)
joiner := make(chan any)
const parallelism = 16
t0 := time.Now()
for idx := 0; idx < parallelism; idx++ {
go hd.worker(ctx, joiner, emitter, t0, collector)
}
Expand Down Expand Up @@ -257,6 +256,36 @@ func (hd *httpsDialer) DialTLSContext(ctx context.Context, network string, endpo
return httpsDialerReduceResult(connv, errorv)
}

// httpsDialerWorkerZeroTime contains the zero time used when dialing. We set this
// zero time when we start the first dialing attempt, such that subsequent attempts
// are correctly spaced starting from such a zero time.
//
// A previous approach was that we were taking the zero time when we started
// getting tactics, but this approach was wrong, because it caused several tactics
// to be ready, when the DNS lookup was slow.
//
// The zero value of this structure is ready to use.
type httpsDialerWorkerZeroTime struct {
// mu provides mutual exclusion.
mu sync.Mutex

// t is the zero time.
t time.Time
}

// Get returns the zero dialing time. The first invocation of this method
// saves the zero dialing time and subsquent invocations just return it.
//
// This method is safe to be called concurrently by goroutines.
func (t0 *httpsDialerWorkerZeroTime) Get() time.Time {
defer t0.mu.Unlock()
t0.mu.Lock()
if t0.t.IsZero() {
t0.t = time.Now()
}
return t0.t
}

// httpsDialerFilterTactics filters the tactics to:
//
// 1. be paranoid and filter out nil tactics if any;
Expand Down Expand Up @@ -297,7 +326,7 @@ func (hd *httpsDialer) worker(
ctx context.Context,
joiner chan<- any,
reader <-chan *httpsDialerTactic,
t0 time.Time,
t0 *httpsDialerWorkerZeroTime,
writer chan<- *httpsDialerErrorOrConn,
) {
// let the parent know that we terminated
Expand All @@ -321,7 +350,7 @@ func (hd *httpsDialer) worker(
func (hd *httpsDialer) dialTLS(
ctx context.Context,
logger model.Logger,
t0 time.Time,
t0 *httpsDialerWorkerZeroTime,
tactic *httpsDialerTactic,
) (model.TLSConn, error) {
// honor happy-eyeballs delays and wait for the tactic to be ready to run
Expand Down Expand Up @@ -398,10 +427,10 @@ func (hd *httpsDialer) dialTLS(
// return the context error if the context expires.
func httpsDialerTacticWaitReady(
ctx context.Context,
t0 time.Time,
t0 *httpsDialerWorkerZeroTime,
tactic *httpsDialerTactic,
) error {
deadline := t0.Add(tactic.InitialDelay)
deadline := t0.Get().Add(tactic.InitialDelay)
delta := time.Until(deadline)
if delta <= 0 {
return nil
Expand Down
Loading