Skip to content
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

[do not review] WIP: graceful plugin shutdown #13760

Closed
wants to merge 35 commits into from

Conversation

abhinav
Copy link
Contributor

@abhinav abhinav commented Aug 23, 2023

Do not review yet

This is just to experiment with graceful shutdown of plugins.

@abhinav abhinav marked this pull request as draft August 23, 2023 17:37
@pulumi-bot
Copy link
Contributor

pulumi-bot commented Aug 23, 2023

Changelog

[uncommitted] (2023-08-25)

@abhinav abhinav added ci/test Test CI pipelines on this PR impact/no-changelog-required This issue doesn't require a CHANGELOG update do-not-merge labels Aug 23, 2023
@abhinav abhinav force-pushed the abhinav/graceful-shutdown branch 5 times, most recently from 411c924 to 10d08ff Compare August 23, 2023 18:21
@abhinav abhinav removed the ci/test Test CI pipelines on this PR label Aug 23, 2023
@abhinav abhinav force-pushed the abhinav/graceful-shutdown branch 11 times, most recently from 6841e19 to 2a2ff3f Compare August 25, 2023 16:32
@abhinav
Copy link
Contributor Author

abhinav commented Aug 25, 2023

Excellent! CREATE_NEW_PROCESS_GROUP propagates signals to child processes similar to kill(-pid, sig) on Unix systems.

@abhinav abhinav force-pushed the abhinav/graceful-shutdown branch 3 times, most recently from 250afee to 29d15e9 Compare August 25, 2023 19:46
@abhinav
Copy link
Contributor Author

abhinav commented Aug 25, 2023

GREEN! Excellent, I'll pull this stuff into a real PR now.

abhinav added a commit that referenced this pull request Aug 25, 2023
Adds a new **currently unused** function TerminateProcessGroup
that terminates all processes in a group gracefully.

It does so by first sending the process
a SIGINT on Unix systems, and CTRL_BREAK_EVENT on Windows,
and waiting a specified duration for the process to exit.
The choice of signals was very deliberate
and is documented in the comments for TerminateProcessGroup.

If the process does not exit in the given duration,
it and its child processes are forcibly terminated
with SIGKILL or equivalent.

Testing:
The core behaviors are tested against Python, Go, and Node.
The corner cases of signal handling are tested
against rogue Go processes.
The changes were experimented with in #13760.

Refs #9780
abhinav added a commit that referenced this pull request Aug 26, 2023
Adds a new **currently unused** function TerminateProcessGroup
that terminates all processes in a group gracefully.

It does so by first sending the process
a SIGINT on Unix systems, and CTRL_BREAK_EVENT on Windows,
and waiting a specified duration for the process to exit.
The choice of signals was very deliberate
and is documented in the comments for TerminateProcessGroup.

If the process does not exit in the given duration,
it and its child processes are forcibly terminated
with SIGKILL or equivalent.

Testing:
The core behaviors are tested against Python, Go, and Node.
The corner cases of signal handling are tested
against rogue Go processes.
The changes were experimented with in #13760.

Refs #9780
@abhinav
Copy link
Contributor Author

abhinav commented Aug 26, 2023

This PR has served its purpose.
Closing in favor of #13792 and #13795.

@abhinav abhinav closed this Aug 26, 2023
@abhinav abhinav deleted the abhinav/graceful-shutdown branch August 26, 2023 00:17
github-merge-queue bot pushed a commit that referenced this pull request Aug 27, 2023
Adds a new **currently unused** function TerminateProcessGroup
that terminates all processes in a group gracefully.

It does so by first sending the process
a SIGINT on Unix systems, and CTRL_BREAK_EVENT on Windows,
and waiting a specified duration for the process to exit.
The choice of signals was very deliberate
and is documented in the comments for TerminateProcessGroup.

If the process does not exit in the given duration,
it and its child processes are forcibly terminated
with SIGKILL or equivalent.

Testing:
The core behaviors are tested against Python, Go, and Node.
The corner cases of signal handling are tested
against rogue Go processes.
The changes were experimented with in #13760.

Refs #9780
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge impact/no-changelog-required This issue doesn't require a CHANGELOG update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants