Merge pull request #6405 from tinyspeck/am_fallback_retries

Update s3 retryer to correctly fallback to the default retryer

go/vt/mysqlctl/s3backupstorage/retryer.go

@@ -35,7 +35,9 @@ func (retryer *ClosedConnectionRetryer) ShouldRetry(r *request.Request) bool {
 
 	if r.Error != nil {
 		if awsErr, ok := r.Error.(awserr.Error); ok {
-			return strings.Contains(awsErr.Error(), "use of closed network connection")
+			if strings.Contains(awsErr.Error(), "use of closed network connection") {
+				return true
+			}
 		}
 	}
 

go/vt/mysqlctl/s3backupstorage/retryer_test.go

@@ -11,62 +11,77 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-type testRetryer struct{}
+type testRetryer struct{ retry bool }
 
 func (r *testRetryer) MaxRetries() int                               { return 5 }
 func (r *testRetryer) RetryRules(req *request.Request) time.Duration { return time.Second }
-func (r *testRetryer) ShouldRetry(req *request.Request) bool         { return false }
+func (r *testRetryer) ShouldRetry(req *request.Request) bool         { return r.retry }
 
 func TestShouldRetry(t *testing.T) {
 	tests := []struct {
-		name     string
-		r        *request.Request
-		expected bool
+		name           string
+		r              *request.Request
+		fallbackPolicy bool
+		expected       bool
 	}{
 
 		{
 			name: "non retryable request",
 			r: &request.Request{
 				Retryable: aws.Bool(false),
 			},
-			expected: false,
+			fallbackPolicy: false,
+			expected:       false,
 		},
 		{
 			name: "retryable request",
 			r: &request.Request{
 				Retryable: aws.Bool(true),
 			},
-			expected: true,
+			fallbackPolicy: false,
+			expected:       true,
 		},
 		{
 			name: "non aws error",
 			r: &request.Request{
 				Retryable: nil,
 				Error:     errors.New("some error"),
 			},
-			expected: false,
+			fallbackPolicy: false,
+			expected:       false,
 		},
 		{
 			name: "closed connection error",
 			r: &request.Request{
 				Retryable: nil,
 				Error:     awserr.New("5xx", "use of closed network connection", nil),
 			},
-			expected: true,
+			fallbackPolicy: false,
+			expected:       true,
 		},
 		{
 			name: "closed connection error (non nil origError)",
 			r: &request.Request{
 				Retryable: nil,
 				Error:     awserr.New("5xx", "use of closed network connection", errors.New("some error")),
 			},
-			expected: true,
+			fallbackPolicy: false,
+			expected:       true,
+		},
+		{
+			name: "other aws error hits fallback policy",
+			r: &request.Request{
+				Retryable: nil,
+				Error:     awserr.New("code", "not a closed network connectionn", errors.New("some error")),
+			},
+			fallbackPolicy: true,
+			expected:       true,
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			retryer := &ClosedConnectionRetryer{&testRetryer{}}
+			retryer := &ClosedConnectionRetryer{&testRetryer{test.fallbackPolicy}}
 			msg := ""
 			if test.r.Error != nil {
 				if awsErr, ok := test.r.Error.(awserr.Error); ok {