remove reset_heartbeats field in ResetActivityExecutionRequest#10954
Open
spkane31 wants to merge 6 commits into
Open
remove reset_heartbeats field in ResetActivityExecutionRequest#10954spkane31 wants to merge 6 commits into
spkane31 wants to merge 6 commits into
Conversation
Implements Pause, Unpause, Reset, and UpdateActivityExecutionOptions RPCs for standalone CHASM activities. Adds matching frontend and history service handlers in chasm/lib/activity that dispatch to either the workflow-activity path or the standalone-activity path based on the presence of a workflow ID. Also adds PAUSE_REQUESTED and RESET_REQUESTED state machine states for standalone activities, the original_options snapshot for restore-on-reset semantics, the activity-options merge helpers (common/activityoptions), and the supporting proto schema changes (start_request_id, sdk_name, sdk_version, etc.). Squash of the 29 commits previously on this branch. Original PRs: #9693 RPC boilerplate and code generation #9850 Implement UpdateActivityExecutionOptions #9851 Implement Pause/UnpauseActivityExecution for SAA #9852 Implement ResetActivityExecution for SAA #10001 Pause returns FailedPrecondition when already paused #10025 Validator function fixes #10265 Replace PauseState flag with PAUSE_REQUESTED state #10364 Add RESET_REQUESTED state to standalone activity
## What changed? `start_delay` is now updatable via `UpdateActivityOptions` for standalone activities. The new value is anchored to the original `schedule_time`, so users can shorten it (including to 0, which dispatches immediately if the target is already past), extend it, or restore the original via `RestoreOriginal`. Updates once the activity has left its delay window (first dispatch passed, or no longer `SCHEDULED`) are rejected with `InvalidArgument`. The merge, validation, and namespace dynamic-config check from activity creation are reused, so create and update stay consistent. This also adds the shared infrastructure for the downstream PRs: - A persisted `first_attempt_started_time` on `ActivityState`, set once on the first `SCHEDULED → STARTED` and never updated on retries/resets. It is the canonical "has the first dispatch happened" discriminator. - A `respectStartDelay` helper that lifts a candidate dispatch time to `scheduleTime + start_delay` until first pickup, and no-ops afterward. It drives `ActivityDispatchTask` re-issuance so an unrelated update in the delay window no longer dispatches early, and gates the `RestoreOriginal` restore so it never shows a stale value post-dispatch. - Timer re-issuance split into `reissueRunningAttemptTimers` (StartToClose/Heartbeat) and `reissueScheduledDispatch` (Dispatch/ScheduleToStart). Also changes a pre-existing behavior where the update path anchored `ScheduleToClose` to `scheduleTime + timeout` instead of `firstDispatchTime + timeout`; it now uses the canonical `scheduleToCloseDeadline()`, matching `TransitionScheduled` and `hasEnoughTimeForRetry`. ## Why? Run-now (cancel-delay / kickstart) is table-stakes in the job queues SAA is replacing (BullMQ, Sidekiq, Cloud Tasks, Asynq) and a GA recommendation. Exposing `start_delay` through `UpdateActivityOptions` covers both run-now (set 0) and rescheduling with one primitive, using the same anchored-to-schedule-time semantics as creation. `start_delay` is a first-attempt parameter, so `first_attempt_started_time` keeps every re-scheduling path from re-applying or restoring it after dispatch, where it would only mislead. The `ScheduleToClose` change prevents an unrelated update during the delay window from shrinking the deadline (sometimes into the past) and timing the activity out before it dispatches. ## How did you test it? - [X] built - [X] run locally and tested manually - [X] covered by existing tests - [ ] added new unit test(s) - [X] added new functional test(s) --------- Co-authored-by: Dan Davison <dan.davison@temporal.io>
There was a problem hiding this comment.
Pull request overview
This PR simplifies the activity reset API/behavior by making activity resets always clear heartbeat details, removing the now-unnecessary reset_heartbeats state, and updating state machine behavior and tests accordingly.
Changes:
- Always clear stored heartbeat details on activity reset (including deferred reset application paths).
- Remove
reset_heartbeatsfrom the CHASMActivityStateproto/state and delete associated state machine logic. - Update functional/unit tests to reflect the new “reset always clears heartbeat” semantics.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tests/activity_standalone_test.go |
Updates standalone reset tests to remove the reset-heartbeat toggle and assert heartbeat clearing as the default behavior. |
chasm/lib/activity/statemachine.go |
Removes ResetHeartbeats state handling and makes deferred reset transitions always clear heartbeat state. |
chasm/lib/activity/statemachine_test.go |
Replaces the old reset-flag tests with focused unit tests that verify heartbeat state is cleared on reset/deferred reset. |
chasm/lib/activity/proto/v1/activity_state.proto |
Removes the reset_heartbeats field from persisted activity state. |
chasm/lib/activity/handler.go |
Forces reset-heartbeat behavior on the workflow-path reset request (consistent with “always clear”). |
chasm/lib/activity/gen/activitypb/v1/activity_state.pb.go |
Regenerated/updated protobuf output reflecting the removed field. |
chasm/lib/activity/activity.go |
Makes reset paths unconditionally clear heartbeat state and removes now-unused request-driven branching. |
Files not reviewed (1)
- chasm/lib/activity/gen/activitypb/v1/activity_state.pb.go: Generated file
| @@ -128,11 +128,6 @@ message ActivityState { | |||
| // metadata only. | |||
| ActivityPauseState last_pause_state = 16; | |||
fretz12
approved these changes
Jul 9, 2026
| }, | ||
| Activity: &workflowservice.ResetActivityRequest_Id{Id: frontendReq.GetActivityId()}, | ||
| ResetHeartbeat: frontendReq.GetResetHeartbeat(), | ||
| ResetHeartbeat: true, |
Contributor
There was a problem hiding this comment.
Do you think we should remove this from the proto as well since it's always true now?
fretz12
force-pushed
the
feature/activity-operator-cmds
branch
from
6d922e5 to
9ccc41b
Compare
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?
Resetting an activity execution now always clears heartbeat details.
temporalio/api#820 Removes the
reset_heartbeatfield from the Request message.Why?
This change simplifies the API surface and we (the engineering/product team) do not see a good use case for resetting an activity and keeping heartbeat data. If this is needed or there is a big customer ask for it we can add this back in the future.
How did you test it?
Potential risks
Minimal, PR is into a feature branch