Add support for standalone activity termination #8665
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>
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.