Return-Path: 
 <dev-return-21236-apmail-mesos-dev-archive=mesos.apache.org@mesos.apache.org>
X-Original-To: apmail-mesos-dev-archive@www.apache.org
Delivered-To: apmail-mesos-dev-archive@www.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id 5ADE41029A
	for <apmail-mesos-dev-archive@www.apache.org>;
 Thu, 20 Mar 2014 23:12:37 +0000 (UTC)
Received: (qmail 96672 invoked by uid 500); 20 Mar 2014 23:12:36 -0000
Delivered-To: apmail-mesos-dev-archive@mesos.apache.org
Received: (qmail 96578 invoked by uid 500); 20 Mar 2014 23:12:36 -0000
Mailing-List: contact dev-help@mesos.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:dev-help@mesos.apache.org>
List-Unsubscribe: <mailto:dev-unsubscribe@mesos.apache.org>
List-Post: <mailto:dev@mesos.apache.org>
List-Id: <dev.mesos.apache.org>
Reply-To: dev@mesos.apache.org
Delivered-To: mailing list dev@mesos.apache.org
Received: (qmail 96565 invoked by uid 99); 20 Mar 2014 23:12:36 -0000
Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40)
    by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 20 Mar 2014 23:12:36 +0000
Received: from reviews.apache.org (localhost [127.0.0.1])
	by reviews.apache.org (Postfix) with ESMTP id 798B61D5559;
	Thu, 20 Mar 2014 23:12:33 +0000 (UTC)
Content-Type: multipart/alternative;
 boundary="===============3655438544984452252=="
MIME-Version: 1.0
Subject: Re: Review Request 19239: Added signal escalation to command
 executor.
From: "Ben Mahler" <benjamin.mahler@gmail.com>
To: "Ben Mahler" <benjamin.mahler@gmail.com>,
 "Ian Downes" <ian.downes@gmail.com>
Cc: "Niklas Nielsen" <nik@qni.dk>, "mesos" <dev@mesos.apache.org>
Date: Thu, 20 Mar 2014 23:12:33 -0000
Message-ID: <20140320231233.10646.66201@reviews.apache.org>
X-ReviewBoard-URL: https://reviews.apache.org
Auto-Submitted: auto-generated
Sender: "Ben Mahler" <noreply@reviews.apache.org>
X-ReviewGroup: mesos
X-ReviewRequest-URL: https://reviews.apache.org/r/19239/
X-Sender: "Ben Mahler" <noreply@reviews.apache.org>
References: <20140314221655.30484.73230@reviews.apache.org>
In-Reply-To: <20140314221655.30484.73230@reviews.apache.org>
Reply-To: "Ben Mahler" <benjamin.mahler@gmail.com>
X-ReviewRequest-Repository: mesos-git

--===============3655438544984452252==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit


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


--===============3655438544984452252==--