----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44660/#review124353 -----------------------------------------------------------
Sorry I forgot to publish some stale comments from before, these are partial and may be out-of-date but figured you may find them useful. include/mesos/mesos.proto (line 360) <https://reviews.apache.org/r/44660/#comment186851> For docker executor-less tasks? include/mesos/mesos.proto (line 361) <https://reviews.apache.org/r/44660/#comment186847> To be more helpful s/-t/--time/ include/mesos/mesos.proto (line 1247) <https://reviews.apache.org/r/44660/#comment186852> ditto here, the user doesn't know about the docker executor include/mesos/v1/mesos.proto (lines 358 - 1244) <https://reviews.apache.org/r/44660/#comment186853> Ditto here. include/mesos/v1/mesos.proto (line 361) <https://reviews.apache.org/r/44660/#comment186848> To be more helpful s/-t/--time/ src/docker/executor.cpp (lines 216 - 242) <https://reviews.apache.org/r/44660/#comment186859> It's not immediately clear to me why the approach taken here is different from the approach taken in the command executor: In the command executor, the kill policy overrides the shutdown grace period. In the docker executor, we still take the minimum of the kill policy and the shutdown grace period. After thinking about it, I assume it's because you want to respect the --docker_stop_timeout flag when it's set to a smaller value than the kill policy? If so, please document why we have the minimum logic, because in the command executor there is an assumption that we never need to take a minimum of the shutdown grace period and the kill policy. src/docker/executor.cpp (line 222) <https://reviews.apache.org/r/44660/#comment186856> Let's say --time to be more clear than -t src/docker/executor.cpp (lines 236 - 239) <https://reviews.apache.org/r/44660/#comment186861> I added this TODO for killTask, can you help by clarifying that the shutdown arrives after a kill task? src/docker/executor.cpp (line 630) <https://reviews.apache.org/r/44660/#comment186854> It doesn't crash it just exits :) src/docker/executor.cpp (line 631) <https://reviews.apache.org/r/44660/#comment186855> removing flags is also difficult :) - Ben Mahler On March 23, 2016, 11:25 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44660/ > ----------------------------------------------------------- > > (Updated March 23, 2016, 11:25 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 b965f5a6b1aff28feb2ab088c30ea927b9247403 > include/mesos/v1/mesos.proto d2ab6ed187ec0d0e5dbac92607b612bb55b1a682 > 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 > >
