----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19239/#review38538 -----------------------------------------------------------
Hey Niklas looks good to me other than the new helper function. Thanks for adding the constant! src/launcher/executor.cpp <https://reviews.apache.org/r/19239/#comment70776> Let's call this 'escalated'? src/launcher/executor.cpp <https://reviews.apache.org/r/19239/#comment70777> I'm not sure this wrapper is simplifying anything, since it looks like you're ignoring the result and all it's doing is wrapping os::killtree? One has to know that 'killAll' is just an alias for 'killtree(_, _, true, true)' without any additional functionality. It also doesn't look like this reduced the amount of code, and it added more indirection. I preferred your 2nd patch without this helper, can we go back to that? :) - Ben Mahler On March 25, 2014, 6:56 p.m., Niklas Nielsen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19239/ > ----------------------------------------------------------- > > (Updated March 25, 2014, 6:56 p.m.) > > > Review request for mesos, Ben Mahler and Ian Downes. > > > Bugs: MESOS-1031 > https://issues.apache.org/jira/browse/MESOS-1031 > > > Repository: mesos-git > > > Description > ------- > > This patch adds a two step signal escalation: 1) SIGTERM is sent to > the process tree 2) If the process tree root hasn't been reaped within > escalationTimeout (3 seconds), SIGKILL is sent to the process tree. > Currently, this may leave behind orphan processes if parent processes > terminates and leave non-responsive processes behind. This will be > captured when PID namespaces are in place, as the orphan processes > will end up hanging off the process tree root instead of init. > > > Diffs > ----- > > src/launcher/executor.cpp e30d77a > src/slave/constants.hpp d237383 > src/slave/constants.cpp 3fb0cde > > Diff: https://reviews.apache.org/r/19239/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Niklas Nielsen > >
