> On March 15, 2016, 11:21 p.m., Ben Mahler wrote:
> > src/docker/executor.cpp, lines 235-239
> > <https://reviews.apache.org/r/44660/diff/4/?file=1299829#file1299829line235>
> >
> > Ditto earlier comments. I would expect the buffer to be in addition to
> > the reap interval. It also wasn't obvious to me that docker->run uses the
> > reaper internally, so perhaps a note here about that.
> >
> > At some point we'll make reaping children be non-polling :)
>
> Alexander Rukletsov wrote:
> I'm a bit hesitant to add the comment here about how `docker->run()`
> works. Such comments tend to become outdated when the internal implementation
> changes.
The point I was trying to make was that you essentially already have this
comment in the code itself:
```
Option<Duration> gracePeriod = min(
shutdownGracePeriod - process::MAX_REAP_INTERVAL(),
killPolicyGracePeriod);
```
We're using the reap interval, so that means we're assuming docker->run
involves the reap interval already, the comment was just to reflect this
assumption :)
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44660/#review123782
-----------------------------------------------------------
On March 18, 2016, 5:21 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44660/
> -----------------------------------------------------------
>
> (Updated March 18, 2016, 5:21 p.m.)
>
>
> Review request for mesos, Ben Mahler, Gilbert Song, and Timothy Chen.
>
>
> Bugs: MESOS-4909
> https://issues.apache.org/jira/browse/MESOS-4909
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The docker executor determines how much time it allots the
> underlying container to clean up (via passing the timeout to
> the docker daemon) based on both optional task's `KillPolicy`
> and optional `shutdown_grace_period` field in `ExecutorInfo`.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b
> include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b
> src/docker/executor.cpp afc769d0887e3842106e4c350e94c95c8ffc085e
>
> Diff: https://reviews.apache.org/r/44660/diff/
>
>
> Testing
> -------
>
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
