-
Notifications
You must be signed in to change notification settings - Fork 143
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
Add StickyTaskQueueDrainTimeout #2019
Add StickyTaskQueueDrainTimeout #2019
Conversation
Add an option to drain sticky task queue during graceful shutdown.
7ad2f28
to
14b9861
Compare
@@ -77,6 +77,23 @@ public CompletableFuture<Void> waitForSemaphorePermitsReleaseUntimed( | |||
return future; | |||
} | |||
|
|||
/** | |||
* waitForStickyQueueBalancer -> disableNormalPoll -> timed wait for graceful completion of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fucking Javadoc lol
temporal-sdk/src/main/java/io/temporal/worker/WorkerOptions.java
Outdated
Show resolved
Hide resolved
* shutting down the worker. If set the worker will stop making new poll requests on the normal | ||
* task queue, but will continue to poll the sticky task queue until the timeout is reached. | ||
* This value should always be greater than clients rpc long poll timeout, which can be set via | ||
* {@link WorkflowServiceStubsOptions.Builder#setRpcLongPollTimeout(Duration)}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I wonder if it'd make more sense to make this a boolean
for waitForStickyEmptyPoll
, because what if you have a time race? For instance, what if my long poll timeout and my sticky drain timeout is 1m. And as soon as the poll comes back empty I haven't hit sticky drain timeout exactly yet, so I make another sticky poll for just some nanoseconds before sticky drain timeout does hit? Maybe that's ok though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That poll would be cancelled, there is a possible race, but the race also exists for normal shutdown as well and I don't think we should tackle it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Would like to see us not kill poll calls anywhere and always wait for graceful completion, but yes can be tackled elsewhere. I wonder if it's worth an issue on that to ensure SDK behavior wrt poll interruption.
a747cc8
to
28d6529
Compare
Add an option to drain sticky task queue during graceful shutdown. Worker will stop polling on the normal task queue and only poll on the stick task queue to complete workflows while the worker is shutting down.
There is a lot of follow up work we could do around here based on user demand and if the server adds some APIs. For know this is the minimal implementation to avoid sticky workflow timeout for short workflow during graceful shutdown.