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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multiple node shutdown fixes #1563
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.
Mostly looking good. Ran this locally and it reduced the shutdown time with plugins a lot. Your reasoning why this fixes the rare assertion failure in CI seems sound to me.
This PR needs two changelog entries:
bug-fixes
for the rare assertion failure fixfeatures
for the parallel shutdown of auxiliary actors
- We've been terminating non-essential components after the main ones, but now we do it before. - We don't try to terminate remote actors any more. - The shutdown sequence used to treat labels as types, which means the actor of the label wouldn't be terminated at all and keeping the process alive.
6eae71a
to
597385f
Compare
59cbb94
to
df7e4c3
Compare
-> done.
I doubt this will really be noticeable under normal circumstances. That change was mostly done to speed things up in case on of the components is not responsive. |
The shutdown logic contained a bug that would make the node fail to terminate | ||
in case a plugin actor is registered at said node. | ||
|
||
A race condition in the shutdown logic that caused an assertion was fixed. |
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.
Every paragraph is a changelog entry of its own, I doubt this is what you wanted to achieve 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.
These are 2 separate Bugs that are both fixed in this PR.
馃摂 Description
ones, but now we do it before.
the actor of the label wouldn't be terminated at all and keeping
the process alive.
馃摑 Checklist
馃幆 Review Instructions
By commit.