Add support for standalone activity termination#8665
Add support for standalone activity termination#8665fretz12 merged 111 commits intostandalone-activityfrom
Conversation
fretz12
force-pushed
the
saa-terminate-activity
branch
from
8710d0b to
de5e4f2
Compare
|
cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
chasm/lib/activity/statemachine.go
Outdated
| } | ||
|
|
||
| return store.RecordCompleted(ctx, func(ctx chasm.MutableContext) error { | ||
| attempt, err := a.Attempt.Get(ctx) |
There was a problem hiding this comment.
Here we're updating the last worker identity from the termination req. If the identity should not be updated on termination, lmk.
There was a problem hiding this comment.
This is incorrect, it's not safe to assume that the identity in the termination request is the identity of a worker processing the current attempt. Most likely it'll be a separate client.
There was a problem hiding this comment.
ok, removed.
What's the purpose of the Identity field in TerminateActivityExecutionRequest? For our purposes should we ignore it?
chasm/lib/activity/activity.go
Outdated
| return heartbeat, nil | ||
| } | ||
|
|
||
| func (a *Activity) handleTerminated(ctx chasm.MutableContext, req *activitypb.TerminateActivityExecutionRequest) ( |
There was a problem hiding this comment.
We should dedupe based on the request ID. I forgot that detail in my initial review.
There was a problem hiding this comment.
Since this requires API change, I'll do this as a separate PR.
| } | ||
|
|
||
| failure := &failurepb.Failure{ | ||
| Message: req.GetFrontendRequest().GetReason(), |
There was a problem hiding this comment.
Put a TODO to revisit this. If the reason isn't provided we'll have an empty message, which isn't great.
I'm also wondering if we want to prefix the reason with:
"Activity terminated: " + req.GetFrontendRequest().GetReason().
Best bring this up in the crew channel on slack. Same for cancelation reason.
There was a problem hiding this comment.
Added TODO, will ping crew
## What changed? Added standalone activity termination handling. ## Why? Needed to support standalone activities full operation. ## How did you test it? - [X] built - [X] run locally and tested manually - [X] covered by existing tests - [X] added new unit test(s) - [X] added new functional test(s) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds termination support for standalone activities across API, frontend/handler, state machine, and tests. > > - **API/Proto**: > - Add `TerminateActivityExecution` RPC: request/response messages in `proto/v1/request_response.proto` and service in `proto/v1/service.proto`. > - Regenerate clients/servers: `activitypb` request/response, service, layered client, and gRPC stubs. > - **Frontend**: > - Implement `TerminateActivityExecution` in `frontend.go` to resolve namespace and forward to activity service. > - **Backend/Handler**: > - Add handler `TerminateActivityExecution` to update component via `(*Activity).handleTerminated`. > - Add `Activity.handleTerminated` to apply termination transition. > - **State Machine**: > - Enhance `TransitionTerminated` to finalize execution, set `LastWorkerIdentity`, and record `TerminatedFailureInfo` in outcome. > - **Tests**: > - Add unit test `TestTransitionTerminated`. > - Add functional tests for terminating an activity and preventing termination after completion. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit de5e4f2. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Dan Davison <dan.davison@temporal.io>
3 participants
What changed?
Added standalone activity termination handling.
Why?
Needed to support standalone activities full operation.
How did you test it?
Note
Adds termination support for standalone activities across API, frontend/handler, state machine, and tests.
TerminateActivityExecutionRPC: request/response messages inproto/v1/request_response.protoand service inproto/v1/service.proto.activitypbrequest/response, service, layered client, and gRPC stubs.TerminateActivityExecutioninfrontend.goto resolve namespace and forward to activity service.TerminateActivityExecutionto update component via(*Activity).handleTerminated.Activity.handleTerminatedto apply termination transition.TransitionTerminatedto finalize execution, setLastWorkerIdentity, and recordTerminatedFailureInfoin outcome.TestTransitionTerminated.Written by Cursor Bugbot for commit de5e4f2. This will update automatically on new commits. Configure here.