Skip to content

Commit

Permalink
Add a workaround for HTTP/2 connection reuse
Browse files Browse the repository at this point in the history
Sometimes we're seeing HTTP/2 connections be reused for up to ~16
minutes despite being completely defunct -- i.e. we're receiving no
response from the remote end.

We believe that this is in part a result of Go bug

  golang/go#59690

in which failed connections are not correctly discarded under certain
circumstances.

This commit enables the http2 transport's `ReadIdleTimeout` function,
which will send an h2 "ping" frame on any connection that's been idle
longer than the value of `ReadIdleTimeout`. If it doesn't hear back in
`PingTimeout`, it will throw away the connection. This seems like a
reasonable thing to leave on in general.
  • Loading branch information
nickstenning committed Nov 27, 2023
1 parent e251d3c commit 1389694
Showing 1 changed file with 28 additions and 0 deletions.
28 changes: 28 additions & 0 deletions httpclient/httpclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
"go.opentelemetry.io/otel/propagation"
"golang.org/x/net/http2"
)

const ConnectTimeout = 5 * time.Second
Expand Down Expand Up @@ -72,6 +73,7 @@ func DefaultPooledTransport() *http.Transport {
ExpectContinueTimeout: 1 * time.Second,
MaxIdleConnsPerHost: runtime.GOMAXPROCS(0) + 1,
}
configureHTTP2(transport)
return transport
}

Expand All @@ -93,3 +95,29 @@ func DefaultPooledClient() *http.Client {
Transport: DefaultPooledRoundTripper(),
}
}

func configureHTTP2(t *http.Transport) {
h2t, err := http2.ConfigureTransports(t)
if err != nil {
// ConfigureTransports should only ever return an error if the transport
// passed in has already been configured for http2, which shouldn't be
// possible for us.
panic(err)
}

// Send a ping frame on any connection that's been idle for more than 10
// seconds.
//
// The default is to never do this. We set it primarily as a workaround for
//
// https://github.com/golang/go/issues/59690
//
// where a connection that goes AWOL will not be correctly terminated and
// removed from the connection pool under certain circumstances. Together
// `ReadIdleTimeout` and `PingTimeout` should ensure that we remove defunct
// connections in ~20 seconds.
h2t.ReadIdleTimeout = 10 * time.Second
// Give the other end 10 seconds to respond. If we don't hear back, we'll
// close the connection.
h2t.PingTimeout = 10 * time.Second
}

0 comments on commit 1389694

Please sign in to comment.