When evicting workflows from cache make sure all go routines are evicted serially #1718
Conversation
| } | ||
| // We need to make sure the coroutine is closed, otherwise we risk concurrent coroutines running | ||
| // at the same time causing a race condition. | ||
| <-s.aboutToBlock |
There was a problem hiding this comment.
How does this accomplish the comment above? If we receive a signal from aboutToBlock we exit cleanly, and otherwise if the timer fire first, why do we send a signal to aboutToBlock when the timer expires?
Is it just to trigger the next coroutine exiting that it can exit cleanly?
There was a problem hiding this comment.
We don't send a signal to aboutToBlock, we wait on receiving a signal from aboutToBlock, which is sent on exit
There was a problem hiding this comment.
oops, misread this statement as sending a signal.. thanks for clarifying. So this function's behavior is the same, with an added log statement on timeout.
I'm a little confused by the PR title, this seems more like adding logs to help identify if evicted co-routines are timing out/exiting in the same order, but there is no code to actually make sure this is happening?
There was a problem hiding this comment.
It behaves a bit differently because previously we didn't have this second <-s.aboutToBlock to if the coroutine didn't exit fast enough we would end up evicting multiple coroutines in parallel. Now we always wait for <-s.aboutToBlock to fire before we continue
There was a problem hiding this comment.
You're right.. Maybe I need more coffee.. thanks for clarifying!
Quinn-With-Two-Ns
force-pushed
the
SDK-3056
branch
from
1d73f5d to
33fa708
Compare
Quinn-With-Two-Ns
force-pushed
the
SDK-3056
branch
from
33fa708 to
1da9d5e
Compare
When evicting workflows from cache make sure all go routines are evicted serially.