From a68e370351c9fd58399be797a5be19d82b1c40f5 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Wed, 28 Aug 2019 15:02:49 -0700 Subject: [PATCH 01/24] Fixes bug in calculating throttled retry delay --- CHANGELOG_PENDING.md | 4 ++++ aws/default_retryer.go | 18 ++++++++++-------- aws/default_retryer_test.go | 36 +++++++++++++++++++++++++++++++++++- 3 files changed, 49 insertions(+), 9 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index eca8f3ba1d6..bca7c5a9ef0 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -3,4 +3,8 @@ ### SDK Enhancements ### SDK Bugs +* `aws/client`: Fixes bug where the throttled retry's math was off + * The `Retry-After` duration specified in the request is now added to the Retry delay for throttled exception + * Adds test for retry delays for throttled exceptions [#370](https://github.com/aws/aws-sdk-go/pull/370) + * Fixes [#45](https://github.com/aws/aws-sdk-go/issues/45) diff --git a/aws/default_retryer.go b/aws/default_retryer.go index a08af806de6..8d4946f5aaa 100644 --- a/aws/default_retryer.go +++ b/aws/default_retryer.go @@ -34,10 +34,12 @@ var seededRand = rand.New(&lockedSource{src: rand.NewSource(time.Now().UnixNano( // RetryRules returns the delay duration before retrying this request again func (d DefaultRetryer) RetryRules(r *Request) time.Duration { // Set the upper limit of delay in retrying at ~five minutes - minTime := 30 + var minTime int64 = 30 + var initialDelay time.Duration + throttle := d.shouldThrottle(r) if throttle { - if delay, ok := getRetryDelay(r); ok { + if delay, ok := getRetryAfterDelay(r); ok { return delay } @@ -45,14 +47,14 @@ func (d DefaultRetryer) RetryRules(r *Request) time.Duration { } retryCount := r.RetryCount - if retryCount > 13 { - retryCount = 13 - } else if throttle && retryCount > 8 { + if throttle && retryCount > 8 { retryCount = 8 + } else if retryCount > 12 { + retryCount = 12 } - delay := (1 << uint(retryCount)) * (seededRand.Intn(minTime) + minTime) - return time.Duration(delay) * time.Millisecond + delay := (1 << uint(retryCount)) * (seededRand.Int63n(minTime) + minTime) + return (time.Duration(delay) * time.Millisecond) + initialDelay } // ShouldRetry returns true if the request should be retried. @@ -85,7 +87,7 @@ func (d DefaultRetryer) shouldThrottle(r *Request) bool { // 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 } diff --git a/aws/default_retryer_test.go b/aws/default_retryer_test.go index bc38a4e6902..c9f6b2180e8 100644 --- a/aws/default_retryer_test.go +++ b/aws/default_retryer_test.go @@ -152,7 +152,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 +162,37 @@ func TestGetRetryDelay(t *testing.T) { } } } + +func TestRetryDelay(t *testing.T) { + d:= DefaultRetryer{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) + } + } + + for i := 0; i < 100; i++ { + rTemp := r + rTemp.RetryCount = i + 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 value, received %s for retrycount %d", a, i) + } + } +} \ No newline at end of file From ec30fecac61bb9eeac630c7461ad561ba975f324 Mon Sep 17 00:00:00 2001 From: Shantanu Kotambkar <52007797+skotambkar@users.noreply.github.com> Date: Wed, 28 Aug 2019 15:07:29 -0700 Subject: [PATCH 02/24] Update CHANGELOG_PENDING.md --- CHANGELOG_PENDING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index bca7c5a9ef0..3fd8dc0f438 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -3,7 +3,7 @@ ### SDK Enhancements ### SDK Bugs -* `aws/client`: Fixes bug where the throttled retry's math was off +* `aws`: Fixes bug where the throttled retry's math was off * The `Retry-After` duration specified in the request is now added to the Retry delay for throttled exception * Adds test for retry delays for throttled exceptions [#370](https://github.com/aws/aws-sdk-go/pull/370) * Fixes [#45](https://github.com/aws/aws-sdk-go/issues/45) From 818b3b7f3b3aee97b7e30985854a4d7cdddacf2d Mon Sep 17 00:00:00 2001 From: Shantanu Kotambkar <52007797+skotambkar@users.noreply.github.com> Date: Wed, 28 Aug 2019 15:08:00 -0700 Subject: [PATCH 03/24] Update CHANGELOG_PENDING.md --- CHANGELOG_PENDING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 3fd8dc0f438..719ade174ce 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -5,6 +5,6 @@ ### SDK Bugs * `aws`: Fixes bug where the throttled retry's math was off * The `Retry-After` duration specified in the request is now added to the Retry delay for throttled exception - * Adds test for retry delays for throttled exceptions [#370](https://github.com/aws/aws-sdk-go/pull/370) + * Adds test for retry delays for throttled exceptions [#373](https://github.com/aws/aws-sdk-go/pull/373) * Fixes [#45](https://github.com/aws/aws-sdk-go/issues/45) From c3b6533b8279a472d1e974e7131d614499336899 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Wed, 28 Aug 2019 15:49:28 -0700 Subject: [PATCH 04/24] Adds suggested changes --- CHANGELOG_PENDING.md | 6 ++---- aws/default_retryer.go | 2 +- aws/default_retryer_test.go | 18 ++++++++---------- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 719ade174ce..f4aa3293191 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -3,8 +3,6 @@ ### SDK Enhancements ### SDK Bugs -* `aws`: Fixes bug where the throttled retry's math was off - * The `Retry-After` duration specified in the request is now added to the Retry delay for throttled exception - * Adds test for retry delays for throttled exceptions [#373](https://github.com/aws/aws-sdk-go/pull/373) - * Fixes [#45](https://github.com/aws/aws-sdk-go/issues/45) +* `aws`: The `Retry-After` duration specified in the request is now added to the Retry delay for throttled exception. Adds test for retry delays for throttled exceptions. [#373](https://github.com/aws/aws-sdk-go/pull/373) + * Fixes bug where the throttled retry's math was off. Fixes [#45](https://github.com/aws/aws-sdk-go/issues/45) diff --git a/aws/default_retryer.go b/aws/default_retryer.go index 8d4946f5aaa..bcf6fcce7c2 100644 --- a/aws/default_retryer.go +++ b/aws/default_retryer.go @@ -40,7 +40,7 @@ func (d DefaultRetryer) RetryRules(r *Request) time.Duration { throttle := d.shouldThrottle(r) if throttle { if delay, ok := getRetryAfterDelay(r); ok { - return delay + initialDelay = delay } minTime = 500 diff --git a/aws/default_retryer_test.go b/aws/default_retryer_test.go index c9f6b2180e8..7bb485084de 100644 --- a/aws/default_retryer_test.go +++ b/aws/default_retryer_test.go @@ -164,7 +164,7 @@ func TestGetRetryDelay(t *testing.T) { } func TestRetryDelay(t *testing.T) { - d:= DefaultRetryer{100} + d := DefaultRetryer{100} r := Request{} for i := 0; i < 100; i++ { rTemp := r @@ -186,13 +186,11 @@ func TestRetryDelay(t *testing.T) { } } - for i := 0; i < 100; i++ { - rTemp := r - rTemp.RetryCount = i - 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 value, 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) } -} \ No newline at end of file +} From 9cdb3a9f2859f9b564b3dfae67bc29f8d8c859a0 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Wed, 28 Aug 2019 15:50:34 -0700 Subject: [PATCH 05/24] Adds trailing space in changelog_pending --- CHANGELOG_PENDING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index f4aa3293191..d366aaa67d4 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -5,4 +5,4 @@ ### SDK Bugs * `aws`: The `Retry-After` duration specified in the request is now added to the Retry delay for throttled exception. Adds test for retry delays for throttled exceptions. [#373](https://github.com/aws/aws-sdk-go/pull/373) * Fixes bug where the throttled retry's math was off. Fixes [#45](https://github.com/aws/aws-sdk-go/issues/45) - + \ No newline at end of file From 4fc52124bd1586a013d17a09113f92f7da79e449 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Thu, 29 Aug 2019 11:10:58 -0700 Subject: [PATCH 06/24] Ports fix for int overflow in minTime and adds custom retryer for service/ec2 --- aws/client.go | 3 ++- aws/config.go | 5 +++++ aws/default_retryer.go | 37 +++++++++++++++++++++--------- aws/default_retryer_test.go | 4 ++-- service/ec2/customizations.go | 36 ++++++++++++++++++++++++++++++ service/ec2/retryer_test.go | 42 +++++++++++++++++++++++++++++++++++ 6 files changed, 113 insertions(+), 14 deletions(-) create mode 100644 service/ec2/retryer_test.go diff --git a/aws/client.go b/aws/client.go index 8e90d2e96d6..62740ba8615 100644 --- a/aws/client.go +++ b/aws/client.go @@ -64,7 +64,7 @@ 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 = DefaultRetryer{3, 0} } svc.Retryer = retryer @@ -76,6 +76,7 @@ func NewClient(cfg Config, metadata Metadata) *Client { // NewRequest returns a new Request pointer for the service API // operation and parameters. func (c *Client) NewRequest(operation *Operation, params interface{}, data interface{}) *Request { + return New(c.Config, c.Metadata, c.Handlers, c.Retryer, operation, params, data) } diff --git a/aws/config.go b/aws/config.go index a695e684170..616cfeea302 100644 --- a/aws/config.go +++ b/aws/config.go @@ -1,5 +1,10 @@ package aws +// UseServiceDefaultRetries instructs the config to use the service's own +// default number of retries. This will be the default action if +// Config.MaxRetries is nil also. +const UseServiceDefaultRetries = -1 + // A Config provides service configuration for service clients. type Config struct { // The region to send requests to. This parameter is required and must diff --git a/aws/default_retryer.go b/aws/default_retryer.go index bcf6fcce7c2..23439173ad4 100644 --- a/aws/default_retryer.go +++ b/aws/default_retryer.go @@ -21,6 +21,7 @@ import ( // func (d retryer) MaxRetries() int { return 100 } type DefaultRetryer struct { NumMaxRetries int + MinTime int64 } // MaxRetries returns the number of maximum returns the service will use to make @@ -34,9 +35,14 @@ var seededRand = rand.New(&lockedSource{src: rand.NewSource(time.Now().UnixNano( // RetryRules returns the delay duration before retrying this request again func (d DefaultRetryer) RetryRules(r *Request) time.Duration { // Set the upper limit of delay in retrying at ~five minutes - var minTime int64 = 30 - var initialDelay time.Duration + var minTime int64 + if d.MinTime != 0 { + minTime = d.MinTime + } else { + minTime = 30 + } + var initialDelay time.Duration throttle := d.shouldThrottle(r) if throttle { if delay, ok := getRetryAfterDelay(r); ok { @@ -53,6 +59,13 @@ func (d DefaultRetryer) RetryRules(r *Request) time.Duration { retryCount = 12 } + maxRetriesAllowed := d.MaxRetries() + if maxRetriesAllowed != 0 { + if retryCount > maxRetriesAllowed-1 { + retryCount = maxRetriesAllowed - 1 + } + } + delay := (1 << uint(retryCount)) * (seededRand.Int63n(minTime) + minTime) return (time.Duration(delay) * time.Millisecond) + initialDelay } @@ -73,16 +86,18 @@ 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 diff --git a/aws/default_retryer_test.go b/aws/default_retryer_test.go index 7bb485084de..4285815c565 100644 --- a/aws/default_retryer_test.go +++ b/aws/default_retryer_test.go @@ -164,7 +164,7 @@ func TestGetRetryDelay(t *testing.T) { } func TestRetryDelay(t *testing.T) { - d := DefaultRetryer{100} + d := DefaultRetryer{100, 0} r := Request{} for i := 0; i < 100; i++ { rTemp := r @@ -190,7 +190,7 @@ func TestRetryDelay(t *testing.T) { 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{ + if a < 5*time.Minute { t.Errorf("retry delay should not be less than retry-after duration, received %s for retrycount %d", a, 1) } } diff --git a/service/ec2/customizations.go b/service/ec2/customizations.go index be2abb295b0..eed45a867e4 100644 --- a/service/ec2/customizations.go +++ b/service/ec2/customizations.go @@ -8,7 +8,43 @@ import ( "github.com/aws/aws-sdk-go-v2/internal/awsutil" ) +type retryer struct { + aws.DefaultRetryer +} + +func (d retryer) RetryRules(r *request.Request) time.Duration { + switch r.Operation.Name { + case opModifyNetworkInterfaceAttribute: + fallthrough + case opAssignPrivateIpAddresses: + return d.customRetryRule(r) + default: + return d.DefaultRetryer.RetryRules(r) + } +} + +func (d retryer) customRetryRule(r *request.Request) time.Duration { + d.MinTime = 1000 + return d.DefaultRetryer.RetryRules(r) +} + +func setCustomRetryer(c *Client) { + maxRetries := 3 + c.Retryer = retryer{ + DefaultRetryer: request.DefaultRetryer{ + NumMaxRetries: maxRetries, + }, + } +} + func init() { + initClient = func(c *Client) { + if c.Config.Retryer == nil { + // Only override the retryer with a custom one if the config + // does not already contain a retryer + setCustomRetryer(c) + } + } initRequest = func(c *Client, r *request.Request) { if r.Operation.Name == opCopySnapshot { // fill the PresignedURL parameter r.Handlers.Build.PushFront(fillPresignedURL) diff --git a/service/ec2/retryer_test.go b/service/ec2/retryer_test.go new file mode 100644 index 00000000000..7d45e53f2f6 --- /dev/null +++ b/service/ec2/retryer_test.go @@ -0,0 +1,42 @@ +package ec2 + +import ( + "testing" + "time" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/internal/awstesting/unit" +) + +func TestCustomRetryer(t *testing.T) { + + svc := New(unit.Config()) + + if _, ok := svc.Client.Retryer.(retryer); !ok { + t.Error("expected custom retryer, but received otherwise") + } + + req := svc.ModifyNetworkInterfaceAttributeRequest(&ModifyNetworkInterfaceAttributeInput{ + NetworkInterfaceId: aws.String("foo"), + }) + + duration := svc.Client.Retryer.RetryRules(req.Request) + if duration < time.Second*1 || duration > time.Second*2 { + t.Errorf("expected duration to be between 1s and 2s, but received %s", duration) + } + + req.Request.RetryCount = 15 + duration = svc.Client.Retryer.RetryRules(req.Request) + + if duration < time.Second*4 || duration > time.Second*8 { + t.Errorf("expected duration to be between 4s and 8s, but received %s", duration) + } + + svc = New(aws.Config{ + Region: "us-west-2", + Retryer: aws.DefaultRetryer{}}, + ) + if _, ok := svc.Client.Retryer.(aws.DefaultRetryer); !ok { + t.Error("expected default retryer, but received otherwise") + } +} From d11e7f7e41cffab4165fb4a955ad8c69949d18ab Mon Sep 17 00:00:00 2001 From: skotambkar Date: Thu, 29 Aug 2019 11:12:12 -0700 Subject: [PATCH 07/24] changes by go fmt: Fixes formatting of the project --- aws/request.go | 1 + aws/signer/v4/v4.go | 38 +++++++++---------- private/model/api/shape.go | 10 ++--- private/model/api/shape_marshal.go | 19 +++++----- private/protocol/ec2query/build_bench_test.go | 2 +- private/protocol/fields.go | 8 ++-- .../json/jsonutil/unmarshal_bench_test.go | 2 +- private/protocol/timestamp.go | 6 +-- 8 files changed, 43 insertions(+), 43 deletions(-) diff --git a/aws/request.go b/aws/request.go index 11c561d5249..c5928185819 100644 --- a/aws/request.go +++ b/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 diff --git a/aws/signer/v4/v4.go b/aws/signer/v4/v4.go index ddbade68697..943174407a6 100644 --- a/aws/signer/v4/v4.go +++ b/aws/signer/v4/v4.go @@ -96,25 +96,25 @@ var ignoredHeaders = rules{ var requiredSignedHeaders = rules{ whitelist{ mapRule{ - "Cache-Control": struct{}{}, - "Content-Disposition": struct{}{}, - "Content-Encoding": struct{}{}, - "Content-Language": struct{}{}, - "Content-Md5": struct{}{}, - "Content-Type": struct{}{}, - "Expires": struct{}{}, - "If-Match": struct{}{}, - "If-Modified-Since": struct{}{}, - "If-None-Match": struct{}{}, - "If-Unmodified-Since": struct{}{}, - "Range": struct{}{}, - "X-Amz-Acl": struct{}{}, - "X-Amz-Copy-Source": struct{}{}, - "X-Amz-Copy-Source-If-Match": struct{}{}, - "X-Amz-Copy-Source-If-Modified-Since": struct{}{}, - "X-Amz-Copy-Source-If-None-Match": struct{}{}, - "X-Amz-Copy-Source-If-Unmodified-Since": struct{}{}, - "X-Amz-Copy-Source-Range": struct{}{}, + "Cache-Control": struct{}{}, + "Content-Disposition": struct{}{}, + "Content-Encoding": struct{}{}, + "Content-Language": struct{}{}, + "Content-Md5": struct{}{}, + "Content-Type": struct{}{}, + "Expires": struct{}{}, + "If-Match": struct{}{}, + "If-Modified-Since": struct{}{}, + "If-None-Match": struct{}{}, + "If-Unmodified-Since": struct{}{}, + "Range": struct{}{}, + "X-Amz-Acl": struct{}{}, + "X-Amz-Copy-Source": struct{}{}, + "X-Amz-Copy-Source-If-Match": struct{}{}, + "X-Amz-Copy-Source-If-Modified-Since": struct{}{}, + "X-Amz-Copy-Source-If-None-Match": struct{}{}, + "X-Amz-Copy-Source-If-Unmodified-Since": struct{}{}, + "X-Amz-Copy-Source-Range": struct{}{}, "X-Amz-Copy-Source-Server-Side-Encryption-Customer-Algorithm": struct{}{}, "X-Amz-Copy-Source-Server-Side-Encryption-Customer-Key": struct{}{}, "X-Amz-Copy-Source-Server-Side-Encryption-Customer-Key-Md5": struct{}{}, diff --git a/private/model/api/shape.go b/private/model/api/shape.go index fb20130e669..ad9d3af758f 100644 --- a/private/model/api/shape.go +++ b/private/model/api/shape.go @@ -30,11 +30,11 @@ type ShapeRef struct { Ignore bool XMLNamespace XMLInfo Payload string - IdempotencyToken bool `json:"idempotencyToken"` + IdempotencyToken bool `json:"idempotencyToken"` TimestampFormat string `json:"timestampFormat"` - JSONValue bool `json:"jsonvalue"` - Deprecated bool `json:"deprecated"` - HostLabel bool `json:"hostLabel"` + JSONValue bool `json:"jsonvalue"` + Deprecated bool `json:"deprecated"` + HostLabel bool `json:"hostLabel"` OrigShapeName string `json:"-"` @@ -84,7 +84,7 @@ type Shape struct { Streaming bool Location string LocationName string - IdempotencyToken bool `json:"idempotencyToken"` + IdempotencyToken bool `json:"idempotencyToken"` TimestampFormat string `json:"timestampFormat"` XMLNamespace XMLInfo Min float64 // optional Minimum length (string, list) or value (number) diff --git a/private/model/api/shape_marshal.go b/private/model/api/shape_marshal.go index 9ee9cfe5adb..964ea0d9c54 100644 --- a/private/model/api/shape_marshal.go +++ b/private/model/api/shape_marshal.go @@ -79,7 +79,7 @@ func getContentType(s *Shape) string { if s.API.Metadata.JSONVersion != "" && s.API.Metadata.Protocol == "json" { return fmt.Sprintf("application/x-amz-json-%s", s.API.Metadata.JSONVersion) } - if s.API.Metadata.Protocol == "json" || s.API.Metadata.Protocol == "rest-json" { + if s.API.Metadata.Protocol == "json" || s.API.Metadata.Protocol == "rest-json" { return "application/json" } return "" @@ -90,8 +90,7 @@ var marshalShapeTmpl = template.Must(template.New("marshalShapeTmpl").Funcs( "MarshalShapeRefGoCode": MarshalShapeRefGoCode, "nestedRefsByLocation": nestedRefsByLocation, "isShapeFieldsNested": isShapeFieldsNested, - "getContentType": getContentType, - + "getContentType": getContentType, }, ).Parse(` {{ define "encode shape" -}} @@ -207,15 +206,15 @@ func isShapeFieldsNested(loc string, s *Shape) bool { return loc == "Body" && len(s.LocationName) != 0 && s.API.Metadata.Protocol == "rest-xml" } -func QuotedFormatTime(s marshalShapeRef) string { - if (s.Ref.API.Metadata.Protocol == "json" || s.Ref.API.Metadata.Protocol == "rest-json") && s.Location() == "Body" { +func QuotedFormatTime(s marshalShapeRef) string { + if (s.Ref.API.Metadata.Protocol == "json" || s.Ref.API.Metadata.Protocol == "rest-json") && s.Location() == "Body" { return "true" } return "false" } var marshalShapeRefTmpl = template.Must(template.New("marshalShapeRefTmpl").Funcs(template.FuncMap{ - "Collection": Collection, + "Collection": Collection, "quotedFormatTime": QuotedFormatTime, }).Parse(` {{ define "encode field" -}} @@ -723,11 +722,11 @@ func (r marshalShapeRef) IsIdempotencyToken() bool { } func (r marshalShapeRef) TimeFormat() string { - if r.Ref.TimestampFormat!="" { - return fmt.Sprintf("%q",r.Ref.TimestampFormat) + if r.Ref.TimestampFormat != "" { + return fmt.Sprintf("%q", r.Ref.TimestampFormat) } - if r.Ref.Shape.TimestampFormat!= "" { - return fmt.Sprintf("%q",r.Ref.Shape.TimestampFormat) + if r.Ref.Shape.TimestampFormat != "" { + return fmt.Sprintf("%q", r.Ref.Shape.TimestampFormat) } switch r.Location() { diff --git a/private/protocol/ec2query/build_bench_test.go b/private/protocol/ec2query/build_bench_test.go index 54c3895f33b..875d0a33dcc 100644 --- a/private/protocol/ec2query/build_bench_test.go +++ b/private/protocol/ec2query/build_bench_test.go @@ -49,7 +49,7 @@ func BenchmarkEC2QueryBuild_Complex_ec2AuthorizeSecurityGroupEgress(b *testing.B IpProtocol: aws.String("String"), SourceSecurityGroupName: aws.String("String"), SourceSecurityGroupOwnerId: aws.String("String"), - ToPort: aws.Int64(1), + ToPort: aws.Int64(1), } benchEC2QueryBuild(b, "AuthorizeSecurityGroupEgress", params) diff --git a/private/protocol/fields.go b/private/protocol/fields.go index 41aa6f6703f..3ea555caf62 100644 --- a/private/protocol/fields.go +++ b/private/protocol/fields.go @@ -135,8 +135,8 @@ func (v JSONValue) MarshalValueBuf(b []byte) ([]byte, error) { // TimeValue provies encoding of time.Time for AWS protocols. type TimeValue struct { - V time.Time - Format string + V time.Time + Format string QuotedFormatTime bool } @@ -148,8 +148,8 @@ func (v TimeValue) MarshalValue() (string, error) { } if v.QuotedFormatTime { - format, err := FormatTime(v.Format, v.V) - return fmt.Sprintf("%q",format),err + format, err := FormatTime(v.Format, v.V) + return fmt.Sprintf("%q", format), err } return FormatTime(v.Format, v.V) diff --git a/private/protocol/json/jsonutil/unmarshal_bench_test.go b/private/protocol/json/jsonutil/unmarshal_bench_test.go index 291f5cd8b60..4c3f2c2ba5d 100644 --- a/private/protocol/json/jsonutil/unmarshal_bench_test.go +++ b/private/protocol/json/jsonutil/unmarshal_bench_test.go @@ -12,7 +12,7 @@ import ( ) var ( - simpleJSON = []byte(`{"FooEnum": "foo", "ListEnums": ["0", "1"]}`) + simpleJSON = []byte(`{"FooEnum": "foo", "ListEnums": ["0", "1"]}`) complexJSON = []byte(`{"Table":{"AttributeDefinitions":[{"AttributeName":"1","AttributeType":"N"}],"CreationDateTime":1.562054355238E9,"ItemCount":0,"KeySchema":[{"AttributeName":"1","KeyType":"HASH"}],"ProvisionedThroughput":{"NumberOfDecreasesToday":0,"ReadCapacityUnits":5,"WriteCapacityUnits":5},"TableArn":"arn:aws:dynamodb:us-west-2:183557167593:table/TestTable","TableId":"575d0be6-34e3-4843-838c-8e8e8d4ea2f7","TableName":"TestTable","TableSizeBytes":0,"TableStatus":"ACTIVE"}}`) ) diff --git a/private/protocol/timestamp.go b/private/protocol/timestamp.go index 5f6177548a1..f750ceb81d6 100644 --- a/private/protocol/timestamp.go +++ b/private/protocol/timestamp.go @@ -38,7 +38,7 @@ func IsKnownTimestampFormat(name string) bool { } // FormatTime returns a string value of the time. -func FormatTime(name string, t time.Time) (string,error) { +func FormatTime(name string, t time.Time) (string, error) { t = t.UTC() switch name { @@ -49,7 +49,7 @@ func FormatTime(name string, t time.Time) (string,error) { case UnixTimeFormatName: return strconv.FormatInt(t.Unix(), 10), nil default: - return "",fmt.Errorf("unknown timestamp format name, " + name) + return "", fmt.Errorf("unknown timestamp format name, " + name) } } @@ -67,7 +67,7 @@ func ParseTime(formatName, value string) (time.Time, error) { return time.Time{}, err } t := time.Unix(int64(v), 0) - return t.UTC(),nil + return t.UTC(), nil default: panic("unknown timestamp format name, " + formatName) From 30e6541f2e58c0a8b6947ae22ba8857feb1a8c78 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Thu, 29 Aug 2019 11:16:05 -0700 Subject: [PATCH 08/24] Adds changelog pending entry --- CHANGELOG_PENDING.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index d366aaa67d4..b1763e3cddd 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -1,8 +1,9 @@ ### SDK Features ### SDK Enhancements - +* `service/ec2`: Adds custom retryer implementation for service/ec2. [#374](https://github.com/aws/aws-sdk-go/pull/374) + * Adds test case to test custom retryer. Fixes [#370](https://github.com/aws/aws-sdk-go/issues/370) + ### SDK Bugs -* `aws`: The `Retry-After` duration specified in the request is now added to the Retry delay for throttled exception. Adds test for retry delays for throttled exceptions. [#373](https://github.com/aws/aws-sdk-go/pull/373) - * Fixes bug where the throttled retry's math was off. Fixes [#45](https://github.com/aws/aws-sdk-go/issues/45) + \ No newline at end of file From 3e83bc75330f5b1d7ccbb2b7062b68025dfee608 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Thu, 29 Aug 2019 11:19:31 -0700 Subject: [PATCH 09/24] Updated Changelog_Pending --- CHANGELOG_PENDING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index b1763e3cddd..ac2b7150e44 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -1,7 +1,7 @@ ### SDK Features ### SDK Enhancements -* `service/ec2`: Adds custom retryer implementation for service/ec2. [#374](https://github.com/aws/aws-sdk-go/pull/374) +* `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 From 8aea163294e6022ab4350eb073b804d9b78240b6 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Fri, 6 Sep 2019 14:20:32 -0700 Subject: [PATCH 10/24] Updates logic for default retryer --- aws/client.go | 5 +- aws/config.go | 5 -- aws/default_retryer.go | 80 +++++++++++++++++++++++--------- aws/default_retryer_test.go | 2 +- service/ec2/customizations.go | 54 ++++++++-------------- service/ec2/retryer_test.go | 86 ++++++++++++++++++++++++++++++----- 6 files changed, 153 insertions(+), 79 deletions(-) diff --git a/aws/client.go b/aws/client.go index 62740ba8615..704bbe10409 100644 --- a/aws/client.go +++ b/aws/client.go @@ -64,19 +64,16 @@ func NewClient(cfg Config, metadata Metadata) *Client { retryer := cfg.Retryer if retryer == nil { // TODO need better way of specifing default num retries - retryer = DefaultRetryer{3, 0} + retryer = DefaultRetryer{NumMaxRetries: 3} } svc.Retryer = retryer - svc.AddDebugHandlers() - return svc } // NewRequest returns a new Request pointer for the service API // operation and parameters. func (c *Client) NewRequest(operation *Operation, params interface{}, data interface{}) *Request { - return New(c.Config, c.Metadata, c.Handlers, c.Retryer, operation, params, data) } diff --git a/aws/config.go b/aws/config.go index 616cfeea302..a695e684170 100644 --- a/aws/config.go +++ b/aws/config.go @@ -1,10 +1,5 @@ package aws -// UseServiceDefaultRetries instructs the config to use the service's own -// default number of retries. This will be the default action if -// Config.MaxRetries is nil also. -const UseServiceDefaultRetries = -1 - // A Config provides service configuration for service clients. type Config struct { // The region to send requests to. This parameter is required and must diff --git a/aws/default_retryer.go b/aws/default_retryer.go index 23439173ad4..7410a85a1d1 100644 --- a/aws/default_retryer.go +++ b/aws/default_retryer.go @@ -1,6 +1,7 @@ package aws import ( + "math" "math/rand" "strconv" "sync" @@ -20,10 +21,22 @@ import ( // // This implementation always has 100 max retries // func (d retryer) MaxRetries() int { return 100 } type DefaultRetryer struct { - NumMaxRetries int - MinTime int64 + // num of max retries allowed + NumMaxRetries int + MinRetryDelay time.Duration + MinThrottleDelay time.Duration + MaxRetryDelay time.Duration + MaxThrottleDelay time.Duration } +const ( + DefaultRetryerMaxNumRetries = 3 + DefaultRetryerMinRetryDelay = 30 * time.Millisecond + DefaultRetryerMinThrottleDelay = 500 * time.Millisecond + DefaultRetryerMaxRetryDelay = 300 * time.Second + 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 { @@ -32,42 +45,67 @@ func (d DefaultRetryer) MaxRetries() int { var seededRand = rand.New(&lockedSource{src: rand.NewSource(time.Now().UnixNano())}) +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 - var minTime int64 - if d.MinTime != 0 { - minTime = d.MinTime - } else { - minTime = 30 - } + // set default values for the retryer if not set. + d.setDefaults() + + // Set the upper limit of delay in retrying at ~five minutes + minDelay := d.MinRetryDelay var initialDelay time.Duration throttle := d.shouldThrottle(r) if throttle { if delay, ok := getRetryAfterDelay(r); ok { initialDelay = delay } - - minTime = 500 + minDelay = d.MinThrottleDelay } retryCount := r.RetryCount - if throttle && retryCount > 8 { - retryCount = 8 - } else if retryCount > 12 { - retryCount = 12 + maxDelay := d.MaxRetryDelay + if throttle { + maxDelay = d.MaxThrottleDelay } - maxRetriesAllowed := d.MaxRetries() - if maxRetriesAllowed != 0 { - if retryCount > maxRetriesAllowed-1 { - retryCount = maxRetriesAllowed - 1 + 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< maxDelay { + delay = getDelaySeed(maxDelay / 2) } + } else { + delay = getDelaySeed(maxDelay / 2) } + return delay + initialDelay +} - delay := (1 << uint(retryCount)) * (seededRand.Int63n(minTime) + minTime) - return (time.Duration(delay) * time.Millisecond) + initialDelay +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. diff --git a/aws/default_retryer_test.go b/aws/default_retryer_test.go index 4285815c565..75551cae1f1 100644 --- a/aws/default_retryer_test.go +++ b/aws/default_retryer_test.go @@ -164,7 +164,7 @@ func TestGetRetryDelay(t *testing.T) { } func TestRetryDelay(t *testing.T) { - d := DefaultRetryer{100, 0} + d := DefaultRetryer{NumMaxRetries: 100} r := Request{} for i := 0; i < 100; i++ { rTemp := r diff --git a/service/ec2/customizations.go b/service/ec2/customizations.go index eed45a867e4..ec91b57ec1e 100644 --- a/service/ec2/customizations.go +++ b/service/ec2/customizations.go @@ -4,55 +4,37 @@ import ( "time" "github.com/aws/aws-sdk-go-v2/aws" - request "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/internal/awsutil" ) -type retryer struct { - aws.DefaultRetryer -} - -func (d retryer) RetryRules(r *request.Request) time.Duration { - switch r.Operation.Name { - case opModifyNetworkInterfaceAttribute: - fallthrough - case opAssignPrivateIpAddresses: - return d.customRetryRule(r) - default: - return d.DefaultRetryer.RetryRules(r) - } -} - -func (d retryer) customRetryRule(r *request.Request) time.Duration { - d.MinTime = 1000 - return d.DefaultRetryer.RetryRules(r) -} +const ( + customRetryerMaxNumRetries = 3 + customRetryerMinRetryDelay = 1 * time.Second + customRetryerMaxRetryDelay = 8 * time.Second +) -func setCustomRetryer(c *Client) { - maxRetries := 3 - c.Retryer = retryer{ - DefaultRetryer: request.DefaultRetryer{ - NumMaxRetries: maxRetries, - }, +func setCustomRetryer(c *Client) aws.Retryer { + return aws.DefaultRetryer{ + NumMaxRetries: customRetryerMaxNumRetries, + MinRetryDelay: customRetryerMinRetryDelay, + MinThrottleDelay: customRetryerMinRetryDelay, + MaxRetryDelay: customRetryerMaxRetryDelay, + MaxThrottleDelay: customRetryerMaxRetryDelay, } } func init() { - initClient = func(c *Client) { - if c.Config.Retryer == nil { - // Only override the retryer with a custom one if the config - // does not already contain a retryer - setCustomRetryer(c) - } - } - initRequest = func(c *Client, r *request.Request) { + initRequest = func(c *Client, r *aws.Request) { if r.Operation.Name == opCopySnapshot { // fill the PresignedURL parameter r.Handlers.Build.PushFront(fillPresignedURL) } + if c.Config.Retryer == nil && (r.Operation.Name == opModifyNetworkInterfaceAttribute || r.Operation.Name == opAssignPrivateIpAddresses) { + r.Retryer = setCustomRetryer(c) + } } } -func fillPresignedURL(r *request.Request) { +func fillPresignedURL(r *aws.Request) { if !r.ParamsFilled() { return } @@ -85,7 +67,7 @@ func fillPresignedURL(r *request.Request) { metadata.SigningRegion = resolved.SigningRegion // Presign a CopySnapshot request with modified params - req := request.New(cfgCp, metadata, r.Handlers, r.Retryer, r.Operation, newParams, r.Data) + req := aws.New(cfgCp, metadata, r.Handlers, r.Retryer, r.Operation, newParams, r.Data) url, err := req.Presign(5 * time.Minute) // 5 minutes should be enough. if err != nil { // bubble error back up to original request r.Error = err diff --git a/service/ec2/retryer_test.go b/service/ec2/retryer_test.go index 7d45e53f2f6..b729282150b 100644 --- a/service/ec2/retryer_test.go +++ b/service/ec2/retryer_test.go @@ -8,35 +8,97 @@ import ( "github.com/aws/aws-sdk-go-v2/internal/awstesting/unit" ) -func TestCustomRetryer(t *testing.T) { +func TestCustomRetryRules(t *testing.T) { svc := New(unit.Config()) - - if _, ok := svc.Client.Retryer.(retryer); !ok { - t.Error("expected custom retryer, but received otherwise") - } - req := svc.ModifyNetworkInterfaceAttributeRequest(&ModifyNetworkInterfaceAttributeInput{ NetworkInterfaceId: aws.String("foo"), }) - duration := svc.Client.Retryer.RetryRules(req.Request) + duration := req.Request.Retryer.RetryRules(req.Request) if duration < time.Second*1 || duration > time.Second*2 { t.Errorf("expected duration to be between 1s and 2s, but received %s", duration) } req.Request.RetryCount = 15 - duration = svc.Client.Retryer.RetryRules(req.Request) + duration = req.Request.Retryer.RetryRules(req.Request) if duration < time.Second*4 || duration > time.Second*8 { t.Errorf("expected duration to be between 4s and 8s, but received %s", duration) } - svc = New(aws.Config{ - Region: "us-west-2", - Retryer: aws.DefaultRetryer{}}, - ) +} + +func TestCustomRetryer_WhenRetrierSpecified(t *testing.T) { + svc := New(aws.Config{ + Region: "us-west-2", + Retryer: aws.DefaultRetryer{ + NumMaxRetries: 4, + MinThrottleDelay: 50 * time.Millisecond, + MinRetryDelay: 10 * time.Millisecond, + MaxThrottleDelay: 200 * time.Millisecond, + MaxRetryDelay: 300 * time.Millisecond, + }, + EndpointResolver: unit.Config().EndpointResolver, + }) + if _, ok := svc.Client.Retryer.(aws.DefaultRetryer); !ok { t.Error("expected default retryer, but received otherwise") } + + req := svc.AssignPrivateIpAddressesRequest(&AssignPrivateIpAddressesInput{ + NetworkInterfaceId: aws.String("foo"), + }) + + d := req.Request.Retryer.(aws.DefaultRetryer) + + if d.NumMaxRetries != 4 { + t.Errorf("expected max retries to be %v, got %v", 4, d.NumMaxRetries) + } + + if d.MinRetryDelay != 10*time.Millisecond { + t.Errorf("expected min retry delay to be %v, got %v", "10 ms", d.MinRetryDelay) + } + + if d.MinThrottleDelay != 50*time.Millisecond { + t.Errorf("expected min throttle delay to be %v, got %v", "50 ms", d.MinThrottleDelay) + } + + if d.MaxRetryDelay != 300*time.Millisecond { + t.Errorf("expected max retry delay to be %v, got %v", "300 ms", d.MaxRetryDelay) + } + + if d.MaxThrottleDelay != 200*time.Millisecond { + t.Errorf("expected max throttle delay to be %v, got %v", "200 ms", d.MaxThrottleDelay) + } +} + +func TestCustomRetryer(t *testing.T) { + svc := New(unit.Config()) + + req := svc.AssignPrivateIpAddressesRequest(&AssignPrivateIpAddressesInput{ + NetworkInterfaceId: aws.String("foo"), + }) + + d := req.Request.Retryer.(aws.DefaultRetryer) + + if d.NumMaxRetries != customRetryerMaxNumRetries { + t.Errorf("expected max retries to be %v, got %v", customRetryerMaxNumRetries, d.NumMaxRetries) + } + + if d.MinRetryDelay != customRetryerMinRetryDelay { + t.Errorf("expected min retry delay to be %v, got %v", customRetryerMinRetryDelay, d.MinRetryDelay) + } + + if d.MinThrottleDelay != customRetryerMinRetryDelay { + t.Errorf("expected min throttle delay to be %v, got %v", customRetryerMinRetryDelay, d.MinThrottleDelay) + } + + if d.MaxRetryDelay != customRetryerMaxRetryDelay { + t.Errorf("expected max retry delay to be %v, got %v", customRetryerMaxRetryDelay, d.MaxRetryDelay) + } + + if d.MaxThrottleDelay != customRetryerMaxRetryDelay { + t.Errorf("expected max throttle delay to be %v, got %v", customRetryerMaxRetryDelay, d.MaxThrottleDelay) + } } From bde7a066d0f20db1473aa67a008cb302611bfedb Mon Sep 17 00:00:00 2001 From: skotambkar Date: Fri, 6 Sep 2019 14:50:18 -0700 Subject: [PATCH 11/24] go lint fixes --- aws/default_retryer.go | 16 ++++++++++++---- service/ec2/customizations.go | 5 +++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/aws/default_retryer.go b/aws/default_retryer.go index 7410a85a1d1..71fcf00b089 100644 --- a/aws/default_retryer.go +++ b/aws/default_retryer.go @@ -21,7 +21,6 @@ import ( // // This implementation always has 100 max retries // func (d retryer) MaxRetries() int { return 100 } type DefaultRetryer struct { - // num of max retries allowed NumMaxRetries int MinRetryDelay time.Duration MinThrottleDelay time.Duration @@ -30,10 +29,19 @@ type DefaultRetryer struct { } const ( - DefaultRetryerMaxNumRetries = 3 - DefaultRetryerMinRetryDelay = 30 * time.Millisecond + // Default max number of retries + DefaultRetryerMaxNumRetries = 3 + + // Default min retry delay + DefaultRetryerMinRetryDelay = 30 * time.Millisecond + + // Default minimum delay when throttled DefaultRetryerMinThrottleDelay = 500 * time.Millisecond - DefaultRetryerMaxRetryDelay = 300 * time.Second + + // Default max retry delay + DefaultRetryerMaxRetryDelay = 300 * time.Second + + // Default maximum delay when throttled DefaultRetryerMaxThrottleDelay = 300 * time.Second ) diff --git a/service/ec2/customizations.go b/service/ec2/customizations.go index ec91b57ec1e..101bdd1c556 100644 --- a/service/ec2/customizations.go +++ b/service/ec2/customizations.go @@ -8,8 +8,13 @@ import ( ) const ( + // custom max number of retries customRetryerMaxNumRetries = 3 + + // custom min retry delay customRetryerMinRetryDelay = 1 * time.Second + + // custom max retry delay customRetryerMaxRetryDelay = 8 * time.Second ) From 18443880a2b333be9fa29d578ecb7b3ce8dd3ba0 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Fri, 6 Sep 2019 14:56:12 -0700 Subject: [PATCH 12/24] fixes go lint --- aws/default_retryer.go | 11 ++++++----- service/ec2/customizations.go | 7 ++++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/aws/default_retryer.go b/aws/default_retryer.go index 71fcf00b089..98266cc6352 100644 --- a/aws/default_retryer.go +++ b/aws/default_retryer.go @@ -29,19 +29,19 @@ type DefaultRetryer struct { } const ( - // Default max number of retries + // DefaultRetryerMaxNumRetries sets max number of retries DefaultRetryerMaxNumRetries = 3 - // Default min retry delay + // DefaultRetryerMinRetryDelay sets min retry delay DefaultRetryerMinRetryDelay = 30 * time.Millisecond - // Default minimum delay when throttled + // DefaultRetryerMinThrottleDelay sets minimum delay when throttled DefaultRetryerMinThrottleDelay = 500 * time.Millisecond - // Default max retry delay + // DefaultRetryerMaxRetryDelay sets max retry delay DefaultRetryerMaxRetryDelay = 300 * time.Second - // Default maximum delay when throttled + // DefaultRetryerMaxThrottleDelay sets maximum delay when throttled DefaultRetryerMaxThrottleDelay = 300 * time.Second ) @@ -53,6 +53,7 @@ func (d DefaultRetryer) MaxRetries() int { var seededRand = rand.New(&lockedSource{src: rand.NewSource(time.Now().UnixNano())}) +// setDefaults sets default values for the default Retryer func (d *DefaultRetryer) setDefaults() { if d.NumMaxRetries == 0 { d.NumMaxRetries = DefaultRetryerMaxNumRetries diff --git a/service/ec2/customizations.go b/service/ec2/customizations.go index 101bdd1c556..7a9d49c14ef 100644 --- a/service/ec2/customizations.go +++ b/service/ec2/customizations.go @@ -8,16 +8,17 @@ import ( ) const ( - // custom max number of retries + // customRetryerMaxNumRetries sets max number of retries customRetryerMaxNumRetries = 3 - // custom min retry delay + // customRetryerMinRetryDelay sets min retry delay customRetryerMinRetryDelay = 1 * time.Second - // custom max retry delay + // customRetryerMaxRetryDelay sets max retry delay customRetryerMaxRetryDelay = 8 * time.Second ) +// setCustomRetryer func returns aws.Retryer func setCustomRetryer(c *Client) aws.Retryer { return aws.DefaultRetryer{ NumMaxRetries: customRetryerMaxNumRetries, From 3c1f0c3859325b6bb7112fd6eed4aac59ea9a7af Mon Sep 17 00:00:00 2001 From: skotambkar Date: Fri, 6 Sep 2019 16:28:58 -0700 Subject: [PATCH 13/24] updated test for exhaustive retries --- aws/default_retryer.go | 1 + aws/request_test.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/aws/default_retryer.go b/aws/default_retryer.go index 98266cc6352..3ffa0d0ef21 100644 --- a/aws/default_retryer.go +++ b/aws/default_retryer.go @@ -48,6 +48,7 @@ const ( // MaxRetries returns the number of maximum returns the service will use to make // an individual API func (d DefaultRetryer) MaxRetries() int { + d.setDefaults() return d.NumMaxRetries } diff --git a/aws/request_test.go b/aws/request_test.go index 12de9fab3d4..feaf2ef8cd5 100644 --- a/aws/request_test.go +++ b/aws/request_test.go @@ -193,7 +193,7 @@ func TestRequestExhaustRetries(t *testing.T) { orig := sdk.SleepWithContext defer func() { sdk.SleepWithContext = orig }() - delays := []time.Duration{} + var delays []time.Duration sdk.SleepWithContext = func(ctx context.Context, dur time.Duration) error { delays = append(delays, dur) return nil @@ -236,7 +236,7 @@ func TestRequestExhaustRetries(t *testing.T) { t.Errorf("expect %d retry count, got %d", e, a) } - expectDelays := []struct{ min, max time.Duration }{{30, 59}, {60, 118}, {120, 236}} + expectDelays := []struct{ min, max time.Duration }{{30, 60}, {60, 120}, {120, 240}} for i, v := range delays { min := expectDelays[i].min * time.Millisecond max := expectDelays[i].max * time.Millisecond From 7b68b5862cac5d4eeee645887da5069ee94cd086 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Sun, 8 Sep 2019 11:11:44 -0700 Subject: [PATCH 14/24] fixed failing test case --- aws/default_retryer.go | 1 - service/s3/s3manager/download_test.go | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/aws/default_retryer.go b/aws/default_retryer.go index 3ffa0d0ef21..98266cc6352 100644 --- a/aws/default_retryer.go +++ b/aws/default_retryer.go @@ -48,7 +48,6 @@ const ( // MaxRetries returns the number of maximum returns the service will use to make // an individual API func (d DefaultRetryer) MaxRetries() int { - d.setDefaults() return d.NumMaxRetries } diff --git a/service/s3/s3manager/download_test.go b/service/s3/s3manager/download_test.go index 3777411d5c9..0bf493b40a5 100644 --- a/service/s3/s3manager/download_test.go +++ b/service/s3/s3manager/download_test.go @@ -157,12 +157,11 @@ func dlLoggingSvcContentRangeTotalAny(data []byte, states []int) (*s3.Client, *[ func dlLoggingSvcWithErrReader(cases []testErrReader) (*s3.Client, *[]string) { var m sync.Mutex - names := []string{} + var names []string var index int cfg := unit.Config() cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: len(cases) - 1} - svc := s3.New(cfg) svc.Handlers.Send.Clear() From bd76e4b106ddce25780744a18a96f38da8a63334 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Mon, 9 Sep 2019 12:36:35 -0700 Subject: [PATCH 15/24] adds constructor for default retryer --- aws/default_retryer.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/aws/default_retryer.go b/aws/default_retryer.go index 98266cc6352..b39c9d8b1b2 100644 --- a/aws/default_retryer.go +++ b/aws/default_retryer.go @@ -53,6 +53,17 @@ 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 { @@ -77,10 +88,6 @@ func (d *DefaultRetryer) setDefaults() { // Note: RetryRules method must be a value receiver so that the // defaultRetryer is safe. func (d DefaultRetryer) RetryRules(r *Request) time.Duration { - - // set default values for the retryer if not set. - d.setDefaults() - // Set the upper limit of delay in retrying at ~five minutes minDelay := d.MinRetryDelay var initialDelay time.Duration From 3d7c0e5089611956c7838afcb8b53fc7f2aa1cc3 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Mon, 9 Sep 2019 12:37:35 -0700 Subject: [PATCH 16/24] updates test cases to work with constructor for default retryer --- aws/default_retryer_test.go | 10 +++- aws/http_request_retry_test.go | 4 +- aws/request_1_6_test.go | 2 +- aws/request_pagination_test.go | 16 ++++-- aws/request_test.go | 50 ++++++++++++++----- aws/timeout_read_closer_benchmark_test.go | 4 +- internal/awstesting/client.go | 2 +- internal/awstesting/unit/unit.go | 1 + service/dynamodb/customizations_test.go | 12 +++-- service/ec2/customizations.go | 18 +++---- service/ec2/retryer_test.go | 28 +++++++---- service/kinesis/customizations_test.go | 4 +- service/s3/s3crypto/cipher_util_test.go | 12 +++-- service/s3/s3crypto/decryption_client_test.go | 12 +++-- service/s3/s3crypto/encryption_client_test.go | 8 ++- service/s3/s3crypto/kms_key_handler_test.go | 12 +++-- service/s3/s3manager/download.go | 4 +- service/s3/s3manager/download_test.go | 4 +- service/s3/statusok_error_test.go | 4 +- 19 files changed, 146 insertions(+), 61 deletions(-) diff --git a/aws/default_retryer_test.go b/aws/default_retryer_test.go index 75551cae1f1..5b2b9e82357 100644 --- a/aws/default_retryer_test.go +++ b/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) @@ -164,7 +166,11 @@ func TestGetRetryDelay(t *testing.T) { } func TestRetryDelay(t *testing.T) { - d := DefaultRetryer{NumMaxRetries: 100} + + d := NewDefaultRetryer(func(d *DefaultRetryer) { + d.NumMaxRetries = 100 + }) + r := Request{} for i := 0; i < 100; i++ { rTemp := r diff --git a/aws/http_request_retry_test.go b/aws/http_request_retry_test.go index 270d5f8ab09..e4c145a2729 100644 --- a/aws/http_request_retry_test.go +++ b/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) diff --git a/aws/request_1_6_test.go b/aws/request_1_6_test.go index d9f7921f980..8a5c9b337db 100644 --- a/aws/request_1_6_test.go +++ b/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, diff --git a/aws/request_pagination_test.go b/aws/request_pagination_test.go index 99b4b1b9b36..506f93776f2 100644 --- a/aws/request_pagination_test.go +++ b/aws/request_pagination_test.go @@ -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{ @@ -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", @@ -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{ @@ -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{ diff --git a/aws/request_test.go b/aws/request_test.go index feaf2ef8cd5..6709b1ef014 100644 --- a/aws/request_test.go +++ b/aws/request_test.go @@ -87,7 +87,9 @@ func TestRequestRecoverRetry5xx(t *testing.T) { } cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 10} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 10 + }) s := awstesting.NewClient(cfg) s.Handlers.Validate.Clear() @@ -126,7 +128,9 @@ func TestRequestRecoverRetry4xxRetryable(t *testing.T) { } cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 10} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 10 + }) s := awstesting.NewClient(cfg) s.Handlers.Validate.Clear() @@ -154,7 +158,9 @@ func TestRequestRecoverRetry4xxRetryable(t *testing.T) { // test that retries don't occur for 4xx status codes with a response type that can't be retried func TestRequest4xxUnretryable(t *testing.T) { cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 10} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 10 + }) s := awstesting.NewClient(cfg) @@ -207,7 +213,9 @@ func TestRequestExhaustRetries(t *testing.T) { {StatusCode: 500, Body: body(`{"__type":"UnknownError","message":"An error occurred."}`)}, } - s := awstesting.NewClient(unit.Config()) + cfg := unit.Config() + cfg.Retryer = aws.NewDefaultRetryer() + s := awstesting.NewClient(cfg) s.Handlers.Validate.Clear() s.Handlers.Unmarshal.PushBack(unmarshal) @@ -266,7 +274,9 @@ func TestRequest_RecoverExpiredCreds(t *testing.T) { } cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 10} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 10 + }) credsInvalidated := false credsProvider := func() aws.CredentialsProvider { @@ -394,7 +404,7 @@ func TestRequestThrottleRetries(t *testing.T) { orig := sdk.SleepWithContext defer func() { sdk.SleepWithContext = orig }() - delays := []time.Duration{} + var delays []time.Duration sdk.SleepWithContext = func(ctx context.Context, dur time.Duration) error { delays = append(delays, dur) return nil @@ -408,7 +418,9 @@ func TestRequestThrottleRetries(t *testing.T) { {StatusCode: 500, Body: body(`{"__type":"Throttling","message":"An error occurred."}`)}, } - s := awstesting.NewClient(unit.Config()) + cfg := unit.Config() + cfg.Retryer = aws.NewDefaultRetryer() + s := awstesting.NewClient(cfg) s.Handlers.Validate.Clear() s.Handlers.Unmarshal.PushBack(unmarshal) @@ -460,7 +472,9 @@ func TestRequestRecoverTimeoutWithNilBody(t *testing.T) { } cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 10} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 10 + }) s := awstesting.NewClient(cfg) @@ -507,7 +521,9 @@ func TestRequestRecoverTimeoutWithNilResponse(t *testing.T) { } cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 10} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 10 + }) s := awstesting.NewClient(cfg) @@ -572,7 +588,9 @@ func TestRequest_NoBody(t *testing.T) { cfg := unit.Config() cfg.Region = "mock-region" - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 0} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 0 + }) cfg.EndpointResolver = aws.ResolveWithEndpointURL(server.URL) s := awstesting.NewClient(cfg) @@ -744,13 +762,17 @@ func TestSerializationErrConnectionReset(t *testing.T) { TargetPrefix: "Foo", } cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 5} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 5 + }) req := aws.New( cfg, meta, handlers, - aws.DefaultRetryer{NumMaxRetries: 5}, + aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 5 + }), op, &struct{}{}, &struct{}{}, @@ -895,7 +917,9 @@ func TestRequest_TemporaryRetry(t *testing.T) { defer server.Close() cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 1} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 1 + }) cfg.HTTPClient = &http.Client{ Timeout: 100 * time.Millisecond, } diff --git a/aws/timeout_read_closer_benchmark_test.go b/aws/timeout_read_closer_benchmark_test.go index f25097dd0af..b34be170296 100644 --- a/aws/timeout_read_closer_benchmark_test.go +++ b/aws/timeout_read_closer_benchmark_test.go @@ -56,7 +56,9 @@ func BenchmarkTimeoutReadCloser(b *testing.B) { cfg, meta, handlers, - aws.DefaultRetryer{NumMaxRetries: 5}, + aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 5 + }), op, &struct { Foo *string diff --git a/internal/awstesting/client.go b/internal/awstesting/client.go index 20ef4055deb..64a3ed10ee3 100644 --- a/internal/awstesting/client.go +++ b/internal/awstesting/client.go @@ -7,7 +7,7 @@ import ( // NewClient creates and initializes a generic service client for testing. func NewClient(cfg aws.Config) *aws.Client { if cfg.Retryer == nil { - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 3} + cfg.Retryer = aws.NewDefaultRetryer() } return aws.NewClient(cfg, aws.Metadata{ServiceName: "mockService"}) } diff --git a/internal/awstesting/unit/unit.go b/internal/awstesting/unit/unit.go index a4f98b68655..4f65e8caff0 100644 --- a/internal/awstesting/unit/unit.go +++ b/internal/awstesting/unit/unit.go @@ -16,6 +16,7 @@ func init() { Source: "unit test credentials", }, } + config.Retryer = aws.NewDefaultRetryer() } var config aws.Config diff --git a/service/dynamodb/customizations_test.go b/service/dynamodb/customizations_test.go index ae23100c503..e09754f77ab 100644 --- a/service/dynamodb/customizations_test.go +++ b/service/dynamodb/customizations_test.go @@ -19,7 +19,9 @@ var db *dynamodb.Client func TestMain(m *testing.M) { cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 2} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 2 + }) db = dynamodb.New(cfg) db.Handlers.Send.Clear() // mock sending @@ -57,7 +59,9 @@ func TestDefaultRetryRules(t *testing.T) { func TestCustomRetryRules(t *testing.T) { cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 2} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 2 + }) svc := dynamodb.New(cfg) if e, a := 2, svc.Retryer.MaxRetries(); e != a { @@ -138,7 +142,9 @@ func TestValidateCRC32DoesNotMatch(t *testing.T) { func TestValidateCRC32DoesNotMatchNoComputeChecksum(t *testing.T) { cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 2} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 2 + }) svc := dynamodb.New(cfg) svc.DisableComputeChecksums = true diff --git a/service/ec2/customizations.go b/service/ec2/customizations.go index 7a9d49c14ef..31b0fd8c666 100644 --- a/service/ec2/customizations.go +++ b/service/ec2/customizations.go @@ -18,15 +18,13 @@ const ( customRetryerMaxRetryDelay = 8 * time.Second ) -// setCustomRetryer func returns aws.Retryer -func setCustomRetryer(c *Client) aws.Retryer { - return aws.DefaultRetryer{ - NumMaxRetries: customRetryerMaxNumRetries, - MinRetryDelay: customRetryerMinRetryDelay, - MinThrottleDelay: customRetryerMinRetryDelay, - MaxRetryDelay: customRetryerMaxRetryDelay, - MaxThrottleDelay: customRetryerMaxRetryDelay, - } +// setCustomRetryer overides the default Retryer values +func setCustomRetryer(d *aws.DefaultRetryer) { + d.NumMaxRetries = customRetryerMaxNumRetries + d.MinRetryDelay = customRetryerMinRetryDelay + d.MinThrottleDelay = customRetryerMinRetryDelay + d.MaxRetryDelay = customRetryerMaxRetryDelay + d.MaxThrottleDelay = customRetryerMaxRetryDelay } func init() { @@ -35,7 +33,7 @@ func init() { r.Handlers.Build.PushFront(fillPresignedURL) } if c.Config.Retryer == nil && (r.Operation.Name == opModifyNetworkInterfaceAttribute || r.Operation.Name == opAssignPrivateIpAddresses) { - r.Retryer = setCustomRetryer(c) + r.Retryer = aws.Retryer(aws.NewDefaultRetryer(setCustomRetryer)) } } } diff --git a/service/ec2/retryer_test.go b/service/ec2/retryer_test.go index b729282150b..b8f125247e0 100644 --- a/service/ec2/retryer_test.go +++ b/service/ec2/retryer_test.go @@ -10,7 +10,12 @@ import ( func TestCustomRetryRules(t *testing.T) { - svc := New(unit.Config()) + cfg := unit.Config() + // set retryer to nil as unit.config sets default retryer + // retryer is set to nil as we want to use the custom retryer + cfg.Retryer = nil + svc := New(cfg) + req := svc.ModifyNetworkInterfaceAttributeRequest(&ModifyNetworkInterfaceAttributeInput{ NetworkInterfaceId: aws.String("foo"), }) @@ -32,13 +37,13 @@ func TestCustomRetryRules(t *testing.T) { func TestCustomRetryer_WhenRetrierSpecified(t *testing.T) { svc := New(aws.Config{ Region: "us-west-2", - Retryer: aws.DefaultRetryer{ - NumMaxRetries: 4, - MinThrottleDelay: 50 * time.Millisecond, - MinRetryDelay: 10 * time.Millisecond, - MaxThrottleDelay: 200 * time.Millisecond, - MaxRetryDelay: 300 * time.Millisecond, - }, + Retryer: aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 4 + d.MinThrottleDelay = 50 * time.Millisecond + d.MinRetryDelay = 10 * time.Millisecond + d.MaxThrottleDelay = 200 * time.Millisecond + d.MaxRetryDelay = 300 * time.Millisecond + }), EndpointResolver: unit.Config().EndpointResolver, }) @@ -74,7 +79,12 @@ func TestCustomRetryer_WhenRetrierSpecified(t *testing.T) { } func TestCustomRetryer(t *testing.T) { - svc := New(unit.Config()) + + cfg := unit.Config() + // set retryer to nil as unit.config sets default retryer + // retryer is set to nil as we want to use the custom retryer + cfg.Retryer = nil + svc := New(cfg) req := svc.AssignPrivateIpAddressesRequest(&AssignPrivateIpAddressesInput{ NetworkInterfaceId: aws.String("foo"), diff --git a/service/kinesis/customizations_test.go b/service/kinesis/customizations_test.go index 6c1e0b05e80..ae18b6f9fb2 100644 --- a/service/kinesis/customizations_test.go +++ b/service/kinesis/customizations_test.go @@ -33,7 +33,9 @@ func TestKinesisGetRecordsCustomization(t *testing.T) { retryCount := 0 cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 4} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 4 + }) svc := New(cfg) req := svc.GetRecordsRequest(&GetRecordsInput{ diff --git a/service/s3/s3crypto/cipher_util_test.go b/service/s3/s3crypto/cipher_util_test.go index c896ec8dbd7..245e3d2e7ed 100644 --- a/service/s3/s3crypto/cipher_util_test.go +++ b/service/s3/s3crypto/cipher_util_test.go @@ -100,7 +100,9 @@ func TestCEKFactory(t *testing.T) { defer ts.Close() cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 0} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 0 + }) cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" @@ -156,7 +158,9 @@ func TestCEKFactoryNoCEK(t *testing.T) { defer ts.Close() cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 0} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 0 + }) cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" @@ -212,7 +216,9 @@ func TestCEKFactoryCustomEntry(t *testing.T) { defer ts.Close() cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 0} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 0 + }) cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" diff --git a/service/s3/s3crypto/decryption_client_test.go b/service/s3/s3crypto/decryption_client_test.go index 3303a21f5e4..a3e9c35d53f 100644 --- a/service/s3/s3crypto/decryption_client_test.go +++ b/service/s3/s3crypto/decryption_client_test.go @@ -30,7 +30,9 @@ func TestGetObjectGCM(t *testing.T) { defer ts.Close() cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 0} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 0 + }) cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" @@ -101,7 +103,9 @@ func TestGetObjectCBC(t *testing.T) { defer ts.Close() cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 0} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 0 + }) cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" @@ -170,7 +174,9 @@ func TestGetObjectCBC2(t *testing.T) { defer ts.Close() cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 0} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 0 + }) cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" diff --git a/service/s3/s3crypto/encryption_client_test.go b/service/s3/s3crypto/encryption_client_test.go index 46110b11307..4d39dc1c0b0 100644 --- a/service/s3/s3crypto/encryption_client_test.go +++ b/service/s3/s3crypto/encryption_client_test.go @@ -21,7 +21,9 @@ import ( func TestDefaultConfigValues(t *testing.T) { cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 0} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 0 + }) cfg.Region = "us-west-2" svc := kms.New(cfg) @@ -49,7 +51,9 @@ func TestPutObject(t *testing.T) { cb := mockCipherBuilder{generator} cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 0} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 0 + }) cfg.Region = "us-west-2" c := s3crypto.NewEncryptionClient(cfg, cb) diff --git a/service/s3/s3crypto/kms_key_handler_test.go b/service/s3/s3crypto/kms_key_handler_test.go index 85136741a9f..a0c7baa6532 100644 --- a/service/s3/s3crypto/kms_key_handler_test.go +++ b/service/s3/s3crypto/kms_key_handler_test.go @@ -49,7 +49,9 @@ func TestKMSGenerateCipherData(t *testing.T) { })) cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 0} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 0 + }) cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" @@ -79,7 +81,9 @@ func TestKMSDecrypt(t *testing.T) { })) cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 0} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 0 + }) cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" @@ -106,7 +110,9 @@ func TestKMSDecryptBadJSON(t *testing.T) { })) cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 0} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 0 + }) cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" diff --git a/service/s3/s3manager/download.go b/service/s3/s3manager/download.go index d263f74450b..a0d737ed470 100644 --- a/service/s3/s3manager/download.go +++ b/service/s3/s3manager/download.go @@ -87,7 +87,7 @@ func NewDownloader(cfg aws.Config, options ...func(*Downloader)) *Downloader { Retryer: cfg.Retryer, } if d.Retryer == nil { - d.Retryer = aws.DefaultRetryer{NumMaxRetries: 3} + d.Retryer = aws.NewDefaultRetryer() } for _, option := range options { @@ -122,7 +122,7 @@ func NewDownloaderWithClient(svc s3iface.ClientAPI, options ...func(*Downloader) if s3Svc, ok := svc.(*s3.Client); ok { retryer = s3Svc.Retryer } else { - retryer = aws.DefaultRetryer{NumMaxRetries: 3} + retryer = aws.NewDefaultRetryer() } d := &Downloader{ diff --git a/service/s3/s3manager/download_test.go b/service/s3/s3manager/download_test.go index 0bf493b40a5..3f194329f43 100644 --- a/service/s3/s3manager/download_test.go +++ b/service/s3/s3manager/download_test.go @@ -161,7 +161,9 @@ func dlLoggingSvcWithErrReader(cases []testErrReader) (*s3.Client, *[]string) { var index int cfg := unit.Config() - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: len(cases) - 1} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = len(cases) - 1 + }) svc := s3.New(cfg) svc.Handlers.Send.Clear() diff --git a/service/s3/statusok_error_test.go b/service/s3/statusok_error_test.go index 8bf6521490f..5661a41e135 100644 --- a/service/s3/statusok_error_test.go +++ b/service/s3/statusok_error_test.go @@ -167,7 +167,9 @@ func newCopyTestSvc(errMsg string) *s3.Client { })) cfg := unit.Config() cfg.EndpointResolver = aws.ResolveWithEndpointURL(server.URL) - cfg.Retryer = aws.DefaultRetryer{NumMaxRetries: 0} + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 0 + }) svc := s3.New(cfg) svc.ForcePathStyle = true From 5b03a3ce41583bdcdae33d52969abab75c83c33f Mon Sep 17 00:00:00 2001 From: skotambkar Date: Tue, 10 Sep 2019 16:32:55 -0700 Subject: [PATCH 17/24] adds no op retryer for zero retries --- aws/no_op_retryer.go | 24 +++++++++++++++++++++ aws/no_op_retryer_test.go | 44 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 aws/no_op_retryer.go create mode 100644 aws/no_op_retryer_test.go diff --git a/aws/no_op_retryer.go b/aws/no_op_retryer.go new file mode 100644 index 00000000000..d00474ee996 --- /dev/null +++ b/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 +} + +// NoOpRetryer should never retry; so ShouldRetry will always return false. +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 +} diff --git a/aws/no_op_retryer_test.go b/aws/no_op_retryer_test.go new file mode 100644 index 00000000000..dec439393e9 --- /dev/null +++ b/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) + } + } +} From 23d434ea40f082be4ef4c2d1ab76b5626e1b6c3c Mon Sep 17 00:00:00 2001 From: skotambkar Date: Tue, 10 Sep 2019 16:43:27 -0700 Subject: [PATCH 18/24] updates to correctly set retryer when service client is initialized --- aws/client.go | 3 +-- internal/awstesting/client.go | 3 --- internal/awstesting/unit/unit.go | 1 - 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/aws/client.go b/aws/client.go index 704bbe10409..ffbfc3df357 100644 --- a/aws/client.go +++ b/aws/client.go @@ -63,8 +63,7 @@ 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() diff --git a/internal/awstesting/client.go b/internal/awstesting/client.go index 64a3ed10ee3..1cd260bd1dc 100644 --- a/internal/awstesting/client.go +++ b/internal/awstesting/client.go @@ -6,8 +6,5 @@ import ( // NewClient creates and initializes a generic service client for testing. func NewClient(cfg aws.Config) *aws.Client { - if cfg.Retryer == nil { - cfg.Retryer = aws.NewDefaultRetryer() - } return aws.NewClient(cfg, aws.Metadata{ServiceName: "mockService"}) } diff --git a/internal/awstesting/unit/unit.go b/internal/awstesting/unit/unit.go index 4f65e8caff0..a4f98b68655 100644 --- a/internal/awstesting/unit/unit.go +++ b/internal/awstesting/unit/unit.go @@ -16,7 +16,6 @@ func init() { Source: "unit test credentials", }, } - config.Retryer = aws.NewDefaultRetryer() } var config aws.Config From cef80f1a6b7df8f559b5edf0ecacd5f9aadf3834 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Tue, 10 Sep 2019 18:04:59 -0700 Subject: [PATCH 19/24] adds suggested changes and handles fatal errors in s3 crypto --- aws/default_retryer.go | 50 +++++++------------ aws/default_retryer_test.go | 2 +- aws/request_pagination_test.go | 4 +- aws/request_test.go | 12 ++--- service/dynamodb/customizations.go | 16 +++--- service/dynamodb/customizations_test.go | 12 ++--- service/ec2/customizations.go | 6 +-- service/ec2/retryer_test.go | 6 --- service/s3/s3crypto/cipher_util_test.go | 12 ++--- service/s3/s3crypto/decryption_client_test.go | 30 +++++------ service/s3/s3crypto/encryption_client_test.go | 14 ++---- service/s3/s3crypto/kms_key_handler_test.go | 12 ++--- service/s3/s3manager/download.go | 5 -- service/s3/s3manager/download_test.go | 12 +++-- service/s3/statusok_error_test.go | 5 +- 15 files changed, 73 insertions(+), 125 deletions(-) diff --git a/aws/default_retryer.go b/aws/default_retryer.go index b39c9d8b1b2..96c1313792e 100644 --- a/aws/default_retryer.go +++ b/aws/default_retryer.go @@ -18,8 +18,6 @@ import ( // client.DefaultRetryer // } // -// // This implementation always has 100 max retries -// func (d retryer) MaxRetries() int { return 100 } type DefaultRetryer struct { NumMaxRetries int MinRetryDelay time.Duration @@ -29,16 +27,16 @@ type DefaultRetryer struct { } const ( - // DefaultRetryerMaxNumRetries sets max number of retries + // DefaultRetryerMaxNumRetries sets maximum number of retries DefaultRetryerMaxNumRetries = 3 - // DefaultRetryerMinRetryDelay sets min retry delay + // DefaultRetryerMinRetryDelay sets minimum retry delay DefaultRetryerMinRetryDelay = 30 * time.Millisecond // DefaultRetryerMinThrottleDelay sets minimum delay when throttled DefaultRetryerMinThrottleDelay = 500 * time.Millisecond - // DefaultRetryerMaxRetryDelay sets max retry delay + // DefaultRetryerMaxRetryDelay sets maximum retry delay DefaultRetryerMaxRetryDelay = 300 * time.Second // DefaultRetryerMaxThrottleDelay sets maximum delay when throttled @@ -53,38 +51,25 @@ 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 +// 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{} - d.setDefaults() + d := DefaultRetryer{ + NumMaxRetries: DefaultRetryerMaxNumRetries, + MinRetryDelay: DefaultRetryerMinRetryDelay, + MinThrottleDelay: DefaultRetryerMinThrottleDelay, + MaxRetryDelay: DefaultRetryerMaxRetryDelay, + MaxThrottleDelay: DefaultRetryerMaxThrottleDelay, + } + 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 { @@ -110,17 +95,18 @@ func (d DefaultRetryer) RetryRules(r *Request) 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< maxDelay { - delay = getDelaySeed(maxDelay / 2) + delay = getJitterDelay(maxDelay / 2) } } else { - delay = getDelaySeed(maxDelay / 2) + delay = getJitterDelay(maxDelay / 2) } return delay + initialDelay } -func getDelaySeed(duration time.Duration) time.Duration { +// getJitterDelay returns a jittered delay for retry +func getJitterDelay(duration time.Duration) time.Duration { return time.Duration(seededRand.Int63n(int64(duration)) + int64(duration)) } diff --git a/aws/default_retryer_test.go b/aws/default_retryer_test.go index 5b2b9e82357..c51f061460c 100644 --- a/aws/default_retryer_test.go +++ b/aws/default_retryer_test.go @@ -73,7 +73,7 @@ func TestRetryThrottleStatusCodes(t *testing.T) { } } -func TestCanUseRetryAfter(t *testing.T) { +func TestGetRetryAfterDelay(t *testing.T) { cases := []struct { r Request e bool diff --git a/aws/request_pagination_test.go b/aws/request_pagination_test.go index 506f93776f2..e32bdd2f24b 100644 --- a/aws/request_pagination_test.go +++ b/aws/request_pagination_test.go @@ -358,7 +358,7 @@ func TestPaginationWithContextCancel(t *testing.T) { for _, c := range cases { input := c.input - inValues := []string{} + var inValues []string p := aws.Pager{ NewRequest: func(ctx context.Context) (*aws.Request, error) { h := defaults.Handlers() @@ -388,7 +388,7 @@ func TestPaginationWithContextCancel(t *testing.T) { ctx, cancelFn := context.WithCancel(context.Background()) cancelFn() - results := []*string{} + var results []*string for p.Next(ctx) { page := p.CurrentPage() output := page.(*mockOutput) diff --git a/aws/request_test.go b/aws/request_test.go index 6709b1ef014..a3350dd1626 100644 --- a/aws/request_test.go +++ b/aws/request_test.go @@ -213,9 +213,7 @@ func TestRequestExhaustRetries(t *testing.T) { {StatusCode: 500, Body: body(`{"__type":"UnknownError","message":"An error occurred."}`)}, } - cfg := unit.Config() - cfg.Retryer = aws.NewDefaultRetryer() - s := awstesting.NewClient(cfg) + s := awstesting.NewClient(unit.Config()) s.Handlers.Validate.Clear() s.Handlers.Unmarshal.PushBack(unmarshal) @@ -418,9 +416,7 @@ func TestRequestThrottleRetries(t *testing.T) { {StatusCode: 500, Body: body(`{"__type":"Throttling","message":"An error occurred."}`)}, } - cfg := unit.Config() - cfg.Retryer = aws.NewDefaultRetryer() - s := awstesting.NewClient(cfg) + s := awstesting.NewClient(unit.Config()) s.Handlers.Validate.Clear() s.Handlers.Unmarshal.PushBack(unmarshal) @@ -588,9 +584,7 @@ func TestRequest_NoBody(t *testing.T) { cfg := unit.Config() cfg.Region = "mock-region" - cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { - d.NumMaxRetries = 0 - }) + cfg.Retryer = aws.NoOpRetryer{} cfg.EndpointResolver = aws.ResolveWithEndpointURL(server.URL) s := awstesting.NewClient(cfg) diff --git a/service/dynamodb/customizations.go b/service/dynamodb/customizations.go index a2dcf2cf06f..f55aa48247a 100644 --- a/service/dynamodb/customizations.go +++ b/service/dynamodb/customizations.go @@ -10,16 +10,14 @@ import ( "time" "github.com/aws/aws-sdk-go-v2/aws" - client "github.com/aws/aws-sdk-go-v2/aws" - request "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/aws/awserr" ) type retryer struct { - client.DefaultRetryer + aws.DefaultRetryer } -func (d retryer) RetryRules(r *request.Request) time.Duration { +func (d retryer) RetryRules(r *aws.Request) time.Duration { delay := time.Duration(math.Pow(2, float64(r.RetryCount))) * 50 return delay * time.Millisecond } @@ -46,9 +44,9 @@ func init() { func setCustomRetryer(c *Client) { c.Retryer = retryer{ - DefaultRetryer: client.DefaultRetryer{ - NumMaxRetries: 10, - }, + DefaultRetryer: aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 10 + }), } } @@ -69,13 +67,13 @@ func drainBody(b io.ReadCloser, length int64) (out *bytes.Buffer, err error) { var disableCompressionHandler = aws.NamedHandler{Name: "dynamodb.DisableCompression", Fn: disableCompression} -func disableCompression(r *request.Request) { +func disableCompression(r *aws.Request) { r.HTTPRequest.Header.Set("Accept-Encoding", "identity") } var validateCRC32Handler = aws.NamedHandler{Name: "dynamodb.ValidateCRC32", Fn: validateCRC32} -func validateCRC32(r *request.Request) { +func validateCRC32(r *aws.Request) { if r.Error != nil { return // already have an error, no need to verify CRC } diff --git a/service/dynamodb/customizations_test.go b/service/dynamodb/customizations_test.go index e09754f77ab..7025f9dd7cc 100644 --- a/service/dynamodb/customizations_test.go +++ b/service/dynamodb/customizations_test.go @@ -8,8 +8,6 @@ import ( "testing" "github.com/aws/aws-sdk-go-v2/aws" - client "github.com/aws/aws-sdk-go-v2/aws" - request "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/aws/awserr" "github.com/aws/aws-sdk-go-v2/internal/awstesting/unit" "github.com/aws/aws-sdk-go-v2/service/dynamodb" @@ -29,13 +27,13 @@ func TestMain(m *testing.M) { os.Exit(m.Run()) } -func mockCRCResponse(svc *dynamodb.Client, status int, body, crc string) (req *request.Request) { +func mockCRCResponse(svc *dynamodb.Client, status int, body, crc string) (req *aws.Request) { header := http.Header{} header.Set("x-amz-crc32", crc) listReq := svc.ListTablesRequest(nil) req = listReq.Request - req.Handlers.Send.PushBack(func(*request.Request) { + req.Handlers.Send.PushBack(func(*aws.Request) { req.HTTPResponse = &http.Response{ ContentLength: int64(len(body)), StatusCode: status, @@ -70,12 +68,14 @@ func TestCustomRetryRules(t *testing.T) { } type testCustomRetryer struct { - client.DefaultRetryer + aws.DefaultRetryer } func TestCustomRetry_FromConfig(t *testing.T) { cfg := unit.Config() - cfg.Retryer = testCustomRetryer{client.DefaultRetryer{NumMaxRetries: 9}} + cfg.Retryer = testCustomRetryer{aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 9. + })} svc := dynamodb.New(cfg) diff --git a/service/ec2/customizations.go b/service/ec2/customizations.go index 31b0fd8c666..6830c59a9fe 100644 --- a/service/ec2/customizations.go +++ b/service/ec2/customizations.go @@ -18,8 +18,8 @@ const ( customRetryerMaxRetryDelay = 8 * time.Second ) -// setCustomRetryer overides the default Retryer values -func setCustomRetryer(d *aws.DefaultRetryer) { +// setRetryerConfig overrides the default Retryer values +func setRetryerConfig(d *aws.DefaultRetryer) { d.NumMaxRetries = customRetryerMaxNumRetries d.MinRetryDelay = customRetryerMinRetryDelay d.MinThrottleDelay = customRetryerMinRetryDelay @@ -33,7 +33,7 @@ func init() { r.Handlers.Build.PushFront(fillPresignedURL) } if c.Config.Retryer == nil && (r.Operation.Name == opModifyNetworkInterfaceAttribute || r.Operation.Name == opAssignPrivateIpAddresses) { - r.Retryer = aws.Retryer(aws.NewDefaultRetryer(setCustomRetryer)) + r.Retryer = aws.NewDefaultRetryer(setRetryerConfig) } } } diff --git a/service/ec2/retryer_test.go b/service/ec2/retryer_test.go index b8f125247e0..a7a8389c106 100644 --- a/service/ec2/retryer_test.go +++ b/service/ec2/retryer_test.go @@ -11,9 +11,6 @@ import ( func TestCustomRetryRules(t *testing.T) { cfg := unit.Config() - // set retryer to nil as unit.config sets default retryer - // retryer is set to nil as we want to use the custom retryer - cfg.Retryer = nil svc := New(cfg) req := svc.ModifyNetworkInterfaceAttributeRequest(&ModifyNetworkInterfaceAttributeInput{ @@ -81,9 +78,6 @@ func TestCustomRetryer_WhenRetrierSpecified(t *testing.T) { func TestCustomRetryer(t *testing.T) { cfg := unit.Config() - // set retryer to nil as unit.config sets default retryer - // retryer is set to nil as we want to use the custom retryer - cfg.Retryer = nil svc := New(cfg) req := svc.AssignPrivateIpAddressesRequest(&AssignPrivateIpAddressesInput{ diff --git a/service/s3/s3crypto/cipher_util_test.go b/service/s3/s3crypto/cipher_util_test.go index 245e3d2e7ed..6a7f73a775c 100644 --- a/service/s3/s3crypto/cipher_util_test.go +++ b/service/s3/s3crypto/cipher_util_test.go @@ -100,9 +100,7 @@ func TestCEKFactory(t *testing.T) { defer ts.Close() cfg := unit.Config() - cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { - d.NumMaxRetries = 0 - }) + cfg.Retryer = aws.NoOpRetryer{} cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" @@ -158,9 +156,7 @@ func TestCEKFactoryNoCEK(t *testing.T) { defer ts.Close() cfg := unit.Config() - cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { - d.NumMaxRetries = 0 - }) + cfg.Retryer = aws.NoOpRetryer{} cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" @@ -216,9 +212,7 @@ func TestCEKFactoryCustomEntry(t *testing.T) { defer ts.Close() cfg := unit.Config() - cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { - d.NumMaxRetries = 0 - }) + cfg.Retryer = aws.NoOpRetryer{} cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" diff --git a/service/s3/s3crypto/decryption_client_test.go b/service/s3/s3crypto/decryption_client_test.go index a3e9c35d53f..2b1f5fa0411 100644 --- a/service/s3/s3crypto/decryption_client_test.go +++ b/service/s3/s3crypto/decryption_client_test.go @@ -30,18 +30,16 @@ func TestGetObjectGCM(t *testing.T) { defer ts.Close() cfg := unit.Config() - cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { - d.NumMaxRetries = 0 - }) + cfg.Retryer = aws.NoOpRetryer{} cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" c := s3crypto.NewDecryptionClient(cfg) - c.S3Client.(*s3.Client).ForcePathStyle = true - if c == nil { - t.Error("expected non-nil value") + t.Fatalf("failed to create a new S3 crypto decryption client") } + + c.S3Client.(*s3.Client).ForcePathStyle = true input := &s3.GetObjectInput{ Key: aws.String("test"), Bucket: aws.String("test"), @@ -103,18 +101,16 @@ func TestGetObjectCBC(t *testing.T) { defer ts.Close() cfg := unit.Config() - cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { - d.NumMaxRetries = 0 - }) + cfg.Retryer = aws.NoOpRetryer{} cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" c := s3crypto.NewDecryptionClient(cfg) - c.S3Client.(*s3.Client).ForcePathStyle = true - if c == nil { - t.Error("expected non-nil value") + t.Fatalf("failed to create a new S3 crypto decryption client") } + + c.S3Client.(*s3.Client).ForcePathStyle = true input := &s3.GetObjectInput{ Key: aws.String("test"), Bucket: aws.String("test"), @@ -174,18 +170,16 @@ func TestGetObjectCBC2(t *testing.T) { defer ts.Close() cfg := unit.Config() - cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { - d.NumMaxRetries = 0 - }) + cfg.Retryer = aws.NoOpRetryer{} cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" c := s3crypto.NewDecryptionClient(cfg) - c.S3Client.(*s3.Client).ForcePathStyle = true - if c == nil { - t.Error("expected non-nil value") + t.Fatalf("failed to create a new S3 crypto decryption client") } + + c.S3Client.(*s3.Client).ForcePathStyle = true input := &s3.GetObjectInput{ Key: aws.String("test"), Bucket: aws.String("test"), diff --git a/service/s3/s3crypto/encryption_client_test.go b/service/s3/s3crypto/encryption_client_test.go index 4d39dc1c0b0..c12d6154060 100644 --- a/service/s3/s3crypto/encryption_client_test.go +++ b/service/s3/s3crypto/encryption_client_test.go @@ -21,9 +21,7 @@ import ( func TestDefaultConfigValues(t *testing.T) { cfg := unit.Config() - cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { - d.NumMaxRetries = 0 - }) + cfg.Retryer = aws.NoOpRetryer{} cfg.Region = "us-west-2" svc := kms.New(cfg) @@ -51,17 +49,15 @@ func TestPutObject(t *testing.T) { cb := mockCipherBuilder{generator} cfg := unit.Config() - cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { - d.NumMaxRetries = 0 - }) + cfg.Retryer = aws.NoOpRetryer{} cfg.Region = "us-west-2" c := s3crypto.NewEncryptionClient(cfg, cb) - c.S3Client.(*s3.Client).ForcePathStyle = true - if c == nil { - t.Error("expected non-vil client value") + t.Fatalf("failed to create a new S3 crypto encryption client") } + + c.S3Client.(*s3.Client).ForcePathStyle = true input := &s3.PutObjectInput{ Key: aws.String("test"), Bucket: aws.String("test"), diff --git a/service/s3/s3crypto/kms_key_handler_test.go b/service/s3/s3crypto/kms_key_handler_test.go index a0c7baa6532..9cf33a407c7 100644 --- a/service/s3/s3crypto/kms_key_handler_test.go +++ b/service/s3/s3crypto/kms_key_handler_test.go @@ -49,9 +49,7 @@ func TestKMSGenerateCipherData(t *testing.T) { })) cfg := unit.Config() - cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { - d.NumMaxRetries = 0 - }) + cfg.Retryer = aws.NoOpRetryer{} cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" @@ -81,9 +79,7 @@ func TestKMSDecrypt(t *testing.T) { })) cfg := unit.Config() - cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { - d.NumMaxRetries = 0 - }) + cfg.Retryer = aws.NoOpRetryer{} cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" @@ -110,9 +106,7 @@ func TestKMSDecryptBadJSON(t *testing.T) { })) cfg := unit.Config() - cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { - d.NumMaxRetries = 0 - }) + cfg.Retryer = aws.NoOpRetryer{} cfg.EndpointResolver = aws.ResolveWithEndpointURL(ts.URL) cfg.Region = "us-west-2" diff --git a/service/s3/s3manager/download.go b/service/s3/s3manager/download.go index a0d737ed470..3c743f1baec 100644 --- a/service/s3/s3manager/download.go +++ b/service/s3/s3manager/download.go @@ -86,9 +86,6 @@ func NewDownloader(cfg aws.Config, options ...func(*Downloader)) *Downloader { Concurrency: DefaultDownloadConcurrency, Retryer: cfg.Retryer, } - if d.Retryer == nil { - d.Retryer = aws.NewDefaultRetryer() - } for _, option := range options { option(d) @@ -121,8 +118,6 @@ func NewDownloaderWithClient(svc s3iface.ClientAPI, options ...func(*Downloader) if s3Svc, ok := svc.(*s3.Client); ok { retryer = s3Svc.Retryer - } else { - retryer = aws.NewDefaultRetryer() } d := &Downloader{ diff --git a/service/s3/s3manager/download_test.go b/service/s3/s3manager/download_test.go index 3f194329f43..fe1bb422d23 100644 --- a/service/s3/s3manager/download_test.go +++ b/service/s3/s3manager/download_test.go @@ -161,9 +161,15 @@ func dlLoggingSvcWithErrReader(cases []testErrReader) (*s3.Client, *[]string) { var index int cfg := unit.Config() - cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { - d.NumMaxRetries = len(cases) - 1 - }) + switch len(cases) - 1 { + case 0: // zero retries expected + cfg.Retryer = aws.NoOpRetryer{} + default: + cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = len(cases) - 1 + }) + } + svc := s3.New(cfg) svc.Handlers.Send.Clear() diff --git a/service/s3/statusok_error_test.go b/service/s3/statusok_error_test.go index 5661a41e135..34916837a1c 100644 --- a/service/s3/statusok_error_test.go +++ b/service/s3/statusok_error_test.go @@ -167,10 +167,7 @@ func newCopyTestSvc(errMsg string) *s3.Client { })) cfg := unit.Config() cfg.EndpointResolver = aws.ResolveWithEndpointURL(server.URL) - cfg.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { - d.NumMaxRetries = 0 - }) - + cfg.Retryer = aws.NoOpRetryer{} svc := s3.New(cfg) svc.ForcePathStyle = true From fda8bcb6df4cf7ab727ca0b89c82f3f7a1fee450 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Tue, 10 Sep 2019 18:42:40 -0700 Subject: [PATCH 20/24] updates change log pending --- CHANGELOG_PENDING.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index ac2b7150e44..30ba01d9c17 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -1,8 +1,11 @@ ### 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) +* `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 From d813158d78b2e836db13ceebc3bf0ccb2bed7a06 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Tue, 10 Sep 2019 18:50:24 -0700 Subject: [PATCH 21/24] go lint changes --- aws/default_retryer.go | 2 +- aws/no_op_retryer.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/aws/default_retryer.go b/aws/default_retryer.go index 96c1313792e..fd1f5279c43 100644 --- a/aws/default_retryer.go +++ b/aws/default_retryer.go @@ -69,7 +69,7 @@ func NewDefaultRetryer(opts ...func(d *DefaultRetryer)) DefaultRetryer { } // 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 { diff --git a/aws/no_op_retryer.go b/aws/no_op_retryer.go index d00474ee996..e0294dabcae 100644 --- a/aws/no_op_retryer.go +++ b/aws/no_op_retryer.go @@ -12,7 +12,7 @@ func (d NoOpRetryer) MaxRetries() int { return 0 } -// NoOpRetryer should never retry; so ShouldRetry will always return false. +// ShouldRetry will always return false for NoOpRetryer, as it should never retry. func (d NoOpRetryer) ShouldRetry(_ *Request) bool { return false } From 38f085712899fac39820c339929ae72b9a089b0b Mon Sep 17 00:00:00 2001 From: skotambkar Date: Wed, 11 Sep 2019 11:56:27 -0700 Subject: [PATCH 22/24] updates dynamodb customization to use default retryer; adds suggested changes --- CHANGELOG_PENDING.md | 7 ++++--- aws/default_retryer.go | 11 ++--------- service/dynamodb/customizations.go | 19 ++++--------------- 3 files changed, 10 insertions(+), 27 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 30ba01d9c17..814eac7e958 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -1,9 +1,10 @@ ### 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) +* `aws`: Adds configurations to the default retryer [#375](https://github.com/aws/aws-sdk-go-v2/pull/375) + * 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. + * 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. diff --git a/aws/default_retryer.go b/aws/default_retryer.go index fd1f5279c43..a7823ff461b 100644 --- a/aws/default_retryer.go +++ b/aws/default_retryer.go @@ -9,15 +9,8 @@ import ( ) // DefaultRetryer implements basic retry logic using exponential backoff for -// most services. If you want to implement custom retry logic, implement the -// Retryer interface or create a structure type that composes this -// struct and override the specific methods. For example, to override only -// the MaxRetries method: -// -// type retryer struct { -// client.DefaultRetryer -// } -// +// most services. You can implement your own custom retryer by implementing +// retryer interface. type DefaultRetryer struct { NumMaxRetries int MinRetryDelay time.Duration diff --git a/service/dynamodb/customizations.go b/service/dynamodb/customizations.go index f55aa48247a..9eb032ee433 100644 --- a/service/dynamodb/customizations.go +++ b/service/dynamodb/customizations.go @@ -5,7 +5,6 @@ import ( "hash/crc32" "io" "io/ioutil" - "math" "strconv" "time" @@ -13,15 +12,6 @@ import ( "github.com/aws/aws-sdk-go-v2/aws/awserr" ) -type retryer struct { - aws.DefaultRetryer -} - -func (d retryer) RetryRules(r *aws.Request) time.Duration { - delay := time.Duration(math.Pow(2, float64(r.RetryCount))) * 50 - return delay * time.Millisecond -} - func init() { initClient = func(c *Client) { if c.Config.Retryer == nil { @@ -43,11 +33,10 @@ func init() { } func setCustomRetryer(c *Client) { - c.Retryer = retryer{ - DefaultRetryer: aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { - d.NumMaxRetries = 10 - }), - } + c.Retryer = aws.NewDefaultRetryer(func(d *aws.DefaultRetryer) { + d.NumMaxRetries = 10 + d.MinRetryDelay = 50 * time.Millisecond + }) } func drainBody(b io.ReadCloser, length int64) (out *bytes.Buffer, err error) { From 6d5283600b293b19515b666b9fe87dad1de91835 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Wed, 11 Sep 2019 12:07:40 -0700 Subject: [PATCH 23/24] adds entries to the changelog pending --- CHANGELOG_PENDING.md | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 814eac7e958..9a46a0cc3ed 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -1,13 +1,27 @@ ### SDK Features ### SDK Enhancements -* `aws`: Adds configurations to the default retryer [#375](https://github.com/aws/aws-sdk-go-v2/pull/375) - * 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. +* `aws/endpoints`: Expose DNSSuffix for partitions ([#369](https://github.com/aws/aws-sdk-go-v2/pull/369)) + * Exposes the underlying partition metadata's DNSSuffix value via the `DNSSuffix` method on the endpoint's `Partition` type. This allows access to the partition's DNS suffix, e.g. "amazon.com". + * Fixes [#347](https://github.com/aws/aws-sdk-go-v2/issues/347) +* `private/protocol`: Add support for parsing fractional timestamp ([#367](https://github.com/aws/aws-sdk-go-v2/pull/367)) + * Fixes the SDK's ability to parse fractional unix timestamp values and adds tests. + * Fixes [#365](https://github.com/aws/aws-sdk-go-v2/issues/365) +* `aws/ec2metadata`: Add marketplaceProductCodes to EC2 Instance Identity Document ([#374](https://github.com/aws/aws-sdk-go-v2/pull/374)) + * Adds `MarketplaceProductCodes` to the EC2 Instance Metadata's Identity Document. The ec2metadata client will now retrieve these values if they are available. + * Related to: [aws/aws-sdk-go#2781](https://github.com/aws/aws-sdk-go/issues/2781) +* `aws`: Adds configurations to the default retryer ([#375](https://github.com/aws/aws-sdk-go-v2/pull/375)) + * 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. * 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 +### SDK Bugs +* `aws`: Fixes bug in calculating throttled retry delay ([#373](https://github.com/aws/aws-sdk-go-v2/pull/373)) + * The `Retry-After` duration specified in the request is now added to the Retry delay for throttled exception. Adds test for retry delays for throttled exceptions. Fixes bug where the throttled retry's math was off. + * Fixes [#45](https://github.com/aws/aws-sdk-go-v2/issues/45) +* `aws` : Adds missing sdk error checking when seeking readers ([#379](https://github.com/aws/aws-sdk-go-v2/pull/379)) + * Adds support for nonseekable io.Reader. Adds support for streamed payloads for unsigned body request. + * Fixes [#371](https://github.com/aws/aws-sdk-go-v2/issues/371) \ No newline at end of file From 604e9db7b65a168a599d5e88ec52c62e62e9d085 Mon Sep 17 00:00:00 2001 From: Shantanu Kotambkar <52007797+skotambkar@users.noreply.github.com> Date: Wed, 11 Sep 2019 12:32:24 -0700 Subject: [PATCH 24/24] Update CHANGELOG_PENDING.md --- CHANGELOG_PENDING.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 605cce68a1d..cc08e0dc0b3 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -14,8 +14,6 @@ * 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. * 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`: Fixes bug in calculating throttled retry delay ([#373](https://github.com/aws/aws-sdk-go-v2/pull/373)) @@ -24,4 +22,4 @@ * `aws` : Adds missing sdk error checking when seeking readers ([#379](https://github.com/aws/aws-sdk-go-v2/pull/379)) * Adds support for nonseekable io.Reader. Adds support for streamed payloads for unsigned body request. * Fixes [#371](https://github.com/aws/aws-sdk-go-v2/issues/371) - \ No newline at end of file +