diff --git a/cmd/lock-rest-client.go b/cmd/lock-rest-client.go index 24c116b38..cf81825d3 100644 --- a/cmd/lock-rest-client.go +++ b/cmd/lock-rest-client.go @@ -19,7 +19,6 @@ package cmd import ( "bytes" "context" - "errors" "io" "net/url" "strconv" @@ -28,6 +27,7 @@ import ( xhttp "github.com/minio/minio/cmd/http" "github.com/minio/minio/cmd/rest" "github.com/minio/minio/pkg/dsync" + xnet "github.com/minio/minio/pkg/net" ) // lockRESTClient is authenticable lock REST client @@ -153,15 +153,15 @@ func newlockRESTClient(endpoint Endpoint) *lockRESTClient { restClient := rest.NewClient(serverURL, globalInternodeTransport, newAuthToken) restClient.ExpectTimeouts = true + // Use a separate client to avoid recursive calls. + healthClient := rest.NewClient(serverURL, globalInternodeTransport, newAuthToken) + healthClient.ExpectTimeouts = true restClient.HealthCheckFn = func() bool { ctx, cancel := context.WithTimeout(GlobalContext, restClient.HealthCheckTimeout) - // Instantiate a new rest client for healthcheck - // to avoid recursive healthCheckFn() - respBody, err := rest.NewClient(serverURL, globalInternodeTransport, newAuthToken).Call(ctx, lockRESTMethodHealth, nil, nil, -1) + defer cancel() + respBody, err := healthClient.Call(ctx, lockRESTMethodHealth, nil, nil, -1) xhttp.DrainBody(respBody) - cancel() - var ne *rest.NetworkError - return !errors.Is(err, context.DeadlineExceeded) && !errors.As(err, &ne) + return !xnet.IsNetworkOrHostDown(err, false) } return &lockRESTClient{endpoint: endpoint, restClient: restClient} diff --git a/cmd/peer-rest-client.go b/cmd/peer-rest-client.go index 136ef4b2d..4ac1c85e3 100644 --- a/cmd/peer-rest-client.go +++ b/cmd/peer-rest-client.go @@ -872,16 +872,17 @@ func newPeerRESTClient(peer *xnet.Host) *peerRESTClient { } restClient := rest.NewClient(serverURL, globalInternodeTransport, newAuthToken) + // Use a separate client to avoid recursive calls. + healthClient := rest.NewClient(serverURL, globalInternodeTransport, newAuthToken) + healthClient.ExpectTimeouts = true + // Construct a new health function. restClient.HealthCheckFn = func() bool { ctx, cancel := context.WithTimeout(GlobalContext, restClient.HealthCheckTimeout) - // Instantiate a new rest client for healthcheck - // to avoid recursive healthCheckFn() - respBody, err := rest.NewClient(serverURL, globalInternodeTransport, newAuthToken).Call(ctx, peerRESTMethodHealth, nil, nil, -1) + defer cancel() + respBody, err := healthClient.Call(ctx, peerRESTMethodHealth, nil, nil, -1) xhttp.DrainBody(respBody) - cancel() - var ne *rest.NetworkError - return !errors.Is(err, context.DeadlineExceeded) && !errors.As(err, &ne) + return !xnet.IsNetworkOrHostDown(err, false) } return &peerRESTClient{host: peer, restClient: restClient} diff --git a/cmd/rest/client.go b/cmd/rest/client.go index 0707fe7c2..fe32136c8 100644 --- a/cmd/rest/client.go +++ b/cmd/rest/client.go @@ -117,7 +117,7 @@ func (c *Client) Call(ctx context.Context, method string, values url.Values, bod } resp, err := c.httpClient.Do(req) if err != nil { - if xnet.IsNetworkOrHostDown(err, c.ExpectTimeouts) { + if c.HealthCheckFn != nil && xnet.IsNetworkOrHostDown(err, c.ExpectTimeouts) { logger.Info("Marking %s temporary offline; caused by %v", c.url.String(), err) c.MarkOffline() } @@ -140,7 +140,7 @@ func (c *Client) Call(ctx context.Context, method string, values url.Values, bod // with the caller to take the client offline purpose // fully it should make sure to respond with '412' // instead, see cmd/storage-rest-server.go for ideas. - if resp.StatusCode == http.StatusPreconditionFailed { + if c.HealthCheckFn != nil && resp.StatusCode == http.StatusPreconditionFailed { logger.Info("Marking %s temporary offline; caused by PreconditionFailed.", c.url.String()) c.MarkOffline() } @@ -148,7 +148,7 @@ func (c *Client) Call(ctx context.Context, method string, values url.Values, bod // Limit the ReadAll(), just in case, because of a bug, the server responds with large data. b, err := ioutil.ReadAll(io.LimitReader(resp.Body, c.MaxErrResponseSize)) if err != nil { - if xnet.IsNetworkOrHostDown(err, c.ExpectTimeouts) { + if c.HealthCheckFn != nil && xnet.IsNetworkOrHostDown(err, c.ExpectTimeouts) { logger.Info("Marking %s temporary offline; caused by %v", c.url.String(), err) c.MarkOffline() } diff --git a/cmd/storage-rest-client.go b/cmd/storage-rest-client.go index cbe521e5a..fad6d5c20 100644 --- a/cmd/storage-rest-client.go +++ b/cmd/storage-rest-client.go @@ -615,15 +615,17 @@ func newStorageRESTClient(endpoint Endpoint, healthcheck bool) *storageRESTClien } restClient := rest.NewClient(serverURL, globalInternodeTransport, newAuthToken) + if healthcheck { + // Use a separate client to avoid recursive calls. + healthClient := rest.NewClient(serverURL, globalInternodeTransport, newAuthToken) + healthClient.ExpectTimeouts = true restClient.HealthCheckFn = func() bool { ctx, cancel := context.WithTimeout(GlobalContext, restClient.HealthCheckTimeout) - // Instantiate a new rest client for healthcheck - // to avoid recursive healthCheckFn() - respBody, err := rest.NewClient(serverURL, globalInternodeTransport, newAuthToken).Call(ctx, storageRESTMethodHealth, nil, nil, -1) + respBody, err := healthClient.Call(ctx, storageRESTMethodHealth, nil, nil, -1) xhttp.DrainBody(respBody) cancel() - return !errors.Is(err, context.DeadlineExceeded) && toStorageErr(err) != errDiskNotFound + return !xnet.IsNetworkOrHostDown(err, false) && toStorageErr(err) != errDiskNotFound } }