Fix retry policy max attempts (#4752)

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

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
 	}
 

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

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()