Add option for launching in own process group #155
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Apologies for the time taken to review this one. Do you think it's worth defaulting this behaviour rather than adding as config @mattjohnsonpint? I can't see a reason we wouldn't always want this. |
|
Yeah, that would make sense to me. |
|
It's been a while since I made this, but IIRC I was thinking that there must be some use case for the current behavior, and I didn't want to add such a drastic change without it being an option - in case it messed things up for others. |
|
I don't recall a specific reason for setting it up this way! If you're happy to switch it around and the tests pass let's do it 👍 |
|
Cool. I'll send a revision soon. 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In order to handle shutdown cleanly in an app, it's important that the application starting the db instance with
Startis also the one the callsStopto shut it down. In the case where an app is running until the user terminates it with a Ctrl-C, the app needs to be able to trap theSIGINTand then callStop.Unfortunately, there's some platform differences in how Ctrl-C is propagated to child processes between platforms. On Linux/Mac, the interrupt request is passed only to the parent process, but on Windows it is passed to ALL child processes in the same process group as well. So by the time the parent app goes to call
Stop- the server is already terminated.This PR adds a config flag,
OwnProcessGroupwhich when passedtruewill make sure that the exec commands that callpg_ctl(and in turn, the postgres server itself) are launched in their own process group. This will change the Ctrl-C behavior on Windows to behave the same as on Linux/Mac.For good measure, I also set the appropriate flag on Linux/Mac so the behavior is truly consistent in all regards. It's
SysProcAttr.Setpgid = trueon Linux/Mac, andSysProcAttr.CreationFlags = syscall.CREATE_NEW_PROCESS_GROUPon Windows.