From 0b8255529a6f74df81f975869b6f2b93ff4b4dda Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 4 Aug 2020 14:55:53 -0700 Subject: [PATCH] fix: proxies set keep-alive timeouts to be system dependent (#10199) Split the DialContext's one for internode and another for all other external communications especially proxy forwarders, gateway transport etc. --- cmd/admin-handlers.go | 18 ++++++++++++++---- cmd/bootstrap-peer-server.go | 2 +- cmd/http/dial_linux.go | 33 ++++++++++++++++++++++++++------- cmd/http/dial_others.go | 3 +++ cmd/lock-rest-client.go | 2 +- cmd/peer-rest-client.go | 2 +- cmd/prepare-storage.go | 6 +++++- cmd/storage-rest-client.go | 2 +- cmd/utils.go | 27 +++++++++++++++++++++++++-- cmd/xl-storage.go | 23 +++++++++++++++++------ 10 files changed, 94 insertions(+), 24 deletions(-) diff --git a/cmd/admin-handlers.go b/cmd/admin-handlers.go index 7d1e7884b..49d67d41a 100644 --- a/cmd/admin-handlers.go +++ b/cmd/admin-handlers.go @@ -1631,9 +1631,6 @@ func fetchLoggerInfo(cfg config.Config) ([]madmin.Logger, []madmin.Audit) { // checkConnection - ping an endpoint , return err in case of no connection func checkConnection(endpointStr string, timeout time.Duration) error { - tr := newCustomHTTPTransport(&tls.Config{RootCAs: globalRootCAs}, timeout)() - defer tr.CloseIdleConnections() - ctx, cancel := context.WithTimeout(GlobalContext, timeout) defer cancel() @@ -1642,7 +1639,20 @@ func checkConnection(endpointStr string, timeout time.Duration) error { return err } - client := &http.Client{Transport: tr} + 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() + resp, err := client.Do(req.WithContext(ctx)) if err != nil { return err diff --git a/cmd/bootstrap-peer-server.go b/cmd/bootstrap-peer-server.go index 92ede4dff..a6c8eae37 100644 --- a/cmd/bootstrap-peer-server.go +++ b/cmd/bootstrap-peer-server.go @@ -228,7 +228,7 @@ func newBootstrapRESTClient(endpoint Endpoint) *bootstrapRESTClient { } } - trFn := newCustomHTTPTransport(tlsConfig, rest.DefaultRESTTimeout) + trFn := newInternodeHTTPTransport(tlsConfig, rest.DefaultRESTTimeout) restClient := rest.NewClient(serverURL, trFn, newAuthToken) restClient.HealthCheckFn = func() bool { ctx, cancel := context.WithTimeout(GlobalContext, restClient.HealthCheckTimeout) diff --git a/cmd/http/dial_linux.go b/cmd/http/dial_linux.go index 74ce22c21..281fd0040 100644 --- a/cmd/http/dial_linux.go +++ b/cmd/http/dial_linux.go @@ -49,20 +49,17 @@ func setInternalTCPParameters(c syscall.RawConn) error { _ = syscall.SetsockoptInt(int(fd), syscall.IPPROTO_TCP, syscall.TCP_KEEPCNT, 5) // Wait time after successful probe in seconds. - // ~ cat /proc/sys/net/ipv4/tcp_keepalive_intvl (defaults to 75 secs, we reduce it to 2 secs) + // ~ cat /proc/sys/net/ipv4/tcp_keepalive_intvl (defaults to 75 secs, we reduce it to 3 secs) // 75 - _ = syscall.SetsockoptInt(int(fd), syscall.IPPROTO_TCP, syscall.TCP_KEEPINTVL, 2) - - // Set TCP_USER_TIMEOUT to TCP_KEEPIDLE + TCP_KEEPINTVL * TCP_KEEPCNT. - _ = syscall.SetsockoptInt(int(fd), syscall.IPPROTO_TCP, unix.TCP_USER_TIMEOUT, 15) + _ = syscall.SetsockoptInt(int(fd), syscall.IPPROTO_TCP, syscall.TCP_KEEPINTVL, 3) }) } // DialContext is a function to make custom Dial for internode communications type DialContext func(ctx context.Context, network, address string) (net.Conn, error) -// NewCustomDialContext setups a custom dialer for internode communications -func NewCustomDialContext(dialTimeout time.Duration) DialContext { +// NewInternodeDialContext setups a custom dialer for internode communication +func NewInternodeDialContext(dialTimeout time.Duration) DialContext { return func(ctx context.Context, network, addr string) (net.Conn, error) { dialer := &net.Dialer{ Timeout: dialTimeout, @@ -73,3 +70,25 @@ func NewCustomDialContext(dialTimeout time.Duration) DialContext { return dialer.DialContext(ctx, network, addr) } } + +// NewCustomDialContext setups a custom dialer for any external communication and proxies. +func NewCustomDialContext(dialTimeout time.Duration) DialContext { + return func(ctx context.Context, network, addr string) (net.Conn, error) { + dialer := &net.Dialer{ + Timeout: dialTimeout, + Control: func(network, address string, c syscall.RawConn) error { + return c.Control(func(fdPtr uintptr) { + // got socket file descriptor to set parameters. + fd := int(fdPtr) + + // Enable TCP fast connect + // TCPFastOpenConnect sets the underlying socket to use + // the TCP fast open connect. This feature is supported + // since Linux 4.11. + _ = syscall.SetsockoptInt(int(fd), syscall.IPPROTO_TCP, unix.TCP_FASTOPEN_CONNECT, 1) + }) + }, + } + return dialer.DialContext(ctx, network, addr) + } +} diff --git a/cmd/http/dial_others.go b/cmd/http/dial_others.go index b1decdbcf..ebdf8e912 100644 --- a/cmd/http/dial_others.go +++ b/cmd/http/dial_others.go @@ -33,6 +33,9 @@ func setInternalTCPParameters(c syscall.RawConn) error { // DialContext is a function to make custom Dial for internode communications type DialContext func(ctx context.Context, network, address string) (net.Conn, error) +// NewInternodeDialContext setups a custom dialer for internode communication +var NewInternodeDialContext = NewCustomDialContext + // NewCustomDialContext configures a custom dialer for internode communications func NewCustomDialContext(dialTimeout time.Duration) DialContext { return func(ctx context.Context, network, addr string) (net.Conn, error) { diff --git a/cmd/lock-rest-client.go b/cmd/lock-rest-client.go index 02421bafd..91e63e6e6 100644 --- a/cmd/lock-rest-client.go +++ b/cmd/lock-rest-client.go @@ -153,7 +153,7 @@ func newlockRESTClient(endpoint Endpoint) *lockRESTClient { } } - trFn := newCustomHTTPTransport(tlsConfig, rest.DefaultRESTTimeout) + trFn := newInternodeHTTPTransport(tlsConfig, rest.DefaultRESTTimeout) restClient := rest.NewClient(serverURL, trFn, newAuthToken) restClient.HealthCheckFn = func() bool { ctx, cancel := context.WithTimeout(GlobalContext, restClient.HealthCheckTimeout) diff --git a/cmd/peer-rest-client.go b/cmd/peer-rest-client.go index f7c7c6eef..832b93b6d 100644 --- a/cmd/peer-rest-client.go +++ b/cmd/peer-rest-client.go @@ -874,7 +874,7 @@ func newPeerRESTClient(peer *xnet.Host) *peerRESTClient { } } - trFn := newCustomHTTPTransport(tlsConfig, rest.DefaultRESTTimeout) + trFn := newInternodeHTTPTransport(tlsConfig, rest.DefaultRESTTimeout) restClient := rest.NewClient(serverURL, trFn, newAuthToken) // Construct a new health function. diff --git a/cmd/prepare-storage.go b/cmd/prepare-storage.go index f128b4b7a..c73b750e9 100644 --- a/cmd/prepare-storage.go +++ b/cmd/prepare-storage.go @@ -17,6 +17,7 @@ package cmd import ( + "context" "crypto/tls" "fmt" "net/http" @@ -212,7 +213,10 @@ func IsServerResolvable(endpoint Endpoint) error { } defer httpClient.CloseIdleConnections() - resp, err := httpClient.Do(req) + ctx, cancel := context.WithTimeout(GlobalContext, 5*time.Second) + defer cancel() + + resp, err := httpClient.Do(req.WithContext(ctx)) if err != nil { return err } diff --git a/cmd/storage-rest-client.go b/cmd/storage-rest-client.go index c37524211..156a0e761 100644 --- a/cmd/storage-rest-client.go +++ b/cmd/storage-rest-client.go @@ -667,7 +667,7 @@ func newStorageRESTClient(endpoint Endpoint) *storageRESTClient { } } - trFn := newCustomHTTPTransport(tlsConfig, rest.DefaultRESTTimeout) + trFn := newInternodeHTTPTransport(tlsConfig, rest.DefaultRESTTimeout) restClient := rest.NewClient(serverURL, trFn, newAuthToken) restClient.HealthCheckInterval = 500 * time.Millisecond restClient.HealthCheckFn = func() bool { diff --git a/cmd/utils.go b/cmd/utils.go index 04dcaef9a..cd2c5addb 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -1,5 +1,5 @@ /* - * MinIO Cloud Storage, (C) 2015, 2016, 2017 MinIO, Inc. + * MinIO Cloud Storage, (C) 2015-2020 MinIO, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -170,7 +170,7 @@ const ( // (Acceptable values range from 1 to 10000 inclusive) globalMaxPartID = 10000 - // Default values used while communicating for internode communication. + // Default values used while communicating for gateway communication defaultDialTimeout = 5 * time.Second ) @@ -449,6 +449,29 @@ func ToS3ETag(etag string) string { return etag } +func newInternodeHTTPTransport(tlsConfig *tls.Config, dialTimeout time.Duration) func() *http.Transport { + // For more details about various values used here refer + // https://golang.org/pkg/net/http/#Transport documentation + tr := &http.Transport{ + Proxy: http.ProxyFromEnvironment, + DialContext: xhttp.NewInternodeDialContext(dialTimeout), + MaxIdleConnsPerHost: 16, + MaxIdleConns: 16, + IdleConnTimeout: 1 * time.Minute, + ResponseHeaderTimeout: 3 * time.Minute, // Set conservative timeouts for MinIO internode. + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 10 * 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, + } + return func() *http.Transport { + return tr + } +} + func newCustomHTTPTransport(tlsConfig *tls.Config, dialTimeout time.Duration) func() *http.Transport { // For more details about various values used here refer // https://golang.org/pkg/net/http/#Transport documentation diff --git a/cmd/xl-storage.go b/cmd/xl-storage.go index a9492de26..b7ea0acc6 100644 --- a/cmd/xl-storage.go +++ b/cmd/xl-storage.go @@ -40,8 +40,10 @@ import ( humanize "github.com/dustin/go-humanize" jsoniter "github.com/json-iterator/go" "github.com/klauspost/readahead" + "github.com/minio/minio/cmd/config" "github.com/minio/minio/cmd/logger" "github.com/minio/minio/pkg/disk" + "github.com/minio/minio/pkg/env" xioutil "github.com/minio/minio/pkg/ioutil" "github.com/minio/minio/pkg/mountinfo" ) @@ -93,6 +95,8 @@ type xlStorage struct { pool sync.Pool + globalSync bool + diskMount bool // indicates if the path is an actual mount. diskID string @@ -245,7 +249,8 @@ func newXLStorage(path string, hostname string) (*xlStorage, error) { return &b }, }, - diskMount: mountinfo.IsLikelyMountPoint(path), + globalSync: env.Get(config.EnvFSOSync, config.EnableOn) == config.EnableOn, + diskMount: mountinfo.IsLikelyMountPoint(path), // Allow disk usage crawler to run with up to 2 concurrent // I/O ops, if and when activeIOCount reaches this // value disk usage routine suspends the crawler @@ -1216,8 +1221,10 @@ func (s *xlStorage) renameLegacyMetadata(volume, path string) error { // Renaming xl.json to xl.meta should be fully synced to disk. defer func() { if err == nil { - // Sync to disk only upon success. - globalSync() + if s.globalSync { + // Sync to disk only upon success. + globalSync() + } } }() @@ -2104,8 +2111,10 @@ func (s *xlStorage) RenameData(srcVolume, srcPath, dataDir, dstVolume, dstPath s return osErrToFileErr(err) } - // Sync all the previous directory operations. - globalSync() + if s.globalSync { + // Sync all the previous directory operations. + globalSync() + } for _, entry := range entries { if entry == xlStorageFormatFile { @@ -2121,7 +2130,9 @@ func (s *xlStorage) RenameData(srcVolume, srcPath, dataDir, dstVolume, dstPath s } // Sync all the metadata operations once renames are done. - globalSync() + if s.globalSync { + globalSync() + } } var oldDstDataPath string