From 4275120424c7f51040c51687d6fd61a57f55989c Mon Sep 17 00:00:00 2001 From: Emily Ye Date: Tue, 11 Feb 2020 18:26:12 -0800 Subject: [PATCH 1/5] add connection reset by peer predicate --- .../terraform/utils/error_retry_predicates.go | 32 ++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/third_party/terraform/utils/error_retry_predicates.go b/third_party/terraform/utils/error_retry_predicates.go index 67ddf32dc7bf..0a2d046c6fb0 100644 --- a/third_party/terraform/utils/error_retry_predicates.go +++ b/third_party/terraform/utils/error_retry_predicates.go @@ -2,19 +2,24 @@ package google import ( "fmt" + "io" "log" + "net" "net/url" "strings" "google.golang.org/api/googleapi" ) +const connectionResetByPeerErr = "connection reset by peer" + type RetryErrorPredicateFunc func(error) (bool, string) /** ADD GLOBAL ERROR RETRY PREDICATES HERE **/ // Retry predicates that shoud apply to all requests should be added here. var defaultErrorRetryPredicates = []RetryErrorPredicateFunc{ - isUrlTimeoutError, + isTemporaryError, + isRetryableUrlError, isCommonRetryableErrorCode, //While this might apply only to Cloud SQL, historically, @@ -25,11 +30,30 @@ var defaultErrorRetryPredicates = []RetryErrorPredicateFunc{ /** END GLOBAL ERROR RETRY PREDICATES HERE **/ -func isUrlTimeoutError(err error) (bool, string) { - if urlerr, ok := err.(*url.Error); ok && urlerr.Timeout() { - log.Printf("[DEBUG] Dismissed an error as retryable based on googleapis.com target: %s", err) +func isTemporaryError(err error) (bool, string) { + if _, ok := err.(interface{ Temporary() bool }); ok { + return true, "Got temporary error %v" + } + return false, "" +} + +func isRetryableUrlError(err error) (bool, string) { + urlerr, ok := err.(*url.Error) + if !ok { + return false, "" + } + + if urlerr.Timeout() { return true, "Got URL timeout error" } + + wrappedErr := urlerr.Unwrap() + if wrappedErr == io.ErrUnexpectedEOF { + return true, "Got unexpected EOF" + } + if neterr, ok := wrappedErr.(*net.OpError); ok && neterr.Err.Error() == connectionResetByPeerErr { + return true, fmt.Sprintf("Got network error: %v", neterr.Err.Error()) + } return false, "" } From 1a23aeecaa61b326f1aa67b04cc2a2f7e8a0f2e3 Mon Sep 17 00:00:00 2001 From: Emily Ye Date: Tue, 11 Feb 2020 20:42:18 -0800 Subject: [PATCH 2/5] lint --- third_party/terraform/utils/error_retry_predicates.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/third_party/terraform/utils/error_retry_predicates.go b/third_party/terraform/utils/error_retry_predicates.go index 0a2d046c6fb0..652e92de8e1e 100644 --- a/third_party/terraform/utils/error_retry_predicates.go +++ b/third_party/terraform/utils/error_retry_predicates.go @@ -30,8 +30,12 @@ var defaultErrorRetryPredicates = []RetryErrorPredicateFunc{ /** END GLOBAL ERROR RETRY PREDICATES HERE **/ +type temporary interface { + Temporary() bool +} + func isTemporaryError(err error) (bool, string) { - if _, ok := err.(interface{ Temporary() bool }); ok { + if _, ok := err.(temporary); ok { return true, "Got temporary error %v" } return false, "" From 33e160d66a8b626a0fb3b03701881c93c825357c Mon Sep 17 00:00:00 2001 From: Emily Ye Date: Wed, 12 Feb 2020 16:30:10 -0800 Subject: [PATCH 3/5] separate error preds, fix temp pred --- .../terraform/utils/error_retry_predicates.go | 48 ++++++++++++------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/third_party/terraform/utils/error_retry_predicates.go b/third_party/terraform/utils/error_retry_predicates.go index 652e92de8e1e..d2eafe47ad15 100644 --- a/third_party/terraform/utils/error_retry_predicates.go +++ b/third_party/terraform/utils/error_retry_predicates.go @@ -19,7 +19,9 @@ type RetryErrorPredicateFunc func(error) (bool, string) // Retry predicates that shoud apply to all requests should be added here. var defaultErrorRetryPredicates = []RetryErrorPredicateFunc{ isTemporaryError, - isRetryableUrlError, + isUrlTimeoutError, + isIoEOFError, + isTemporaryNetOpError, isCommonRetryableErrorCode, //While this might apply only to Cloud SQL, historically, @@ -30,33 +32,45 @@ var defaultErrorRetryPredicates = []RetryErrorPredicateFunc{ /** END GLOBAL ERROR RETRY PREDICATES HERE **/ -type temporary interface { - Temporary() bool +func isTemporaryError(err error) (bool, string) { + if tempErr, ok := err.(interface{ Temporary() bool }); ok && tempErr.Temporary() { + return true, fmt.Sprintf("Got temporary error %v", err) + } + return false, "" } -func isTemporaryError(err error) (bool, string) { - if _, ok := err.(temporary); ok { - return true, "Got temporary error %v" +func isUrlTimeoutError(err error) (bool, string) { + if urlerr, ok := err.(*url.Error); ok && urlerr.Timeout() { + return true, "Got URL timeout error" } return false, "" } -func isRetryableUrlError(err error) (bool, string) { - urlerr, ok := err.(*url.Error) - if !ok { - return false, "" +func isIoEOFError(err error) (bool, string) { + if err == io.ErrUnexpectedEOF { + return true, "Got unexpected EOF" } - if urlerr.Timeout() { - return true, "Got URL timeout error" + if urlerr, ok := err.(*url.Error); ok { + wrappedErr := urlerr.Unwrap() + if wrappedErr == io.ErrUnexpectedEOF { + return true, "Got unexpected EOF" + } } + return false, "" +} - wrappedErr := urlerr.Unwrap() - if wrappedErr == io.ErrUnexpectedEOF { - return true, "Got unexpected EOF" +func isTemporaryNetOpError(err error) (bool, string) { + neterr, ok := err.(*net.OpError) + if !ok { + if urlerr, ok := err.(*url.Error); ok { + wrappedErr := urlerr.Unwrap() + neterr, ok = wrappedErr.(*net.OpError) + } } - if neterr, ok := wrappedErr.(*net.OpError); ok && neterr.Err.Error() == connectionResetByPeerErr { - return true, fmt.Sprintf("Got network error: %v", neterr.Err.Error()) + + if ok && neterr.Err.Error() == connectionResetByPeerErr { + return true, fmt.Sprintf("Connection reset by peer: %v", neterr.Err.Error()) } return false, "" } From f0b7d224097aec9d9911352c3b67968c59239297 Mon Sep 17 00:00:00 2001 From: Emily Ye Date: Thu, 13 Feb 2020 11:02:34 -0800 Subject: [PATCH 4/5] Fix error retries, comments --- .../terraform/utils/error_retry_predicates.go | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/third_party/terraform/utils/error_retry_predicates.go b/third_party/terraform/utils/error_retry_predicates.go index d2eafe47ad15..2ba987cf98ed 100644 --- a/third_party/terraform/utils/error_retry_predicates.go +++ b/third_party/terraform/utils/error_retry_predicates.go @@ -11,37 +11,45 @@ import ( "google.golang.org/api/googleapi" ) -const connectionResetByPeerErr = "connection reset by peer" - type RetryErrorPredicateFunc func(error) (bool, string) /** ADD GLOBAL ERROR RETRY PREDICATES HERE **/ // Retry predicates that shoud apply to all requests should be added here. var defaultErrorRetryPredicates = []RetryErrorPredicateFunc{ - isTemporaryError, - isUrlTimeoutError, + // Common network errors (usually wrapped by URL error) + isNetworkTemporaryError, + isNetworkTimeoutError, isIoEOFError, + isConnectionResetNetworkError, isTemporaryNetOpError, + + // Common GCP error codes isCommonRetryableErrorCode, //While this might apply only to Cloud SQL, historically, - // we had this in our global default error retries, so it is a default - // for now. + // we had this in our global default error retries. + // Keeping it as a default for now. is409OperationInProgressError, } /** END GLOBAL ERROR RETRY PREDICATES HERE **/ -func isTemporaryError(err error) (bool, string) { - if tempErr, ok := err.(interface{ Temporary() bool }); ok && tempErr.Temporary() { - return true, fmt.Sprintf("Got temporary error %v", err) +func isNetworkTemporaryError(err error) (bool, string) { + if netErr, ok := err.(*net.OpError); ok && netErr.Temporary() { + return true, "marked as timeout" + } + if urlerr, ok := err.(*url.Error); ok && urlerr.Temporary() { + return true, "marked as timeout" } return false, "" } -func isUrlTimeoutError(err error) (bool, string) { +func isNetworkTimeoutError(err error) (bool, string) { + if netErr, ok := err.(*net.OpError); ok && netErr.Timeout() { + return true, "marked as timeout" + } if urlerr, ok := err.(*url.Error); ok && urlerr.Timeout() { - return true, "Got URL timeout error" + return true, "marked as timeout" } return false, "" } @@ -51,7 +59,7 @@ func isIoEOFError(err error) (bool, string) { return true, "Got unexpected EOF" } - if urlerr, ok := err.(*url.Error); ok { + if urlerr, urlok := err.(*url.Error); urlok { wrappedErr := urlerr.Unwrap() if wrappedErr == io.ErrUnexpectedEOF { return true, "Got unexpected EOF" @@ -60,17 +68,18 @@ func isIoEOFError(err error) (bool, string) { return false, "" } -func isTemporaryNetOpError(err error) (bool, string) { +const connectionResetByPeerErr = "connection reset by peer" + +func isConnectionResetNetworkError(err error) (bool, string) { neterr, ok := err.(*net.OpError) if !ok { - if urlerr, ok := err.(*url.Error); ok { + if urlerr, urlok := err.(*url.Error); urlok { wrappedErr := urlerr.Unwrap() neterr, ok = wrappedErr.(*net.OpError) } } - if ok && neterr.Err.Error() == connectionResetByPeerErr { - return true, fmt.Sprintf("Connection reset by peer: %v", neterr.Err.Error()) + return true, fmt.Sprintf("Connection reset by peer") } return false, "" } From 819594fe7b6408ca7cd5860d4d22875a0fe6fe20 Mon Sep 17 00:00:00 2001 From: Emily Ye Date: Thu, 13 Feb 2020 11:17:28 -0800 Subject: [PATCH 5/5] remove removed func --- third_party/terraform/utils/error_retry_predicates.go | 1 - 1 file changed, 1 deletion(-) diff --git a/third_party/terraform/utils/error_retry_predicates.go b/third_party/terraform/utils/error_retry_predicates.go index 2ba987cf98ed..ba20ec23dfe5 100644 --- a/third_party/terraform/utils/error_retry_predicates.go +++ b/third_party/terraform/utils/error_retry_predicates.go @@ -21,7 +21,6 @@ var defaultErrorRetryPredicates = []RetryErrorPredicateFunc{ isNetworkTimeoutError, isIoEOFError, isConnectionResetNetworkError, - isTemporaryNetOpError, // Common GCP error codes isCommonRetryableErrorCode,