-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
x/net/http2: requests experience excessive time delays when encountering network errors #67665
Comments
Change https://go.dev/cl/588595 mentions this issue: |
@Taction Do you have a minimal example that reproduces this issue? |
CC @neild |
@hardikmodha Sorry for the late reply. The scenario I've encountered is when there is a network issue, some requests consume more time than the request timeout + dial timeout to finish. I created a test to simulate this situation(also committed in my pr). http2/clientconn_test.go // TestConnectTimeout tests that a request does not exceed request timeout + dial timeout
func TestConnectTimeout(t *testing.T) {
tr := &Transport{
DialTLSContext: func(ctx context.Context, network, addr string, cfg *tls.Config) (net.Conn, error) {
// mock a net dialler with 1s timeout, encountering network issue
// keeping dialing until timeout
var dialer = net.Dialer{Timeout: time.Duration(-1)}
select {
case <-time.After(time.Second):
case <-ctx.Done():
}
return dialer.DialContext(ctx, network, addr)
},
AllowHTTP: true,
}
var sg sync.WaitGroup
parentCtx, cancel := context.WithCancel(context.Background())
defer cancel()
for j := 0; j < 2; j++ {
sg.Add(1)
go func() {
for i := 0; i < 10000; i++ {
sg.Add(1)
go func() {
ctx, _ := context.WithTimeout(parentCtx, time.Second)
req, err := http.NewRequestWithContext(ctx, "GET", "http://127.0.0.1:80", nil)
if err != nil {
t.Errorf("NewRequest: %v", err)
}
start := time.Now()
tr.RoundTrip(req)
duration := time.Since(start)
// duration should not exceed request timeout + dial timeout
if duration > 2*time.Second {
t.Errorf("RoundTrip took %s; want <2s", duration.String())
}
sg.Done()
}()
time.Sleep(1 * time.Millisecond)
}
sg.Done()
}()
}
sg.Wait()
} |
Change https://go.dev/cl/591276 mentions this issue: |
If the client loses its connection to the server, and multiple requests are sent. There would be only one dial start connecting to the server. If the dial timeout due to network issue, only the first request who creates the dial would be finished, and the other requests would continue trying to connect to the server even if their context has been canceled due to timeout.
For getClientConn multi request would creat one call to dial server.
For shouldRetryDial function, even if the request's context has been canceled, the result can be true.
The text was updated successfully, but these errors were encountered: