From 618c24c65a2bb235fc7d6728f755a54dad99f172 Mon Sep 17 00:00:00 2001 From: Joel Hendrix Date: Wed, 30 Aug 2023 14:27:09 -0700 Subject: [PATCH 1/9] Enable health checks for HTTP/2 connections If the health check fails, the connection will be closed and a new one created. --- sdk/azcore/CHANGELOG.md | 2 ++ sdk/azcore/runtime/transport_default_http_client.go | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/sdk/azcore/CHANGELOG.md b/sdk/azcore/CHANGELOG.md index 45685ef4534b..2fa5fe9ed780 100644 --- a/sdk/azcore/CHANGELOG.md +++ b/sdk/azcore/CHANGELOG.md @@ -57,6 +57,8 @@ ### Bugs Fixed * Suppress creating spans for nested SDK API calls. The HTTP span will be a child of the outer API span. +* Fix default HTTP transport to work in WASM modules. +* Enable HTTP connection health checks for HTTP/2 connections. ### Other Changes diff --git a/sdk/azcore/runtime/transport_default_http_client.go b/sdk/azcore/runtime/transport_default_http_client.go index 589d09f2c0d0..7591ae72df7a 100644 --- a/sdk/azcore/runtime/transport_default_http_client.go +++ b/sdk/azcore/runtime/transport_default_http_client.go @@ -11,6 +11,8 @@ import ( "net" "net/http" "time" + + "golang.org/x/net/http2" ) var defaultHTTPClient *http.Client @@ -32,6 +34,12 @@ func init() { Renegotiation: tls.RenegotiateFreelyAsClient, }, } + if http2Transport, err := http2.ConfigureTransports(defaultTransport); err == nil { + // if the connection has been idle for 10 seconds, send a ping frame for a health check + http2Transport.ReadIdleTimeout = 10 * time.Second + // if there's no response to the ping within 2 seconds, close the connection + http2Transport.PingTimeout = 2 * time.Second + } defaultHTTPClient = &http.Client{ Transport: defaultTransport, } From c43d7667db5d5500ebffc0af38eb2296019cbd8a Mon Sep 17 00:00:00 2001 From: Joel Hendrix Date: Thu, 31 Aug 2023 08:15:07 -0700 Subject: [PATCH 2/9] perf tests use live azcore --- sdk/azcore/testdata/perf/go.mod | 5 ++++- sdk/azcore/testdata/perf/go.sum | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/sdk/azcore/testdata/perf/go.mod b/sdk/azcore/testdata/perf/go.mod index 483ccefcb365..d6b50126d686 100644 --- a/sdk/azcore/testdata/perf/go.mod +++ b/sdk/azcore/testdata/perf/go.mod @@ -7,6 +7,9 @@ require ( github.com/Azure/azure-sdk-for-go/sdk/internal v1.3.0 ) -require golang.org/x/text v0.13.0 // indirect +require ( + golang.org/x/net v0.15.0 // indirect + golang.org/x/text v0.13.0 // indirect +) replace github.com/Azure/azure-sdk-for-go/sdk/azcore => ../../ diff --git a/sdk/azcore/testdata/perf/go.sum b/sdk/azcore/testdata/perf/go.sum index 068f240e7151..1695478d9e5e 100644 --- a/sdk/azcore/testdata/perf/go.sum +++ b/sdk/azcore/testdata/perf/go.sum @@ -3,6 +3,8 @@ github.com/Azure/azure-sdk-for-go/sdk/internal v1.3.0/go.mod h1:okt5dMMTOFjX/aov github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= +golang.org/x/net v0.15.0 h1:ugBLEUaxABaB5AJqW9enI0ACdci2RUd4eP51NTBvuJ8= +golang.org/x/net v0.15.0/go.mod h1:idbUs1IY1+zTqbi8yxTbhexhEEk5ur9LInksu6HrEpk= golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k= golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= From d82c2db52bd1970bcf0a40159446b929e499f58a Mon Sep 17 00:00:00 2001 From: Joel Hendrix Date: Thu, 31 Aug 2023 09:32:42 -0700 Subject: [PATCH 3/9] increase MaxIdleConnsPerHost from default of 2 --- sdk/azcore/runtime/transport_default_http_client.go | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/azcore/runtime/transport_default_http_client.go b/sdk/azcore/runtime/transport_default_http_client.go index 7591ae72df7a..71e3b04fdc33 100644 --- a/sdk/azcore/runtime/transport_default_http_client.go +++ b/sdk/azcore/runtime/transport_default_http_client.go @@ -26,6 +26,7 @@ func init() { }), ForceAttemptHTTP2: true, MaxIdleConns: 100, + MaxIdleConnsPerHost: 10, IdleConnTimeout: 90 * time.Second, TLSHandshakeTimeout: 10 * time.Second, ExpectContinueTimeout: 1 * time.Second, From ad62e9fcbd50f026ea473351a206d58eef365cd3 Mon Sep 17 00:00:00 2001 From: Joel Hendrix Date: Tue, 5 Sep 2023 12:40:24 -0700 Subject: [PATCH 4/9] reword changelog entry --- sdk/azcore/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/azcore/CHANGELOG.md b/sdk/azcore/CHANGELOG.md index 2fa5fe9ed780..f42facf37dc4 100644 --- a/sdk/azcore/CHANGELOG.md +++ b/sdk/azcore/CHANGELOG.md @@ -58,7 +58,7 @@ * Suppress creating spans for nested SDK API calls. The HTTP span will be a child of the outer API span. * Fix default HTTP transport to work in WASM modules. -* Enable HTTP connection health checks for HTTP/2 connections. +* Fixed an issue that could cause an HTTP/2 request to hang when the TCP connection becomes unresponsive. ### Other Changes From c6329dafc4b2c9f048714489a794cc8fea5e5af0 Mon Sep 17 00:00:00 2001 From: Joel Hendrix Date: Thu, 28 Sep 2023 12:57:29 -0700 Subject: [PATCH 5/9] update changelog after rebase --- sdk/azcore/CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sdk/azcore/CHANGELOG.md b/sdk/azcore/CHANGELOG.md index f42facf37dc4..38301843f10c 100644 --- a/sdk/azcore/CHANGELOG.md +++ b/sdk/azcore/CHANGELOG.md @@ -43,6 +43,7 @@ * Fixed an issue that could cause some ARM RPs to not be automatically registered. * Block bearer token authentication for non TLS protected endpoints. +* Fixed an issue that could cause an HTTP/2 request to hang when the TCP connection becomes unresponsive. ### Other Changes @@ -57,8 +58,6 @@ ### Bugs Fixed * Suppress creating spans for nested SDK API calls. The HTTP span will be a child of the outer API span. -* Fix default HTTP transport to work in WASM modules. -* Fixed an issue that could cause an HTTP/2 request to hang when the TCP connection becomes unresponsive. ### Other Changes From 770d62b38ae3c30ce0dcac8f1359257cb067bba9 Mon Sep 17 00:00:00 2001 From: Joel Hendrix Date: Thu, 19 Oct 2023 11:40:40 -0700 Subject: [PATCH 6/9] go mod tidy --- sdk/azcore/go.mod | 3 ++- sdk/azcore/go.sum | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/sdk/azcore/go.mod b/sdk/azcore/go.mod index 68d0bfa24d32..25200804ac53 100644 --- a/sdk/azcore/go.mod +++ b/sdk/azcore/go.mod @@ -5,11 +5,12 @@ go 1.18 require ( github.com/Azure/azure-sdk-for-go/sdk/internal v1.3.0 github.com/stretchr/testify v1.7.0 + golang.org/x/net v0.15.0 ) require ( github.com/davecgh/go-spew v1.1.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - golang.org/x/net v0.15.0 // indirect + golang.org/x/text v0.13.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/sdk/azcore/go.sum b/sdk/azcore/go.sum index d04835d84330..352738d867be 100644 --- a/sdk/azcore/go.sum +++ b/sdk/azcore/go.sum @@ -10,6 +10,7 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ golang.org/x/net v0.15.0 h1:ugBLEUaxABaB5AJqW9enI0ACdci2RUd4eP51NTBvuJ8= golang.org/x/net v0.15.0/go.mod h1:idbUs1IY1+zTqbi8yxTbhexhEEk5ur9LInksu6HrEpk= golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k= +golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From be06d8ce22cb6232d06e52c0f1b3b7c20ea68ed4 Mon Sep 17 00:00:00 2001 From: Joel Hendrix Date: Thu, 19 Oct 2023 11:43:53 -0700 Subject: [PATCH 7/9] add TODO comment --- sdk/azcore/runtime/transport_default_http_client.go | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/azcore/runtime/transport_default_http_client.go b/sdk/azcore/runtime/transport_default_http_client.go index 71e3b04fdc33..3af83da36341 100644 --- a/sdk/azcore/runtime/transport_default_http_client.go +++ b/sdk/azcore/runtime/transport_default_http_client.go @@ -35,6 +35,7 @@ func init() { Renegotiation: tls.RenegotiateFreelyAsClient, }, } + // TODO: evaluate removing this once https://github.com/golang/go/issues/59690 has been fixed if http2Transport, err := http2.ConfigureTransports(defaultTransport); err == nil { // if the connection has been idle for 10 seconds, send a ping frame for a health check http2Transport.ReadIdleTimeout = 10 * time.Second From a288459a3b1ab54fd7b97394b755eb70096132f3 Mon Sep 17 00:00:00 2001 From: Joel Hendrix Date: Thu, 19 Oct 2023 11:45:08 -0700 Subject: [PATCH 8/9] fix changelog --- sdk/azcore/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/azcore/CHANGELOG.md b/sdk/azcore/CHANGELOG.md index 38301843f10c..7e490e4669bb 100644 --- a/sdk/azcore/CHANGELOG.md +++ b/sdk/azcore/CHANGELOG.md @@ -10,6 +10,7 @@ * Fixed an issue that could cause some allowed HTTP header values to not show up in logs. * Include error text instead of error type in traces when the transport returns an error. +* Fixed an issue that could cause an HTTP/2 request to hang when the TCP connection becomes unresponsive. ### Other Changes @@ -43,7 +44,6 @@ * Fixed an issue that could cause some ARM RPs to not be automatically registered. * Block bearer token authentication for non TLS protected endpoints. -* Fixed an issue that could cause an HTTP/2 request to hang when the TCP connection becomes unresponsive. ### Other Changes From 4bee0d49c7132f7ced315f46290e5692a4bd83de Mon Sep 17 00:00:00 2001 From: Joel Hendrix Date: Thu, 19 Oct 2023 16:23:39 -0700 Subject: [PATCH 9/9] tweak ping timeout --- sdk/azcore/runtime/transport_default_http_client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/azcore/runtime/transport_default_http_client.go b/sdk/azcore/runtime/transport_default_http_client.go index 3af83da36341..2124c1d48b9a 100644 --- a/sdk/azcore/runtime/transport_default_http_client.go +++ b/sdk/azcore/runtime/transport_default_http_client.go @@ -39,8 +39,8 @@ func init() { if http2Transport, err := http2.ConfigureTransports(defaultTransport); err == nil { // if the connection has been idle for 10 seconds, send a ping frame for a health check http2Transport.ReadIdleTimeout = 10 * time.Second - // if there's no response to the ping within 2 seconds, close the connection - http2Transport.PingTimeout = 2 * time.Second + // if there's no response to the ping within the timeout, the connection will be closed + http2Transport.PingTimeout = 5 * time.Second } defaultHTTPClient = &http.Client{ Transport: defaultTransport,