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

[devUtils/procRunner] wait for proc to exit so we fallback to SIGKILL #20918

Merged
merged 1 commit into from
Jul 18, 2018

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Jul 18, 2018

When the ProcRunner tries to kill a process, but the process never exits, it looks like it's going to try and send a SIGKILL to avoid the Error: Proc "kibana" was stopped but never emitted either the "exit" or "error" event after 30000 ms errors we see in CI every once and a while, but since it's not waiting for the outcome promise it isn't ever reaching that bit of logic.

I think I wrote this originally, and I also think I assumed treeKillAsync() wouldn't resolve until all the processes had exited, but it resolves the promise as soon as it has sent the signal and doesn't care about the result of the process.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@tylersmalley
Copy link
Contributor

tylersmalley commented Jul 18, 2018

Not sure if it's something you want to address - but since we don't wait for the outcome, we don't know what the exit code is:

info [kibana] exited with null after an hour

@spalger spalger merged commit 5afd06b into elastic:master Jul 18, 2018
@spalger
Copy link
Contributor Author

spalger commented Jul 18, 2018

@tylersmalley when we kill the process with using a signal we get null back as the exit code. That log line is produced specifically by observing the proc.outcome$ so it should be shown if it could be known.

@spalger
Copy link
Contributor Author

spalger commented Jul 19, 2018

6.x/6.4: 1bce3b4

@spalger spalger deleted the fix/proc-runner-sigkill-fallback branch July 19, 2018 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants