----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16149/#review33012 -----------------------------------------------------------
src/slave/containerizer/cgroups_launcher.cpp <https://reviews.apache.org/r/16149/#comment62140> s/subsystem/subsystem " << subsystem <</ src/slave/containerizer/cgroups_launcher.cpp <https://reviews.apache.org/r/16149/#comment62142> Why is this an error? If the cgroup was removed but the slave died before it got the signal from the containerizer, wouldn't we be here? I think the right thing to do here is to just skip it. src/slave/containerizer/cgroups_launcher.cpp <https://reviews.apache.org/r/16149/#comment62151> s/fork process/create freezer cgroup/ ? also please include exists.error() in the Error(). src/slave/containerizer/cgroups_launcher.cpp <https://reviews.apache.org/r/16149/#comment62152> How is it possible that a cgroup for this container already exists? If it does, shouldn't we return an error? src/slave/containerizer/cgroups_launcher.cpp <https://reviews.apache.org/r/16149/#comment62154> s/it's/its/ src/slave/containerizer/cgroups_launcher.cpp <https://reviews.apache.org/r/16149/#comment62162> The older semantics was to also do a setsid() here. Why the regression? src/slave/containerizer/cgroups_launcher.cpp <https://reviews.apache.org/r/16149/#comment62155> Include destroyed.failure() in Failure() please. src/slave/containerizer/launcher.cpp <https://reviews.apache.org/r/16149/#comment62156> ditto. see the comment in cgroupslauncher::recover(). src/slave/containerizer/launcher.cpp <https://reviews.apache.org/r/16149/#comment62157> How is this possible? If there is pid wrapping? If yes, please add a comment. src/slave/containerizer/launcher.cpp <https://reviews.apache.org/r/16149/#comment62158> s/Addition/Additional/ - Vinod Kone On Jan. 27, 2014, 3:02 a.m., Ian Downes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16149/ > ----------------------------------------------------------- > > (Updated Jan. 27, 2014, 3:02 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/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454 > src/slave/containerizer/cgroups_launcher.hpp PRE-CREATION > src/slave/containerizer/cgroups_launcher.cpp PRE-CREATION > src/slave/containerizer/launcher.hpp PRE-CREATION > src/slave/containerizer/launcher.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/16149/diff/ > > > Testing > ------- > > > Thanks, > > Ian Downes > >
