Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release WF slot on UnableToAcquireLockException #1871

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

Quinn-With-Two-Ns
Copy link
Contributor

A user noticed that if their workflow task failed with UnableToAcquireLockException their worker would not be able to shutdown. Root cause was on UnableToAcquireLockException we didn't call task.getCompletionCallback().apply(); because the exception was outside the finally block.

Note: this will change the timing of WORKFLOW_TASK_EXECUTION_TOTAL_LATENCY to include the time waiting for the run lock, but this metric is java specific and is undocumented, and arguably should include the run lock.

close #1870

@Quinn-With-Two-Ns Quinn-With-Two-Ns requested a review from a team as a code owner September 25, 2023 22:12
@@ -189,7 +189,7 @@ public void concurrentPollRequestLockTest() throws Exception {
ImmutableMap.of("worker_type", "WorkflowWorker"),
100.0);
// Cleanup
worker.shutdown(new ShutdownManager(), true).get();
worker.shutdown(new ShutdownManager(), false).get();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using false here lets us test the change works

Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this will change the timing of WORKFLOW_TASK_EXECUTION_TOTAL_LATENCY to include the time waiting for the run lock, but this metric is java specific and is undocumented, and arguably should include the run lock.

Concur

@Quinn-With-Two-Ns Quinn-With-Two-Ns merged commit 717ee05 into temporalio:master Sep 26, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UnableToAcquireLockException exception causes slots to not get freed
2 participants