From 54eded2e6f73125cc37a9e144fd7f80543d96e13 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 29 Jul 2019 14:48:18 -0700 Subject: [PATCH] Do not assume all HTTP errors as Network errors (#7983) In situations such as when client uploading data, prematurely disconnects from server such as pressing ctrl-c before uploading all the data. Under this situation in distributed setup we prematurely disconnect disks causing a reconnect loop. This has an adverse affect we end up leaving a lot of files in temporary location which ideally should have been cleaned up when Put() prematurely fails. This is also a regression which got introduced in #7610 --- cmd/object-api-common.go | 2 +- cmd/storage-rest-client.go | 4 ++-- cmd/utils.go | 38 ++++++++++++++------------------------ 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/cmd/object-api-common.go b/cmd/object-api-common.go index d94af00b9..bfbf8e000 100644 --- a/cmd/object-api-common.go +++ b/cmd/object-api-common.go @@ -129,7 +129,7 @@ func cleanupDir(ctx context.Context, storage StorageAPI, volume, dirPath string) // Entry path is empty, just delete it. if len(entries) == 0 { - err = storage.DeleteFile(volume, path.Clean(entryPath)) + err = storage.DeleteFile(volume, entryPath) logger.LogIf(ctx, err) return err } diff --git a/cmd/storage-rest-client.go b/cmd/storage-rest-client.go index 71933dd18..f74a781d1 100644 --- a/cmd/storage-rest-client.go +++ b/cmd/storage-rest-client.go @@ -44,8 +44,8 @@ func isNetworkError(err error) bool { if err.Error() == errConnectionStale.Error() { return true } - if _, ok := err.(*rest.NetworkError); ok { - return true + if nerr, ok := err.(*rest.NetworkError); ok { + return isNetworkOrHostDown(nerr.Err) } return false } diff --git a/cmd/utils.go b/cmd/utils.go index 7a786591d..b9cc12247 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -29,7 +29,6 @@ import ( "io/ioutil" "net" "net/http" - "net/url" "os" "path/filepath" "reflect" @@ -443,30 +442,21 @@ func isNetworkOrHostDown(err error) bool { if err == nil { return false } - switch err.(type) { - case *net.DNSError, *net.OpError, net.UnknownNetworkError: - return true - case *url.Error: - // For a URL error, where it replies back "connection closed" - if strings.Contains(err.Error(), "Connection closed by foreign host") { - return true - } - return true - default: - if strings.Contains(err.Error(), "net/http: TLS handshake timeout") { - // If error is - tlsHandshakeTimeoutError,. - return true - } else if strings.Contains(err.Error(), "i/o timeout") { - // If error is - tcp timeoutError. - return true - } else if strings.Contains(err.Error(), "connection timed out") { - // If err is a net.Dial timeout. - return true - } else if strings.Contains(err.Error(), "net/http: HTTP/1.x transport connection broken") { - return true - } + // We need to figure if the error either a timeout + // or a non-temporary error. + e, ok := err.(net.Error) + if ok { + return e.Timeout() } - return false + // Fallback to other mechanisms. + if strings.Contains(err.Error(), "i/o timeout") { + // If error is - tcp timeoutError. + ok = true + } else if strings.Contains(err.Error(), "connection timed out") { + // If err is a net.Dial timeout. + ok = true + } + return ok } // Used for registering with rest handlers (have a look at registerStorageRESTHandlers for usage example)