> On June 23, 2014, 6:02 p.m., Ian Downes wrote: > > src/slave/containerizer/linux_launcher.cpp, line 291 > > <https://reviews.apache.org/r/22852/diff/2/?file=614738#file614738line291> > > > > Suggestion: move the cgroups::assign as an optional, linux specific > > flag to the subcommand and have the child do the assign. This would obviate > > the sync in the linux launcher and all of the childSetup if the setsid is > > moved too.
That's a pretty big change, can we do that in a new review as this patch is already quite big. I'll add a TODO here. > On June 23, 2014, 6:02 p.m., Ian Downes wrote: > > src/slave/containerizer/launcher.cpp, line 95 > > <https://reviews.apache.org/r/22852/diff/2/?file=614736#file614736line95> > > > > Since this is common between the launchers, what about moving this into > > the subcommand? See my comments below. I'll add a TODO. > On June 23, 2014, 6:02 p.m., Ian Downes wrote: > > src/slave/containerizer/mesos_containerizer.cpp, line 840 > > <https://reviews.apache.org/r/22852/diff/2/?file=614740#file614740line840> > > > > Could we also push checkpointing the pid file into the subcommand as an > > option too? Added a TODO. Again, let's do the refactor in a new patch since this patch is already quite complicated. > On June 23, 2014, 6:02 p.m., Ian Downes wrote: > > src/slave/containerizer/mesos_containerizer.cpp, line 907 > > <https://reviews.apache.org/r/22852/diff/2/?file=614740#file614740line907> > > > > Is it cleaner to pass in the environment variables as well since we're > > specifying the command, working directory, and user. > > > > In the future we should use a clean environment, can we add this as a > > TODO? I am not able to get your first part? Can you elaborate? - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22852/#review46414 ----------------------------------------------------------- On June 21, 2014, 4:57 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22852/ > ----------------------------------------------------------- > > (Updated June 21, 2014, 4:57 a.m.) > > > Review request for mesos, Benjamin Hindman, Ian Downes, and Vinod Kone. > > > Bugs: MESOS-1404 > https://issues.apache.org/jira/browse/MESOS-1404 > > > Repository: mesos-git > > > Description > ------- > > See summary. The main idea is to create a new binary called > 'mesos-containerizer-helper' which performs most of the logic of 'execute' in > the original code. > > Also, we leverage the new Subprocess interface to handle redirections. > > (BTW: I fixed a few 70 char width comments issue when I doing the refactor). > > > Diffs > ----- > > src/Makefile.am fd2c80f > src/slave/containerizer/launcher.hpp 63c0cb6 > src/slave/containerizer/launcher.cpp c43396a > src/slave/containerizer/linux_launcher.hpp 60b401f > src/slave/containerizer/linux_launcher.cpp 85c74f0 > src/slave/containerizer/mesos_containerizer.hpp 1f5908a > src/slave/containerizer/mesos_containerizer.cpp 61c0a8d > src/slave/containerizer/mesos_containerizer_helper.cpp PRE-CREATION > src/tests/isolator_tests.cpp 0bbec09 > > Diff: https://reviews.apache.org/r/22852/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
