Skip to content

Commit

Permalink
Next retry delay should not be bounded by retry policy MaximumInterval (
Browse files Browse the repository at this point in the history
#6063)

## What changed?
When an activity specifies `next_retry_delay` in the application
failure, we should be ignoring the max interval set in the retry policy.
This PR allows it.

## Why?
The intention of the application here is to override the retry policy
and customize the next attempt time.

https://github.com/temporalio/api/blob/5b356b506e0b86ba26dc6f795bfb011eeb4dfa6e/temporal/api/failure/v1/message.proto#L49

## How did you test it?
Added unit test.

## Potential risks
N/A

## Documentation
N/A

## Is hotfix candidate?
No
  • Loading branch information
gow authored Jun 5, 2024
1 parent d128f7d commit 7708a7c
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
8 changes: 7 additions & 1 deletion service/history/workflow/activity.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,20 @@ func newActivityVisitor(
}

now := timesource.Now().In(time.UTC)
retryMaxInterval := ai.RetryMaximumInterval
delay := nextRetryDelayFrom(failure)

// if a delay is specified by the application it should override the maximum interval set by the retry policy.
if delay != nil {
retryMaxInterval = durationpb.New(*delay)
}

backoff, retryState := nextBackoffInterval(
now,
ai.Attempt,
ai.RetryMaximumAttempts,
ai.RetryInitialInterval,
ai.RetryMaximumInterval,
retryMaxInterval,
ai.RetryExpirationTime,
ai.RetryBackoffCoefficient,
makeBackoffAlgorithm(delay),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,24 @@ func (s *retryActivitySuite) TestRetryActivity_should_be_scheduled_when_next_ret
s.assertTruncateFailureCalled()
}

func (s *retryActivitySuite) TestRetryActivity_next_retry_delay_should_override_max_interval() {
s.mutableState.timeSource = s.timeSource
taskGeneratorMock := NewMockTaskGenerator(s.controller)
nextAttempt := s.activity.Attempt + 1
expectedScheduledTime := s.timeSource.Now().Add(3 * time.Minute).UTC()
taskGeneratorMock.EXPECT().GenerateActivityRetryTasks(s.activity.ScheduledEventId, expectedScheduledTime, nextAttempt)
s.mutableState.taskGenerator = taskGeneratorMock

s.failure.GetApplicationFailureInfo().NextRetryDelay = durationpb.New(3 * time.Minute)
s.activity.RetryMaximumInterval = durationpb.New(2 * time.Minute) // set retry max interval to be less than next retry delay duration.
_, err := s.mutableState.RetryActivity(s.activity, s.failure)
s.NoError(err)
s.Equal(s.activity.Attempt, nextAttempt)

s.Equal(expectedScheduledTime, s.activity.ScheduledTime.AsTime(), "Activity scheduled time is incorrect")
s.assertTruncateFailureCalled()
}

func (s *retryActivitySuite) TestRetryActivity_when_no_next_backoff_interval_should_fail() {
taskGeneratorMock := NewMockTaskGenerator(s.controller)
s.mutableState.taskGenerator = taskGeneratorMock
Expand Down

0 comments on commit 7708a7c

Please sign in to comment.