-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
cluster.rb - no zombies #1887
cluster.rb - no zombies #1887
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -227,9 +227,11 @@ def check_workers(force=false) | |
# during this loop by giving the kernel time to kill them. | ||
sleep 1 if any | ||
|
||
@workers.reject! { |w| Process.waitpid(w.pid, Process::WNOHANG) } | ||
|
||
@workers.reject!(&:dead?) | ||
pids = [] | ||
while pid = Process.waitpid(-1, Process::WNOHANG) do | ||
pids << pid | ||
end | ||
@workers.reject! { |w| w.dead? || pids.include?(w.pid) } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could move these lines into a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good suggestion, I'll leave coding style up to others. It's four lines, probably only called by the method it's contained in, and any test would involve that method... |
||
|
||
cull_workers | ||
spawn_workers | ||
|
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.
In the future @MSP-Greg, I'd prefer you make changes to travis.yml in separate PRs. That way, they won't get blocked by the corresponding feature/bugfix.
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.
I was thinking the same.
While I was testing/debugging, Travis seemed to be acting up, and I wasn't sure if it was affecting the CI results I was seeing.. IOW, didn't just decide to throw it in, I was having failures...
How about I remove it right now & add another PR?