mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mesos ReviewBot" <revi...@mesos.apache.org>
Subject Re: Review Request 41285: Made Executor struct assign `pid/http` to be None() explicitly.
Date Wed, 20 Jan 2016 01:43:11 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41285/#review115311
-----------------------------------------------------------


Patch looks great!

Reviews applied: [41285]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc;
./support/docker_build.sh

- Mesos ReviewBot


On Jan. 20, 2016, 1:41 a.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41285/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2016, 1:41 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3550
>     https://issues.apache.org/jira/browse/MESOS-3550
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Currently, most of our logic for finding the executor type is based on the fields pid/http
in the Executor struct. We were erroneously setting the initial pid value to be UPID() instead
of being None().
> 
> The value of pid being UPID() is only possible in this scenario:
> 	 	
> We were unable to find the type of executor upon agent recovery i.e. no pid/http marker
file was found. We then mark this special case as pid=UPID().
>  	
> This special case helps us while recovery in the following ways:
> 	 		 	
> - If the agent crashed before checkpointing the pid file, both executor->pid and executor->http
would be None(). This is similar to the case for HTTP based executors (pid/http being None).
In order, to distinguish between these two cases, we set the pid=UPID(). This helps us in
not shutting the HTTP executor accidently by destroying the container when the agent is recovered
using recovery=cleanup
> - This special value also helps us to do better logging by correctly distinguishing between
HTTP based executors and agent dying before checkpointing the pid/http marker file. ( Both
have pid/http as None)
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 
> 
> Diff: https://reviews.apache.org/r/41285/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


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