Add support for FirstExecutionRunId to cancel and terminate#2692
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| if (workflowChainId != null) { | ||
| TestWorkflowMutableState mutableStateByFirstRunId = getMutableState(workflowChainId, false); | ||
| if (mutableStateByFirstRunId != null) { | ||
| return mutableStateByFirstRunId; | ||
| } | ||
| } | ||
|
|
||
| TestWorkflowMutableState mutableState = getMutableState(executionId, false); | ||
| if (mutableState != null) { | ||
| if (workflowChainId == null) { | ||
| return mutableState; | ||
| } | ||
| WorkflowExecution mutableStateExecution = mutableState.getExecutionId().getExecution(); | ||
| if (mutableStateExecution.getRunId().equals(execution.getRunId()) | ||
| && firstExecutionRunId.equals(mutableState.getFirstExecutionRunId())) { | ||
| return mutableState; | ||
| } | ||
| } |
There was a problem hiding this comment.
Considering that workflow executions that have firstRunId are added to both maps in startWorkflowExecutionNoRunningCheckLocked, in what circumstances will the last if evaluate true (line 295)? If there is an execution with a matching first run ID, I'd expect it to have already returned inside the earlier if (line 285).
There was a problem hiding this comment.
Yeah good point, I think I overcomplicated the check. I simplified it a lot and should match how the current real server finds the correct mutable state. All the tests run on the real server and test server to validate this.
There was a problem hiding this comment.
We should add negative tests for what happens when the wrong first execution ID is provided to cancel/terminate request (both when there was and there wasn't continue-as-new).
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Quinn-With-Two-Ns
force-pushed
the
test-server-first-exec-run-id
branch
from
fceb387 to
7856212
Compare
2 participants
Add support for FirstExecutionRunId to cancel and terminate to the Time skipping test server.
Note
Enable cancel and terminate operations to target a workflow chain via
firstExecutionRunId, not just the currentrunId.firstExecutionRunIdusingWorkflowChainId(namespace,workflowId,firstExecutionRunId).executionsByFirstExecutionRunId; exposegetFirstExecutionRunId()on mutable state.requestCancelWorkflowExecutionandterminateWorkflowExecutionto resolve executions viafirstExecutionRunIdwhen provided.FirstExecutionRunIdSupportTestcovering cancel/terminate byfirstExecutionRunIdand after continue-as-new.Written by Cursor Bugbot for commit 7856212. This will update automatically on new commits. Configure here.