Pin task timeout in tests that assume a single workflow task attempt - #1838
Closed
DABH wants to merge 1 commit into
Closed
Pin task timeout in tests that assume a single workflow task attempt#1838DABH wants to merge 1 commit into
DABH wants to merge 1 commit into
Conversation
Under CI load the first workflow task of these tests occasionally takes longer than the default 10s workflow task timeout. The server then retries the task, which re-runs the workflow from scratch: the patch activation callback is consulted once per attempt (observed as 'assert 2 == 1' in test_workflow_patch_activation_callback), and after two timed-out attempts the server rejects new updates with 'Workflow Task in failed state' (observed as RPCError in test_workflow_current_update). Both are correct server/SDK behaviors, so make the single-attempt precondition explicit by setting a task timeout that cannot expire within the test's own timeout.
Contributor
Author
|
Superseded by #1845, which raises the dev server's default workflow task timeout so this test does not need its own. Reopen if the maintainers prefer per-test timeouts. |
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 was changed
test_workflow_patch_activation_callback,test_workflow_patch_activation_callback_can_decline,test_workflow_patch_activation_callback_not_recalled_on_replayandtest_workflow_current_updatenow start their workflows withtask_timeout=timedelta(hours=1), the same idiomtest_workflow_hello_eageralready uses.Why
On loaded macOS CI runners the first workflow task of these tests is occasionally not completed within the default 10s workflow task timeout. Every failing job logs
Evicting workflow ... message: Error reporting WFT to server, which Core emits when the task completion is rejected with NOT_FOUND because the server already timed the task out. The server then retries the task from scratch, and two correct behaviors break the tests:assert len(calls) == 1sees 2 (run 34253408987 attempt 2, 3.14/macos-arm).FAILED_PRECONDITION: Unable to perform workflow execution update due to Workflow Task in failed state(failUpdateWorkflowTaskAttemptCount = 3in the history service).execute_updatere-sends its request when the server's 20s update long-poll expires, and that re-send is what failstest_workflow_current_updatewith the RPCError (run 33848127955 attempt 5, 3.14/macos-arm).Both tests are only meaningful for a single task attempt, so the long task timeout makes that precondition explicit instead of depending on the runner being fast. The 60s pytest timeout still bounds a genuinely stuck task, and the deadlock detector is unaffected.
Testing
task_timeout=1syieldscalls=2, twoWorkflowTaskStartedevents, oneWorkflowTaskTimedOutevent and the same eviction log as CI.calls=1/ all five update IDs.pytest --flake-finder --flake-runs=30of the four tests alongside a concurrent-n 8pytest session: all passes (30 runs each; the run covered the eight tests from the same investigation, 240/240 in total, load average 45-53). The unmodified tests also pass 240/240 on the same machine, so the CI failures only reproduce through the stall emulation above.poe lintclean.