-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-1394] Remove SIGCHLD handler in worker subprocess #1247
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
Conversation
It should not be the responsibility of the worker subprocess, which does not intentionally fork, to try and cleanup child processes. Doing so is complex and interferes with operations such as platform.system(). If it is desirable to have tighter control over subprocesses, then namespaces should be used and it should be the manager's resposibility to handle cleanup.
|
Can one of the admins verify this patch? |
|
Jekins, test this please. |
|
Merged build triggered. |
|
Merged build started. |
|
Just to make sure I understand the problem, is the issue that the spawned python worker process inherits the SIGCHLD handler of its parent process, which it has no need for? And that libraries which spawn subprocesses can invoke this SIGCHLD handler unexpectedly? |
|
that's more or less it, except a level lower. the spawned python worker processes spawn workers of their own (one per connect) and those inherit the worker's SIGCHLD handler, which they have no need for. |
|
Merged build finished. All automated tests passed. |
|
All automated tests passed. |
|
Gotcha. This sounds like a straightforward fix for a behavior that was totally unintended and unwanted. Merging into master and branch-1.0. |
It should not be the responsibility of the worker subprocess, which does not intentionally fork, to try and cleanup child processes. Doing so is complex and interferes with operations such as platform.system(). If it is desirable to have tighter control over subprocesses, then namespaces should be used and it should be the manager's resposibility to handle cleanup. Author: Matthew Farrellee <[email protected]> Closes #1247 from mattf/SPARK-1394 and squashes the following commits: c36f308 [Matthew Farrellee] [SPARK-1394] Remove SIGCHLD handler in worker subprocess (cherry picked from commit 3c104c7) Signed-off-by: Aaron Davidson <[email protected]>
It should not be the responsibility of the worker subprocess, which does not intentionally fork, to try and cleanup child processes. Doing so is complex and interferes with operations such as platform.system(). If it is desirable to have tighter control over subprocesses, then namespaces should be used and it should be the manager's resposibility to handle cleanup. Author: Matthew Farrellee <[email protected]> Closes apache#1247 from mattf/SPARK-1394 and squashes the following commits: c36f308 [Matthew Farrellee] [SPARK-1394] Remove SIGCHLD handler in worker subprocess
It should not be the responsibility of the worker subprocess, which
does not intentionally fork, to try and cleanup child processes. Doing
so is complex and interferes with operations such as
platform.system().
If it is desirable to have tighter control over subprocesses, then
namespaces should be used and it should be the manager's resposibility
to handle cleanup.