diff --git a/common/backoff/retry_test.go b/common/backoff/retry_test.go index af281eddeff..75ad9ed6986 100644 --- a/common/backoff/retry_test.go +++ b/common/backoff/retry_test.go @@ -234,7 +234,7 @@ func (s *RetrySuite) TestThrottleRetryContext() { throttleRetryPolicy = testThrottleRetryPolicy policy := NewExponentialRetryPolicy(10 * time.Millisecond). - WithMaximumAttempts(1) + WithMaximumAttempts(2) // test if throttle retry policy is used on resource exhausted error attempt := 1 diff --git a/common/backoff/retrypolicy.go b/common/backoff/retrypolicy.go index 65efffaaaa5..277ac56c57a 100644 --- a/common/backoff/retrypolicy.go +++ b/common/backoff/retrypolicy.go @@ -162,7 +162,8 @@ func (p *ExponentialRetryPolicy) WithMaximumAttempts(maximumAttempts int) *Expon // ComputeNextDelay returns the next delay interval. This is used by Retrier to delay calling the operation again func (p *ExponentialRetryPolicy) ComputeNextDelay(elapsedTime time.Duration, numAttempts int) time.Duration { // Check to see if we ran out of maximum number of attempts - if p.maximumAttempts != noMaximumAttempts && numAttempts > p.maximumAttempts { + // NOTE: if maxAttempts is X, return done when numAttempts == X, otherwise there will be attempt X+1 + if p.maximumAttempts != noMaximumAttempts && numAttempts >= p.maximumAttempts { return done } diff --git a/common/backoff/retrypolicy_test.go b/common/backoff/retrypolicy_test.go index 932a535683c..ddd96fdea83 100644 --- a/common/backoff/retrypolicy_test.go +++ b/common/backoff/retrypolicy_test.go @@ -113,16 +113,18 @@ func (s *RetryPolicySuite) TestExponentialBackoff() { } func (s *RetryPolicySuite) TestNumberOfAttempts() { + maxAttempts := 5 policy := createPolicy(time.Second). - WithMaximumAttempts(5) + WithMaximumAttempts(maxAttempts) r, _ := createRetrier(policy) var next time.Duration - for i := 0; i < 6; i++ { + for i := 0; i < maxAttempts-1; i++ { next = r.NextBackOff() + s.NotEqual(done, next) } - s.Equal(done, next) + s.Equal(done, r.NextBackOff()) } // Test to make sure relative maximum interval for each retry is honoured diff --git a/service/history/shard/context_test.go b/service/history/shard/context_test.go index a90ecd558bb..92ba22c6b4d 100644 --- a/service/history/shard/context_test.go +++ b/service/history/shard/context_test.go @@ -415,9 +415,8 @@ func (s *contextSuite) TestAcquireShardNonOwnershipLostErrorIsRetried() { s.mockShard.state = contextStateAcquiring s.mockShard.acquireShardRetryPolicy = backoff.NewExponentialRetryPolicy(time.Nanosecond). WithMaximumAttempts(5) - // TODO: make this 5 times instead of 6 when retry policy is fixed s.mockShardManager.EXPECT().UpdateShard(gomock.Any(), gomock.Any()). - Return(fmt.Errorf("temp error")).Times(6) + Return(fmt.Errorf("temp error")).Times(5) s.mockShard.acquireShard()