Skip to content

Commit 92ea4c7

Browse files
skotambkarjasdel
authored andcommitted
aws: Fixes bug in calculating throttled retry delay (#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
1 parent 502c817 commit 92ea4c7

File tree

3 files changed

+48
-11
lines changed

3 files changed

+48
-11
lines changed

CHANGELOG_PENDING.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,7 @@
99
* Fixes [#365](https://github.com/aws/aws-sdk-go-v2/issues/365)
1010

1111
### SDK Bugs
12-
12+
* `aws`: Fixes bug in calculating throttled retry delay ([#373](https://github.com/aws/aws-sdk-go/pull/373))
13+
* 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.
14+
* Fixes [#45](https://github.com/aws/aws-sdk-go/issues/45)
15+

aws/default_retryer.go

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,25 +34,27 @@ var seededRand = rand.New(&lockedSource{src: rand.NewSource(time.Now().UnixNano(
3434
// RetryRules returns the delay duration before retrying this request again
3535
func (d DefaultRetryer) RetryRules(r *Request) time.Duration {
3636
// Set the upper limit of delay in retrying at ~five minutes
37-
minTime := 30
37+
var minTime int64 = 30
38+
var initialDelay time.Duration
39+
3840
throttle := d.shouldThrottle(r)
3941
if throttle {
40-
if delay, ok := getRetryDelay(r); ok {
41-
return delay
42+
if delay, ok := getRetryAfterDelay(r); ok {
43+
initialDelay = delay
4244
}
4345

4446
minTime = 500
4547
}
4648

4749
retryCount := r.RetryCount
48-
if retryCount > 13 {
49-
retryCount = 13
50-
} else if throttle && retryCount > 8 {
50+
if throttle && retryCount > 8 {
5151
retryCount = 8
52+
} else if retryCount > 12 {
53+
retryCount = 12
5254
}
5355

54-
delay := (1 << uint(retryCount)) * (seededRand.Intn(minTime) + minTime)
55-
return time.Duration(delay) * time.Millisecond
56+
delay := (1 << uint(retryCount)) * (seededRand.Int63n(minTime) + minTime)
57+
return (time.Duration(delay) * time.Millisecond) + initialDelay
5658
}
5759

5860
// ShouldRetry returns true if the request should be retried.
@@ -85,7 +87,7 @@ func (d DefaultRetryer) shouldThrottle(r *Request) bool {
8587

8688
// This will look in the Retry-After header, RFC 7231, for how long
8789
// it will wait before attempting another request
88-
func getRetryDelay(r *Request) (time.Duration, bool) {
90+
func getRetryAfterDelay(r *Request) (time.Duration, bool) {
8991
if !canUseRetryAfterHeader(r) {
9092
return 0, false
9193
}

aws/default_retryer_test.go

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ func TestGetRetryDelay(t *testing.T) {
152152
}
153153

154154
for i, c := range cases {
155-
a, ok := getRetryDelay(&c.r)
155+
a, ok := getRetryAfterDelay(&c.r)
156156
if c.ok != ok {
157157
t.Errorf("%d: expected %v, but received %v", i, c.ok, ok)
158158
}
@@ -162,3 +162,35 @@ func TestGetRetryDelay(t *testing.T) {
162162
}
163163
}
164164
}
165+
166+
func TestRetryDelay(t *testing.T) {
167+
d := DefaultRetryer{100}
168+
r := Request{}
169+
for i := 0; i < 100; i++ {
170+
rTemp := r
171+
rTemp.HTTPResponse = &http.Response{StatusCode: 500, Header: http.Header{"Retry-After": []string{"299"}}}
172+
rTemp.RetryCount = i
173+
a := d.RetryRules(&rTemp)
174+
if a > 5*time.Minute {
175+
t.Errorf("retry delay should never be greater than five minutes, received %s for retrycount %d", a, i)
176+
}
177+
}
178+
179+
for i := 0; i < 100; i++ {
180+
rTemp := r
181+
rTemp.RetryCount = i
182+
rTemp.HTTPResponse = &http.Response{StatusCode: 503, Header: http.Header{"Retry-After": []string{""}}}
183+
a := d.RetryRules(&rTemp)
184+
if a > 5*time.Minute {
185+
t.Errorf("retry delay should not be greater than five minutes, received %s for retrycount %d", a, i)
186+
}
187+
}
188+
189+
rTemp := r
190+
rTemp.RetryCount = 1
191+
rTemp.HTTPResponse = &http.Response{StatusCode: 503, Header: http.Header{"Retry-After": []string{"300"}}}
192+
a := d.RetryRules(&rTemp)
193+
if a < 5*time.Minute{
194+
t.Errorf("retry delay should not be less than retry-after duration, received %s for retrycount %d", a, 1)
195+
}
196+
}

0 commit comments

Comments
 (0)