-
Notifications
You must be signed in to change notification settings - Fork 68
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
Drain buffered polls on shutdown #161
Drain buffered polls on shutdown #161
Conversation
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.
Approved with some minor comments
src/core_tests/workflow_tasks.rs
Outdated
let poll_fut = async move { | ||
// Now poll again, which will start spinning, and buffer the next WFT with timer fired in it | ||
// - it won't stop spinning until the first task is complete | ||
let t = dbg!(core.poll_workflow_task(TEST_Q).await.unwrap()); |
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.
Note that this is only required today because core doesn't poll without an outstanding poll request from lang.
}; | ||
tokio::join!(poll_fut, complete_first, async { | ||
// If the shutdown is sent too too fast, we might not have got a chance to even buffer work | ||
tokio::time::sleep(Duration::from_millis(5)).await; |
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.
You know my opinion on time sensitive tests like this.
We should have a better way of timing the shutdown.
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.
Opened #162
# Conflicts: # src/workflow/workflow_tasks/concurrency_manager.rs
9d15485
to
1aa18f8
Compare
* Fixes resultant hang in test * A whole bunch of refactoring to expose worker options in test code to test that
bb1ad3a
to
2c83e6b
Compare
What was changed
Buffered polls are now drained before poll returns shutdown
Why?
Avoid pointlessly timing out workflow tasks the worker has claimed while shutting down
Checklist
Closes Drain buffered server responses on shutdown #160
How was this tested:
New test
Any docs updates needed?