Drop reset_attempts and reset_heartbeat from UnpauseActivityExecution - #846
Merged
Conversation
Unpause resumes an activity; it should not also mutate the attempt counter or discard the heartbeat checkpoint. Neither selective form has demonstrated user demand, and an operator who wants either would look for it on Reset. Ship the simple API first. The field numbers and names are reserved. The deprecated UnpauseActivity, the batch operations, and the Reset APIs are untouched. make proto
Drops reset_attempts and reset_heartbeat from the deprecated UnpauseActivity and from BatchOperationUnpauseActivities, so unpause means the same thing on every surface. Numbers and names reserved. The Reset APIs keep their flags: Reset is the operation that restarts an activity as if on its first attempt. make proto
This reverts commit 1cc925e.
dandavison
requested review from
a team,
GregoryTravis,
fretz12,
maciejdudko and
spkane31
August 1, 2026 23:28
Merged
1 task
spkane31
approved these changes
Aug 3, 2026
spkane31
left a comment
Contributor
There was a problem hiding this comment.
Do we need to reserve these keywords if this API has never been used in the server? IMO we can keep the api surface a bit cleaner by not using the reserve keyword and acknowledging this API has never been exposed and isn't a real breaking change.
Contributor
Author
We should, because the API protos have been released, and our SDKs have released autogenerated clients that will send those fields in positions 6 and 7. |
fretz12
approved these changes
Aug 3, 2026
maciejdudko
approved these changes
Aug 3, 2026
GregoryTravis
approved these changes
Aug 3, 2026
dandavison
added a commit
to temporalio/temporal
that referenced
this pull request
Aug 3, 2026
…ution` (#11393) See API change temporalio/api#846 ## What changed? - Drop `reset_attempts` and `reset_heartbeat` from `UnpauseActivityExecution` ## Why? - We have so far been unable to assign desirable and consistent semantics to them during implementation: for example if `Unpause[resetAttempts]` is received during retry backoff it is unclear whether to honor the remaining delay time, because this is how Unpause usually behaves, or dispatch immediately, because this is how Reset behaves. - No known user demand - They are confusing: they mix `Unpause` and `Reset` functionality in a confusing way - They can be added later ## How did you test it? - [x] covered by existing tests ## Breaking changes - This API has always been rejected by the server. When server starts to accept it, an old client could submit these options and they would be ignored. Operator API is not GA. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes activity unpause scheduling semantics for the execution API and drops reset-on-unpause behavior that was only partially implemented; low user impact if the API was not GA and had no known callers. > > **Overview** > Aligns the server with the **UnpauseActivityExecution** API change: **`reset_attempts`** and **`reset_heartbeat`** are no longer part of unpause for standalone (CHASM) activities. > > **CHASM activity unpause** no longer resets attempt count, retry interval, or heartbeat state on unpause, and always considers the pending retry backoff when scheduling dispatch (the branch that skipped that when `reset_attempts` was set is removed). Workflow-embedded unpause forwarding via **`UnpauseActivityExecution`** no longer passes those fields to the legacy **`UnpauseActivity`** history call (jitter and identity only). > > **`go.temporal.io/api`** is bumped to the revision that removes the fields from **`UnpauseActivityExecutionRequest`**. > > **Tests** are updated so unpause helpers no longer take a reset flag; reset-on-unpause coverage stays on legacy **`UnpauseActivity`** only (execution API skips that case). Standalone tests for **`UnpauseWithResetAttempts`** and **`UnpauseWithResetHeartbeat`** on **`UnpauseActivityExecution`** are removed. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 7a41d50. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed?
reset_attemptsandreset_heartbeatfromUnpauseActivityExecutionWhy?
UnpauseandResetfunctionality in a confusing wayA commit for extending this to WFA is reverted in history; we can do this later.
Breaking changes