Skip to content

Commit 026200f

Browse files
authored
Breaking change to current_retry_interval (#11203)
## What changed? - current_retry_interval=nil after dispatch to Matching. Previously we were returning the predicted next retry interval - current_retry_interval and next_attempt_schedule_time both nil when paused Consider an activity that's in retry backoff before attempt 2. This retry interval is 5s and the next one, if there is another, will be 10s. `current_retry_interval`: ``` SCHEDULED (dispatched to matching) STARTED (attempt completed/failed) before 5s 10s nil after 5s nil nil ``` `next_attempt_schedule_time` for comparison: ``` SCHEDULED (dispatched to matching at t) STARTED (attempt completed/failed) t nil nil ``` ## Why? I think that the current behavior is a bug. ## How did you test it? - [x] added new unit test(s) ## Potential risks Breaking change: someone could be relying on the old behavior <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Intentional breaking change to DescribeWorkflowExecution/DescribeActivityExecution metadata; clients that relied on post-dispatch predicted retry intervals will see different values. > > **Overview** > **Breaking change:** `current_retry_interval` on pending activities is no longer a predicted *next* backoff after the retry has been dispatched to Matching. It is set only while the server is still waiting to dispatch the retry (`ScheduledTime` in the future and not paused), as the elapsed time from last attempt completion to that schedule time—aligned with when `next_attempt_schedule_time` is set. > > Once the backoff elapses and the task is in Matching (or the activity is **paused**), both `current_retry_interval` and `next_attempt_schedule_time` are **nil**, including for paused activities that would still be “backing off” internally. > > Coverage moves to a focused unit test on `GetPendingActivityInfo`, a workflow-vs-standalone parity suite (`TestParityCurrentRetryInterval`), removal of the older standalone-only test, and XDC pause/failover expectations updated to expect nil `CurrentRetryInterval`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 5676d92. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent b2fab54 commit 026200f

5 files changed

Lines changed: 491 additions & 212 deletions

File tree

service/history/workflow/activity.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,17 +120,15 @@ func GetPendingActivityInfo(
120120
p.Attempt = ai.Attempt
121121
if p.State == enumspb.PENDING_ACTIVITY_STATE_SCHEDULED {
122122
scheduledTime := ai.ScheduledTime.AsTime()
123-
if now.Before(scheduledTime) {
124-
// in this case activity is waiting for a retry
123+
if now.Before(scheduledTime) && !ai.Paused {
124+
// waiting for the retry to be dispatched to Matching
125125
p.NextAttemptScheduleTime = ai.ScheduledTime
126126
currentRetryDuration := p.NextAttemptScheduleTime.AsTime().Sub(p.LastAttemptCompleteTime.AsTime())
127127
p.CurrentRetryInterval = durationpb.New(currentRetryDuration)
128128
} else {
129-
// in this case activity is at least scheduled
129+
// retry has been dispatched to Matching, or the activity is paused so no dispatch will occur
130130
p.NextAttemptScheduleTime = nil
131-
// we rely on the fact that ExponentialBackoffAlgorithm is deterministic, and there's no random jitter
132-
interval := backoff.ExponentialBackoffAlgorithm(ai.RetryInitialInterval, ai.RetryBackoffCoefficient, p.Attempt)
133-
p.CurrentRetryInterval = durationpb.New(interval)
131+
p.CurrentRetryInterval = nil
134132
}
135133
}
136134
}

service/history/workflow/activity_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,36 @@ func (s *activitySuite) TestGetPendingActivityInfoHasRetryPolicy() {
294294
s.Equal(ai.RetryMaximumAttempts, pi.ActivityOptions.RetryPolicy.MaximumAttempts)
295295
}
296296

297+
func (s *activitySuite) TestGetPendingActivityInfoNextAttemptScheduleTimeAndCurrentRetryInterval() {
298+
now := s.mockShard.GetTimeSource().Now().UTC()
299+
activityType := commonpb.ActivityType{
300+
Name: "activityType",
301+
}
302+
ai := &persistencespb.ActivityInfo{
303+
StartedEventId: common.EmptyEventID,
304+
LastAttemptCompleteTime: timestamppb.New(now),
305+
HasRetryPolicy: true,
306+
}
307+
s.mockMutableState.EXPECT().GetActivityType(gomock.Any(), gomock.Any()).Return(&activityType, nil).Times(2)
308+
309+
// Before dispatch to Matching: waiting for the retry, so we report when the next attempt is
310+
// scheduled and the interval until then.
311+
ai.ScheduledTime = timestamppb.New(now.Add(5 * time.Second))
312+
pi, err := GetPendingActivityInfo(context.Background(), s.mockShard, s.mockMutableState, ai)
313+
s.NoError(err)
314+
s.Equal(enumspb.PENDING_ACTIVITY_STATE_SCHEDULED, pi.State)
315+
s.Equal(ai.ScheduledTime, pi.NextAttemptScheduleTime)
316+
s.Equal(durationpb.New(5*time.Second), pi.CurrentRetryInterval)
317+
318+
// After dispatch to Matching: no next attempt schedule time or current retry interval.
319+
ai.ScheduledTime = timestamppb.New(now.Add(-1 * time.Minute))
320+
pi, err = GetPendingActivityInfo(context.Background(), s.mockShard, s.mockMutableState, ai)
321+
s.NoError(err)
322+
s.Equal(enumspb.PENDING_ACTIVITY_STATE_SCHEDULED, pi.State)
323+
s.Nil(pi.NextAttemptScheduleTime)
324+
s.Nil(pi.CurrentRetryInterval)
325+
}
326+
297327
func (s *activitySuite) AddActivityInfo() *persistencespb.ActivityInfo {
298328
activityId := "activity-id"
299329
activityScheduledEvent := &historypb.HistoryEvent{

0 commit comments

Comments
 (0)