Fix VTOrc holding locks after shutdown#11442
Conversation
…rm hooks and add tests for it Signed-off-by: Manan Gupta <manan@planetscale.com>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
If a new flag is being introduced:
If a workflow is added or modified:
Bug fixes
Non-trivial changes
New/Existing features
Backward compatibility
|
|
I think this is good practice anyway, but I do wonder if we have a bug in the lock code where we are not checking for a canceled context. |
deepthi
left a comment
There was a problem hiding this comment.
IIUC, we already had the code for this, we just needed to register it properly with servenv, correct?
Signed-off-by: Manan Gupta <manan@planetscale.com>
|
@deepthi Yes, we had code for this already. The thing is that when we started using Now, we register this function as a on-termination hook to be run, so SIGTERM is only caught by servenv, but as part of the closing of the servenv, this function is called and this blocks until all the locks are released. |
* feat: call the logic for waiting for shard locks on synchronous on-term hooks and add tests for it Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix comment for acceptSighupSignal function Signed-off-by: Manan Gupta <manan@planetscale.com> Signed-off-by: Manan Gupta <manan@planetscale.com> Co-authored-by: Manan Gupta <manan@planetscale.com>
Description
This PR fixes the issue described in #11441. On further investigation it was found that this problem was introduced when we started using the
servenvpackage in VTOrc.The proposed fix is to run the code that wait for the shards to be let go as part of the synchronous on termination hooks.
Unit tests validating the wait function have also been added.
Related Issue(s)
Checklist
Deployment Notes