> On Oct. 15, 2014, 12:15 a.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 472
> > <https://reviews.apache.org/r/26709/diff/1/?file=721122#file721122line472>
> >
> > I'm not sure what executor timeout you're referring to, but for example
> > the executor signal escalation timeout although similiar is not really
> > intended for docker containerizer shutdown. In all our containerizers we
> > simply kill (SIGKILL) the tasks.
> >
> > Let's rename the method and also make the timeout be a method param
> > that we can pass in.
>
> Ryan Thomas wrote:
> Ok will do. I think we do need some sanity checking on the param though,
> maybe we could just set a hard upper bounds on the value - 60 seconds?
Yes, I basically had this in mind (mind the TODO)
```
string cmd = path + " kill " + container;
if(os::hasenv("MESOS_DOCKER_STOP_TIMEOUT")) {
const string timeout = os::getenv("MESOS_DOCKER_STOP_TIMEOUT", false);
// check if it is there and is numeric
if(string != NULL && std::regex_match(timeout, std::regex("[0-9]+"))) {
// it is numeric
// TODO: Verify the value us something sane
cmd = path + " stop -t " + timeout + " " + container;
}
}
```
I definitely agree with Timothy's comment to rename methods and such. I
(personal opinion) wouldn't want to put in any hard coded values - but would
definitely support 60 secs as a good default value for a timeout.
- Ankur
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26709/#review56611
-----------------------------------------------------------
On Oct. 14, 2014, 9:56 p.m., Ryan Thomas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26709/
> -----------------------------------------------------------
>
> (Updated Oct. 14, 2014, 9:56 p.m.)
>
>
> Review request for mesos.
>
>
> Bugs: MESOS-1925
> https://issues.apache.org/jira/browse/MESOS-1925
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Change docker kill to docker stop to allow graceful shutdown of containers.
>
>
> Diffs
> -----
>
> src/docker/docker.cpp e09b51c4f5101c3a8e77caf12b208c88f47fbcb2
>
> Diff: https://reviews.apache.org/r/26709/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ryan Thomas
>
>
