Skip to content

Commit

Permalink
[wip] Fix retry policy attempts to match server behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
Groxx committed Dec 26, 2024
1 parent 1fd8ba0 commit 3e71788
Showing 1 changed file with 12 additions and 1 deletion.
13 changes: 12 additions & 1 deletion internal/internal_task_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,18 @@ func getRetryBackoffWithNowTime(p *RetryPolicy, attempt int32, errReason string,
return noRetryBackoff
}

if p.MaximumAttempts > 0 && attempt > p.MaximumAttempts-1 {
if p.MaximumAttempts > 0 && attempt >= p.MaximumAttempts-1 {
// >=max-1 matches server behavior, which treats all this somewhat oddly, but it has been consistent for a long time.
// basically:
// - attempts means *retry attempts*, as it's only relevant for retries
// - max attempts means *total executions*, counting the first and all retries
// so e.g. max=3 means 3 calls, with attempt counts 0, 1, 2.
//
// first==0 makes the backoff interval below convenient (no coefficient),
// otherwise this feels confusing and it contributes to RetryPolicy's ambiguities,
// as some things apply to the *policy* (expiration, attempts displayed) and some to all executions (max attempts).
//
// we may be able to change this with a completely new API, but for now it must not change for backwards compat.
return noRetryBackoff // max attempt reached
}

Expand Down

0 comments on commit 3e71788

Please sign in to comment.