aws : Adds configurations to the default retryer

CHANGELOG_PENDING.md

@@ -1,6 +1,12 @@
 ### SDK Features
 
 ### SDK Enhancements
-
+* `aws`: Provides more customization options for retryer by adding a constructor for default Retryer which accepts functional options. Adds NoOpRetryer to support no retry behavior. Exposes members of default retryer. [#375](https://github.com/aws/aws-sdk-go-v2/pull/375)
+    * Updates the underlying logic used by the default retryer to calculate jittered delay for retry. 
+    * Handles int overflow for retry delay. Fixes [#370](https://github.com/aws/aws-sdk-go-v2/issues/370)
+* `service/ec2`: Adds custom retryer implementation for service/ec2.
+  * Adds test case to test custom retryer. 
+
 ### SDK Bugs
 
+

aws/client.go

@@ -63,13 +63,10 @@ func NewClient(cfg Config, metadata Metadata) *Client {
 
 	retryer := cfg.Retryer
 	if retryer == nil {
-		// TODO need better way of specifing default num retries
-		retryer = DefaultRetryer{NumMaxRetries: 3}
+		retryer = NewDefaultRetryer()
 	}
 	svc.Retryer = retryer
-
 	svc.AddDebugHandlers()
-
 	return svc
 }
 

aws/default_retryer.go

@@ -1,6 +1,7 @@
 package aws
 
 import (
+	"math"
 	"math/rand"
 	"strconv"
 	"sync"
@@ -17,12 +18,31 @@ import (
 //      client.DefaultRetryer
 //    }
 //
-//    // This implementation always has 100 max retries
-//    func (d retryer) MaxRetries() int { return 100 }
 type DefaultRetryer struct {
-	NumMaxRetries int
+	NumMaxRetries    int
+	MinRetryDelay    time.Duration
+	MinThrottleDelay time.Duration
+	MaxRetryDelay    time.Duration
+	MaxThrottleDelay time.Duration
 }
 
+const (
+	// DefaultRetryerMaxNumRetries sets maximum number of retries
+	DefaultRetryerMaxNumRetries = 3
+
+	// DefaultRetryerMinRetryDelay sets minimum retry delay
+	DefaultRetryerMinRetryDelay = 30 * time.Millisecond
+
+	// DefaultRetryerMinThrottleDelay sets minimum delay when throttled
+	DefaultRetryerMinThrottleDelay = 500 * time.Millisecond
+
+	// DefaultRetryerMaxRetryDelay sets maximum retry delay
+	DefaultRetryerMaxRetryDelay = 300 * time.Second
+
+	// DefaultRetryerMaxThrottleDelay sets maximum delay when throttled
+	DefaultRetryerMaxThrottleDelay = 300 * time.Second
+)
+
 // MaxRetries returns the number of maximum returns the service will use to make
 // an individual API
 func (d DefaultRetryer) MaxRetries() int {
@@ -31,28 +51,63 @@ func (d DefaultRetryer) MaxRetries() int {
 
 var seededRand = rand.New(&lockedSource{src: rand.NewSource(time.Now().UnixNano())})
 
+// NewDefaultRetryer returns a retryer initialized with default values and optionally takes function
+// to override values for default retryer.
+func NewDefaultRetryer(opts ...func(d *DefaultRetryer)) DefaultRetryer {
+	d := DefaultRetryer{
+		NumMaxRetries:    DefaultRetryerMaxNumRetries,
+		MinRetryDelay:    DefaultRetryerMinRetryDelay,
+		MinThrottleDelay: DefaultRetryerMinThrottleDelay,
+		MaxRetryDelay:    DefaultRetryerMaxRetryDelay,
+		MaxThrottleDelay: DefaultRetryerMaxThrottleDelay,
+	}
+
+	for _, opt := range opts {
+		opt(&d)
+	}
+	return d
+}
+
 // RetryRules returns the delay duration before retrying this request again
+//
+// Note: RetryRules method must be a value receiver so that the
+// defaultRetryer is safe.
 func (d DefaultRetryer) RetryRules(r *Request) time.Duration {
 	// Set the upper limit of delay in retrying at ~five minutes
-	minTime := 30
+	minDelay := d.MinRetryDelay
+	var initialDelay time.Duration
 	throttle := d.shouldThrottle(r)
 	if throttle {
-		if delay, ok := getRetryDelay(r); ok {
-			return delay
+		if delay, ok := getRetryAfterDelay(r); ok {
+			initialDelay = delay
 		}
-
-		minTime = 500
+		minDelay = d.MinThrottleDelay
 	}
 
 	retryCount := r.RetryCount
-	if retryCount > 13 {
-		retryCount = 13
-	} else if throttle && retryCount > 8 {
-		retryCount = 8
+	maxDelay := d.MaxRetryDelay
+	if throttle {
+		maxDelay = d.MaxThrottleDelay
 	}
 
-	delay := (1 << uint(retryCount)) * (seededRand.Intn(minTime) + minTime)
-	return time.Duration(delay) * time.Millisecond
+	var delay time.Duration
+
+	// Logic to cap the retry count based on the minDelay provided
+	actualRetryCount := int(math.Log2(float64(minDelay))) + 1
+	if actualRetryCount < 63-retryCount {
+		delay = time.Duration(1<<uint64(retryCount)) * getJitterDelay(minDelay)
+		if delay > maxDelay {
+			delay = getJitterDelay(maxDelay / 2)
+		}
+	} else {
+		delay = getJitterDelay(maxDelay / 2)
+	}
+	return delay + initialDelay
+}
+
+// getJitterDelay returns a jittered delay for retry
+func getJitterDelay(duration time.Duration) time.Duration {
+	return time.Duration(seededRand.Int63n(int64(duration)) + int64(duration))
 }
 
 // ShouldRetry returns true if the request should be retried.
@@ -71,21 +126,23 @@ func (d DefaultRetryer) ShouldRetry(r *Request) bool {
 
 // ShouldThrottle returns true if the request should be throttled.
 func (d DefaultRetryer) shouldThrottle(r *Request) bool {
-	switch r.HTTPResponse.StatusCode {
-	case 429:
-	case 502:
-	case 503:
-	case 504:
-	default:
-		return r.IsErrorThrottle()
+	if r.HTTPResponse != nil {
+		switch r.HTTPResponse.StatusCode {
+		case 429:
+		case 502:
+		case 503:
+		case 504:
+		default:
+			return r.IsErrorThrottle()
+		}
+		return true
 	}
-
-	return true
+	return r.IsErrorThrottle()
 }
 
 // This will look in the Retry-After header, RFC 7231, for how long
 // it will wait before attempting another request
-func getRetryDelay(r *Request) (time.Duration, bool) {
+func getRetryAfterDelay(r *Request) (time.Duration, bool) {
 	if !canUseRetryAfterHeader(r) {
 		return 0, false
 	}

aws/default_retryer_test.go

@@ -56,7 +56,9 @@ func TestRetryThrottleStatusCodes(t *testing.T) {
 		},
 	}
 
-	d := DefaultRetryer{NumMaxRetries: 10}
+	d := NewDefaultRetryer(func(d *DefaultRetryer) {
+		d.NumMaxRetries = 100
+	})
 	for i, c := range cases {
 		throttle := d.shouldThrottle(&c.r)
 		retry := d.ShouldRetry(&c.r)
@@ -71,7 +73,7 @@ func TestRetryThrottleStatusCodes(t *testing.T) {
 	}
 }
 
-func TestCanUseRetryAfter(t *testing.T) {
+func TestGetRetryAfterDelay(t *testing.T) {
 	cases := []struct {
 		r Request
 		e bool
@@ -152,7 +154,7 @@ func TestGetRetryDelay(t *testing.T) {
 	}
 
 	for i, c := range cases {
-		a, ok := getRetryDelay(&c.r)
+		a, ok := getRetryAfterDelay(&c.r)
 		if c.ok != ok {
 			t.Errorf("%d: expected %v, but received %v", i, c.ok, ok)
 		}
@@ -162,3 +164,39 @@ func TestGetRetryDelay(t *testing.T) {
 		}
 	}
 }
+
+func TestRetryDelay(t *testing.T) {
+
+	d := NewDefaultRetryer(func(d *DefaultRetryer) {
+		d.NumMaxRetries = 100
+	})
+
+	r := Request{}
+	for i := 0; i < 100; i++ {
+		rTemp := r
+		rTemp.HTTPResponse = &http.Response{StatusCode: 500, Header: http.Header{"Retry-After": []string{"299"}}}
+		rTemp.RetryCount = i
+		a := d.RetryRules(&rTemp)
+		if a > 5*time.Minute {
+			t.Errorf("retry delay should never be greater than five minutes, received %s for retrycount %d", a, i)
+		}
+	}
+
+	for i := 0; i < 100; i++ {
+		rTemp := r
+		rTemp.RetryCount = i
+		rTemp.HTTPResponse = &http.Response{StatusCode: 503, Header: http.Header{"Retry-After": []string{""}}}
+		a := d.RetryRules(&rTemp)
+		if a > 5*time.Minute {
+			t.Errorf("retry delay should not be greater than five minutes, received %s for retrycount %d", a, i)
+		}
+	}
+
+	rTemp := r
+	rTemp.RetryCount = 1
+	rTemp.HTTPResponse = &http.Response{StatusCode: 503, Header: http.Header{"Retry-After": []string{"300"}}}
+	a := d.RetryRules(&rTemp)
+	if a < 5*time.Minute {
+		t.Errorf("retry delay should not be less than retry-after duration, received %s for retrycount %d", a, 1)
+	}
+}

aws/http_request_retry_test.go

@@ -25,7 +25,9 @@ func TestRequestCancelRetry(t *testing.T) {
 	reqNum := 0
 	cfg := unit.Config()
 	cfg.EndpointResolver = aws.ResolveWithEndpointURL("http://endpoint")
-	cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 10}
+	cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) {
+		d.NumMaxRetries = 10
+	})
 
 	s := mock.NewMockClient(cfg)
 

aws/no_op_retryer.go

@@ -0,0 +1,24 @@
+package aws
+
+import "time"
+
+// NoOpRetryer provides a retryer that performs no retries.
+// It should be used when we do not want retries to be performed.
+type NoOpRetryer struct{}
+
+// MaxRetries returns the number of maximum returns the service will use to make
+// an individual API; For NoOpRetryer the MaxRetries will always be zero.
+func (d NoOpRetryer) MaxRetries() int {
+	return 0
+}
+
+// ShouldRetry will always return false for NoOpRetryer, as it should never retry.
+func (d NoOpRetryer) ShouldRetry(_ *Request) bool {
+	return false
+}
+
+// RetryRules returns the delay duration before retrying this request again;
+// since NoOpRetryer does not retry, RetryRules always returns 0.
+func (d NoOpRetryer) RetryRules(_ *Request) time.Duration {
+	return 0
+}

aws/no_op_retryer_test.go

@@ -0,0 +1,44 @@
+package aws
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestNoOpRetryer(t *testing.T) {
+	cases := []struct {
+		r                Request
+		expectMaxRetries int
+		expectRetryDelay time.Duration
+		expectRetry      bool
+	}{
+		{
+			r: Request{
+				HTTPResponse: &http.Response{StatusCode: 200},
+			},
+			expectMaxRetries: 0,
+			expectRetryDelay: 0,
+			expectRetry:      false,
+		},
+	}
+
+	d := NoOpRetryer{}
+	for i, c := range cases {
+		maxRetries := d.MaxRetries()
+		retry := d.ShouldRetry(&c.r)
+		retryDelay := d.RetryRules(&c.r)
+
+		if e, a := c.expectMaxRetries, maxRetries; e != a {
+			t.Errorf("%d: expected %v, but received %v for number of max retries", i, e, a)
+		}
+
+		if e, a := c.expectRetry, retry; e != a {
+			t.Errorf("%d: expected %v, but received %v for should retry", i, e, a)
+		}
+
+		if e, a := c.expectRetryDelay, retryDelay; e != a {
+			t.Errorf("%d: expected %v, but received %v as retry delay", i, e, a)
+		}
+	}
+}

aws/request.go

@@ -100,6 +100,7 @@ func New(cfg Config, metadata Metadata, handlers Handlers,
 
 	// TODO need better way of handling this error... NewRequest should return error.
 	endpoint, err := cfg.EndpointResolver.ResolveEndpoint(metadata.EndpointsID, cfg.Region)
+
 	if err == nil {
 		// TODO so ugly
 		metadata.Endpoint = endpoint.URL

aws/request_1_6_test.go

@@ -21,7 +21,7 @@ func TestRequestInvalidEndpoint(t *testing.T) {
 		cfg,
 		aws.Metadata{},
 		cfg.Handlers,
-		aws.DefaultRetryer{},
+		aws.NewDefaultRetryer(),
 		&aws.Operation{},
 		nil,
 		nil,