mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request 19239: Added signal escalation to command executor.
Date Thu, 20 Mar 2014 23:12:33 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message