Accept CancelTimer for a timer that fired during a workflow task - #3060
Draft
cameronhotchkies wants to merge 1 commit into
Draft
Accept CancelTimer for a timer that fired during a workflow task#3060cameronhotchkies wants to merge 1 commit into
cameronhotchkies wants to merge 1 commit into
Conversation
A timer that fires in real time while a workflow task is in progress has its TIMER_FIRED event buffered until the workflow task completion and its state machine removed from the timers map. If the workflow task then responds with a CancelTimer command for that timer, processCancelTimer found no state machine and threw INVALID_ARGUMENT "invalid history builder state for action". The server kept rejecting the completion, so the workflow task stayed outstanding forever and the workflow could never progress. When a CancelTimer command references a timer without a state machine, search the buffered events for its TIMER_FIRED event. If one is found, remove it and record a TIMER_CANCELED event in its place instead of failing. This mirrors the real server, which removes the buffered TimerFired event and still writes a TimerCanceled with the started event id of the fired timer. Committing the buffered TIMER_FIRED followed by a TIMER_CANCELED would break the SDK timer state machine during replay, so the removal is required. The unhandledCommand check also now ignores a buffered TIMER_FIRED event that a CancelTimer command from the same response replaces, so a workflow completion command sent alongside the cancel is accepted, like on the real server, which clears its buffered events flag when the cancel command is processed. Fixes temporalio#2606
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?
The in-process test server (
temporal-test-server) now accepts aCancelTimercommand for a timer that fired in real time while the current workflow task was still in progress, instead of failing the workflow task completion withINVALID_ARGUMENT "invalid history builder state for action".processCancelTimer: when there is no state machine for the timer, it searches the buffered events for that timer'sTIMER_FIREDevent, removes it, and records aTIMER_CANCELEDevent in its place (newStateMachines.cancelFiredTimerhelper, using the fired timer'sstartedEventId), mirroring the real server'sAddTimerCanceledEvent, which callsGetAndRemoveTimerFireEventon the buffered TimerFired and still writes a TimerCanceled.unhandledCommand: a bufferedTIMER_FIREDevent that aCancelTimercommand in the same response replaces no longer counts as an unhandled new event, so a workflow completion command can be accepted alongside the cancel. This matches the real server, wherehandleCommandCancelTimerclears the handler's buffered-events flag and the completion command (validated to be the last command) checks it afterwards.Why?
A timer that fires while a workflow task is in progress has its
TIMER_FIREDevent buffered until the task completion (and its state machine removed from the timers map). A worker that still sees the timer as pending can respond withCancelTimer, which the test server rejected — and kept rejecting on every retry, leaving the workflow task outstanding forever and wedging the workflow (see #2606 for the stuck histories). This affects tests usingTestWorkflowEnvironmentthat start a timer and cancel it, with the cancel racing the timer firing during a long-running workflow task.The buffered
TIMER_FIREDmust be removed rather than committed ahead of theTIMER_CANCELED: the SDK'sTimerStateMachinehas no transition for aTIMER_FIREDin theCANCEL_TIMER_COMMAND_SENTstate, so committing both events would break replay determinism. The removal is safe because buffered events carry no real event id yet (ids are assigned at save time).Breaking changes?
None. All changes are confined to the test server's internal classes (
io.temporal.internal.testservice), which are not part of the public API.Server PR
None. The change aligns the test server with existing real server behavior; no coordinated server update is needed.
Testing
CancelFiredTimerTestintemporal-test-server/src/test/java/io/temporal/testserver/functional/reproduces the race deterministically at the gRPC level (a short real-time timer fired while a workflow task is in progress; time skipping stays locked by an outstanding activity so the timer only fires in real time):cancelFiredTimerWithoutWorkflowCompletionfails with the exactinvalid history builder state for actionerror without the fix, and with it verifies the bufferedTIMER_FIREDis replaced byTIMER_CANCELEDand the workflow still progresses and completes on the next workflow task.cancelFiredTimerWithWorkflowCompletionfails withUnhandledCommandwithout the fix, and with it verifies[CancelTimer, CompleteWorkflowExecution]is accepted in one response.mainand pass with the fix../gradlew :temporal-test-server:test,./gradlew :temporal-testing:test, the timer-relatedtemporal-sdkworkflow tests (TimerTest,MultipleTimersTest,TimerCallbackBlockedTest), and./gradlew spotlessApplyall pass.Fixes #2606