> On Sept. 11, 2014, 3:45 p.m., Timothy St. Clair wrote:
> > src/exec/exec.cpp, line 82
> > <https://reviews.apache.org/r/25434/diff/2/?file=683945#file683945line82>
> >
> >     Maybe I'm missing something, but is there a reason we don't check 
> > before a delay?  If ShutdownProcess is spawned, it's possible all other 
> > processes in the group have already cleaned up.

Good questions. Can this be related to non-native bindings?


> On Sept. 11, 2014, 3:45 p.m., Timothy St. Clair wrote:
> > src/exec/exec.cpp, line 725
> > <https://reviews.apache.org/r/25434/diff/2/?file=683945#file683945line725>
> >
> >     Does it make more sense to provide a sane default  and log in the case 
> > of bad config.  Doesn't CHECK_SOME bail?

Agreed, will do so.


> On Sept. 11, 2014, 3:45 p.m., Timothy St. Clair wrote:
> > src/launcher/executor.cpp, line 628
> > <https://reviews.apache.org/r/25434/diff/2/?file=683946#file683946line628>
> >
> >     Doesn't the flags load export override for env vars that are on the 
> > commend line?  I would keep the original comment.  
> >     
> >     e.g. 
> >     
> >     cat /etc/mesos/mesos-slave-env.sh 
> >     export MESOS_containerizers=docker,mesos

I would rather separate them, but you are right, I'll change the order for 
clarity.


> On Sept. 11, 2014, 3:45 p.m., Timothy St. Clair wrote:
> > src/launcher/executor.cpp, line 679
> > <https://reviews.apache.org/r/25434/diff/2/?file=683946#file683946line679>
> >
> >     imho the shutdown timeout logic should be part of a utility function, 
> > vs. re-creating in different locations, with subtle differences.

Agreed.


> On Sept. 11, 2014, 3:45 p.m., Timothy St. Clair wrote:
> > src/tests/containerizer.cpp, line 113
> > <https://reviews.apache.org/r/25434/diff/2/?file=683955#file683955line113>
> >
> >     Manifest constant please.

Ok.


- Alexander


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


On Sept. 9, 2014, 12:54 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25434/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2014, 12:54 p.m.)
> 
> 
> Review request for mesos, Niklas Nielsen, Till Toenshoff, and Timothy St. 
> Clair.
> 
> 
> Bugs: MESOS-1571
>     https://issues.apache.org/jira/browse/MESOS-1571
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> The configurable slave's executor_shutdown_grace_period flag is propagated to 
> Executor and CommandExecutor through an environment variable. Shutdown 
> timeout in Executor and signal escalation timeout in CommandExecutor are now 
> dependent on this flag. Each nested timeout is somewhat shorter than the 
> parent one.
> 
> 
> Diffs
> -----
> 
>   src/exec/exec.cpp 36d1778 
>   src/launcher/executor.cpp 12ac14b 
>   src/slave/constants.hpp 9030871 
>   src/slave/constants.cpp e1da5c0 
>   src/slave/containerizer/containerizer.hpp 8a66412 
>   src/slave/containerizer/containerizer.cpp 0254679 
>   src/slave/containerizer/docker.cpp 0febbac 
>   src/slave/containerizer/external_containerizer.cpp efbc68f 
>   src/slave/containerizer/mesos/containerizer.cpp 9d08329 
>   src/slave/flags.hpp 21e0021 
>   src/tests/containerizer.cpp a17e1e0 
> 
> Diff: https://reviews.apache.org/r/25434/diff/
> 
> 
> Testing
> -------
> 
> make check (OS X 10.9.4; Ubuntu 14.04 amd64)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Reply via email to