----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19239/#review38010 -----------------------------------------------------------
src/launcher/executor.cpp <https://reviews.apache.org/r/19239/#comment69916> Can you pull out a constant instead of using Seconds(3) and move the comment related to the shutdown grace period to be beside the constant? We can put this in the slave constants for now. I'm a bit curious about the strategy here because as soon as we provide a kill escalation, people will want to tune the escalation time, possibly per framework or per executor. src/launcher/executor.cpp <https://reviews.apache.org/r/19239/#comment69911> How about we make the signaling explicit in the logging? E.g. "Sending SIGTERM to process tree..." And below: "Process <pid> did not terminate after <escalation_timeout>, sending SIGKILL to process tree at <pid>" src/launcher/executor.cpp <https://reviews.apache.org/r/19239/#comment69920> "off" init? We may want to mention that this could accidentally kill new processes, if the pids rotate around, right? - Ben Mahler On March 14, 2014, 10:16 p.m., Niklas Nielsen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19239/ > ----------------------------------------------------------- > > (Updated March 14, 2014, 10:16 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 > > Diff: https://reviews.apache.org/r/19239/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Niklas Nielsen > >
