Skip to content
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
a68e370
Fixes bug in calculating throttled retry delay
skotambkar Aug 28, 2019
ec30fec
Update CHANGELOG_PENDING.md
skotambkar Aug 28, 2019
818b3b7
Update CHANGELOG_PENDING.md
skotambkar Aug 28, 2019
c3b6533
Adds suggested changes
skotambkar Aug 28, 2019
9cdb3a9
Adds trailing space in changelog_pending
skotambkar Aug 28, 2019
4fc5212
Ports fix for int overflow in minTime and adds custom retryer for ser…
skotambkar Aug 29, 2019
d11e7f7
changes by go fmt: Fixes formatting of the project
skotambkar Aug 29, 2019
30e6541
Adds changelog pending entry
skotambkar Aug 29, 2019
3e83bc7
Updated Changelog_Pending
skotambkar Aug 29, 2019
8aea163
Updates logic for default retryer
skotambkar Sep 6, 2019
bde7a06
go lint fixes
skotambkar Sep 6, 2019
1844388
fixes go lint
skotambkar Sep 6, 2019
3c1f0c3
updated test for exhaustive retries
skotambkar Sep 6, 2019
7b68b58
fixed failing test case
skotambkar Sep 8, 2019
bd76e4b
adds constructor for default retryer
skotambkar Sep 9, 2019
3d7c0e5
updates test cases to work with constructor for default retryer
skotambkar Sep 9, 2019
5b03a3c
adds no op retryer for zero retries
skotambkar Sep 10, 2019
23d434e
updates to correctly set retryer when service client is initialized
skotambkar Sep 10, 2019
cef80f1
adds suggested changes and handles fatal errors in s3 crypto
skotambkar Sep 11, 2019
fda8bcb
updates change log pending
skotambkar Sep 11, 2019
d813158
go lint changes
skotambkar Sep 11, 2019
38f0857
updates dynamodb customization to use default retryer; adds suggested…
skotambkar Sep 11, 2019
6d52836
adds entries to the changelog pending
skotambkar Sep 11, 2019
cacea80
Merge branch 'master' into issue#370
skotambkar Sep 11, 2019
604e9db
Update CHANGELOG_PENDING.md
skotambkar Sep 11, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion CHANGELOG_PENDING.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
### SDK Features

### SDK Enhancements

* `service/ec2`: Adds custom retryer implementation for service/ec2. [#375](https://github.com/aws/aws-sdk-go/pull/375)
* Adds test case to test custom retryer. Fixes [#370](https://github.com/aws/aws-sdk-go/issues/370)

### SDK Bugs


2 changes: 0 additions & 2 deletions aws/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,7 @@ func NewClient(cfg Config, metadata Metadata) *Client {
retryer = DefaultRetryer{NumMaxRetries: 3}
}
svc.Retryer = retryer

svc.AddDebugHandlers()

return svc
}

Expand Down
115 changes: 93 additions & 22 deletions aws/default_retryer.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package aws

import (
"math"
"math/rand"
"strconv"
"sync"
Expand All @@ -20,9 +21,30 @@ import (
// // 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 max number of retries
DefaultRetryerMaxNumRetries = 3

// DefaultRetryerMinRetryDelay sets min retry delay
DefaultRetryerMinRetryDelay = 30 * time.Millisecond

// DefaultRetryerMinThrottleDelay sets minimum delay when throttled
DefaultRetryerMinThrottleDelay = 500 * time.Millisecond

// DefaultRetryerMaxRetryDelay sets max 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 {
Expand All @@ -31,28 +53,75 @@ func (d DefaultRetryer) MaxRetries() int {

var seededRand = rand.New(&lockedSource{src: rand.NewSource(time.Now().UnixNano())})

// NewDefaultRetryer returns Default Retryer initialized with default values and optionally takes function
// to override values for default retryer.
func NewDefaultRetryer(opts ...func(d *DefaultRetryer)) DefaultRetryer {
d := DefaultRetryer{}
d.setDefaults()
for _, opt := range opts {
opt(&d)
}
return d
}

// setDefaults sets default values for the default Retryer
func (d *DefaultRetryer) setDefaults() {
if d.NumMaxRetries == 0 {
d.NumMaxRetries = DefaultRetryerMaxNumRetries
}
if d.MinRetryDelay == 0 {
d.MinRetryDelay = DefaultRetryerMinRetryDelay
}
if d.MinThrottleDelay == 0 {
d.MinThrottleDelay = DefaultRetryerMinThrottleDelay
}
if d.MaxRetryDelay == 0 {
d.MaxRetryDelay = DefaultRetryerMaxRetryDelay
}
if d.MaxThrottleDelay == 0 {
d.MaxThrottleDelay = DefaultRetryerMaxThrottleDelay
}

}

// 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
}

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)) * getDelaySeed(minDelay)
if delay > maxDelay {
delay = getDelaySeed(maxDelay / 2)
}
} else {
delay = getDelaySeed(maxDelay / 2)
}
return delay + initialDelay
}

delay := (1 << uint(retryCount)) * (seededRand.Intn(minTime) + minTime)
return time.Duration(delay) * time.Millisecond
func getDelaySeed(duration time.Duration) time.Duration {
return time.Duration(seededRand.Int63n(int64(duration)) + int64(duration))
}

// ShouldRetry returns true if the request should be retried.
Expand All @@ -71,21 +140,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
}
Expand Down
42 changes: 40 additions & 2 deletions aws/default_retryer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
}
4 changes: 3 additions & 1 deletion aws/http_request_retry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
1 change: 1 addition & 0 deletions aws/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion aws/request_1_6_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func TestRequestInvalidEndpoint(t *testing.T) {
cfg,
aws.Metadata{},
cfg.Handlers,
aws.DefaultRetryer{},
aws.NewDefaultRetryer(),
&aws.Operation{},
nil,
nil,
Expand Down
16 changes: 12 additions & 4 deletions aws/request_pagination_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ func TestPagination(t *testing.T) {
},
}

retryer := aws.DefaultRetryer{NumMaxRetries: 2}
retryer := aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) {
d.NumMaxRetries = 2
})
op := aws.Operation{
Name: "Operation",
Paginator: &aws.Paginator{
Expand Down Expand Up @@ -160,7 +162,9 @@ func TestPaginationTruncation(t *testing.T) {
}

reqNum := 0
retryer := aws.DefaultRetryer{NumMaxRetries: 2}
retryer := aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) {
d.NumMaxRetries = 2
})
ops := []aws.Operation{
{
Name: "Operation",
Expand Down Expand Up @@ -271,7 +275,9 @@ func BenchmarkPagination(b *testing.B) {
{aws.String("3"), aws.String("")},
}

retryer := aws.DefaultRetryer{NumMaxRetries: 2}
retryer := aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) {
d.NumMaxRetries = 2
})
op := aws.Operation{
Name: "Operation",
Paginator: &aws.Paginator{
Expand Down Expand Up @@ -339,7 +345,9 @@ func TestPaginationWithContextCancel(t *testing.T) {
},
}

retryer := aws.DefaultRetryer{NumMaxRetries: 2}
retryer := aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) {
d.NumMaxRetries = 2
})
op := aws.Operation{
Name: "Operation",
Paginator: &aws.Paginator{
Expand Down
Loading