From 43eb5a001c9fc3feb05097a592a44d05e2a6a489 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 17 Mar 2022 16:20:10 -0700 Subject: [PATCH] re-use transport for AdminInfo() call (#14571) avoids creating new transport for each `isServerResolvable` request, instead re-use the available global transport and do not try to forcibly close connections to avoid TIME_WAIT build upon large clusters. Never use httpClient.CloseIdleConnections() since that can have a drastic effect on existing connections on the transport pool. Remove it everywhere. --- cmd/admin-handlers.go | 17 +++-------------- cmd/admin-server-info.go | 2 +- cmd/prepare-storage.go | 25 +------------------------ cmd/storage-interface.go | 1 - internal/config/dns/operator_dns.go | 1 - internal/config/identity/openid/jwt.go | 1 - internal/event/target/webhook.go | 2 -- 7 files changed, 5 insertions(+), 44 deletions(-) diff --git a/cmd/admin-handlers.go b/cmd/admin-handlers.go index 5fa5d71c7..676ea80d7 100644 --- a/cmd/admin-handlers.go +++ b/cmd/admin-handlers.go @@ -21,7 +21,6 @@ import ( "context" crand "crypto/rand" "crypto/subtle" - "crypto/tls" "encoding/json" "errors" "fmt" @@ -2332,19 +2331,9 @@ func checkConnection(endpointStr string, timeout time.Duration) error { ctx, cancel := context.WithTimeout(GlobalContext, timeout) defer cancel() - client := &http.Client{Transport: &http.Transport{ - Proxy: http.ProxyFromEnvironment, - DialContext: xhttp.NewCustomDialContext(timeout), - ResponseHeaderTimeout: 5 * time.Second, - TLSHandshakeTimeout: 5 * time.Second, - ExpectContinueTimeout: 5 * time.Second, - TLSClientConfig: &tls.Config{RootCAs: globalRootCAs}, - // Go net/http automatically unzip if content-type is - // gzip disable this feature, as we are always interested - // in raw stream. - DisableCompression: true, - }} - defer client.CloseIdleConnections() + client := &http.Client{ + Transport: globalProxyTransport, + } req, err := http.NewRequestWithContext(ctx, http.MethodHead, endpointStr, nil) if err != nil { diff --git a/cmd/admin-server-info.go b/cmd/admin-server-info.go index 9abc2c5ce..2a5b29252 100644 --- a/cmd/admin-server-info.go +++ b/cmd/admin-server-info.go @@ -50,7 +50,7 @@ func getLocalServerProperty(endpointServerPools EndpointServerPools, r *http.Req } _, present := network[nodeName] if !present { - if err := isServerResolvable(endpoint, 2*time.Second); err == nil { + if err := isServerResolvable(endpoint, 5*time.Second); err == nil { network[nodeName] = string(madmin.ItemOnline) } else { network[nodeName] = string(madmin.ItemOffline) diff --git a/cmd/prepare-storage.go b/cmd/prepare-storage.go index ca7fa0cbc..6d4b2d6fb 100644 --- a/cmd/prepare-storage.go +++ b/cmd/prepare-storage.go @@ -19,7 +19,6 @@ package cmd import ( "context" - "crypto/tls" "errors" "fmt" "net/http" @@ -121,31 +120,9 @@ func isServerResolvable(endpoint Endpoint, timeout time.Duration) error { Path: pathJoin(healthCheckPathPrefix, healthCheckLivenessPath), } - var tlsConfig *tls.Config - if globalIsTLS { - tlsConfig = &tls.Config{ - RootCAs: globalRootCAs, - } - } - httpClient := &http.Client{ - Transport: - // For more details about various values used here refer - // https://golang.org/pkg/net/http/#Transport documentation - &http.Transport{ - Proxy: http.ProxyFromEnvironment, - DialContext: xhttp.NewCustomDialContext(3 * time.Second), - ResponseHeaderTimeout: 3 * time.Second, - TLSHandshakeTimeout: 3 * time.Second, - ExpectContinueTimeout: 3 * time.Second, - TLSClientConfig: tlsConfig, - // Go net/http automatically unzip if content-type is - // gzip disable this feature, as we are always interested - // in raw stream. - DisableCompression: true, - }, + Transport: globalInternodeTransport, } - defer httpClient.CloseIdleConnections() ctx, cancel := context.WithTimeout(GlobalContext, timeout) diff --git a/cmd/storage-interface.go b/cmd/storage-interface.go index 6f9cd159e..11754278f 100644 --- a/cmd/storage-interface.go +++ b/cmd/storage-interface.go @@ -63,7 +63,6 @@ type StorageAPI interface { // returns 'nil' once healing is complete or if the disk // has never been replaced. Healing() *healingTracker - DiskInfo(ctx context.Context) (info DiskInfo, err error) NSScanner(ctx context.Context, cache dataUsageCache, updates chan<- dataUsageEntry) (dataUsageCache, error) diff --git a/internal/config/dns/operator_dns.go b/internal/config/dns/operator_dns.go index fd0e06444..dae37c3c1 100644 --- a/internal/config/dns/operator_dns.go +++ b/internal/config/dns/operator_dns.go @@ -155,7 +155,6 @@ func (c *OperatorDNS) DeleteRecord(record SrvRecord) error { // Close closes the internal http client func (c *OperatorDNS) Close() error { - c.httpClient.CloseIdleConnections() return nil } diff --git a/internal/config/identity/openid/jwt.go b/internal/config/identity/openid/jwt.go index 9349fceea..3309e4217 100644 --- a/internal/config/identity/openid/jwt.go +++ b/internal/config/identity/openid/jwt.go @@ -471,7 +471,6 @@ func parseDiscoveryDoc(u *xnet.URL, transport *http.Transport, closeRespFn func( } resp, err := clnt.Do(req) if err != nil { - clnt.CloseIdleConnections() return d, err } defer closeRespFn(resp.Body) diff --git a/internal/event/target/webhook.go b/internal/event/target/webhook.go index e3eed5e24..6421fa519 100644 --- a/internal/event/target/webhook.go +++ b/internal/event/target/webhook.go @@ -228,8 +228,6 @@ func (target *WebhookTarget) Send(eventKey string) error { // Close - does nothing and available for interface compatibility. func (target *WebhookTarget) Close() error { - // Close idle connection with "keep-alive" states - target.httpClient.CloseIdleConnections() return nil }