----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16149/#review30240 -----------------------------------------------------------
Benh and I went over this together. Who owns the process termination, the launcher? Is this implemented? src/slave/container/launcher.hpp <https://reviews.apache.org/r/16149/#comment57900> This seems like part of the containerizer since it delivers these to the slave, could this be Containerizer::Termination? src/slave/container/launcher.hpp <https://reviews.apache.org/r/16149/#comment57901> End comments with a period here and elsewhere. src/slave/container/launcher.hpp <https://reviews.apache.org/r/16149/#comment57899> Place the Launcher API first in this file. src/slave/container/launcher.hpp <https://reviews.apache.org/r/16149/#comment57902> Documentation please :) src/slave/container/launcher.cpp <https://reviews.apache.org/r/16149/#comment57903> We don't use snake case here, is this coming from the flags? src/slave/container/launcher.cpp <https://reviews.apache.org/r/16149/#comment57904> There doesn't seem to be a need for this to be asynchronous, yet. Keeping it simple for now would be nice. src/slave/container/launcher.cpp <https://reviews.apache.org/r/16149/#comment57905> Please use Failure() here and elsewhere. src/slave/container/mesos_launcher.hpp <https://reviews.apache.org/r/16149/#comment57906> Why is there a MesosLauncher? What will the inheritance here be used for? The Forker hierarchy seems to be sufficient for capturing the different behavior. src/slave/container/mesos_launcher.hpp <https://reviews.apache.org/r/16149/#comment57907> Place the { on a newline. Here and elsewhere. src/slave/container/mesos_launcher.cpp <https://reviews.apache.org/r/16149/#comment57909> What if it is launching but not yet running? src/slave/container/mesos_launcher.cpp <https://reviews.apache.org/r/16149/#comment57910> Is there going to be a cgroups version of this termination? src/slave/container/mesos_launcher.cpp <https://reviews.apache.org/r/16149/#comment57911> Is this necessary given you've called killtree with groups=true? - Ben Mahler On Dec. 11, 2013, 4:06 a.m., Ian Downes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16149/ > ----------------------------------------------------------- > > (Updated Dec. 11, 2013, 4:06 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas > Nielsen, samya, and Jason Dusek. > > > Repository: mesos-git > > > Description > ------- > > Launcher interface and MesosLauncher to support MesosContainerizers. > > Launchers handle the lifecycle of the executor process (and descendants). > > > Diffs > ----- > > src/slave/container/launcher.hpp PRE-CREATION > src/slave/container/launcher.cpp PRE-CREATION > src/slave/container/mesos_launcher.hpp PRE-CREATION > src/slave/container/mesos_launcher.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/16149/diff/ > > > Testing > ------- > > > Thanks, > > Ian Downes > >
