----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22852/#review46414 -----------------------------------------------------------
Now that we've got the Subcommand to execute the executor I think we can move code out and simplify the Launchers and MesosContainerizer. - setsid - cgroups::assign for freezer - checkpointing the pid Pushing these down would, I think, let us remove the two cases of using pipes to synchronize with the slave. Details as issues. src/slave/containerizer/launcher.cpp <https://reviews.apache.org/r/22852/#comment81798> Since this is common between the launchers, what about moving this into the subcommand? src/slave/containerizer/linux_launcher.cpp <https://reviews.apache.org/r/22852/#comment81799> 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. src/slave/containerizer/mesos_containerizer.cpp <https://reviews.apache.org/r/22852/#comment81801> Could we also push checkpointing the pid file into the subcommand as an option too? src/slave/containerizer/mesos_containerizer.cpp <https://reviews.apache.org/r/22852/#comment81802> 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? src/slave/containerizer/mesos_containerizer.cpp <https://reviews.apache.org/r/22852/#comment81803> This formatting seems strange? src/slave/containerizer/mesos_containerizer.cpp <https://reviews.apache.org/r/22852/#comment81804> Kill extra newline. src/slave/containerizer/mesos_containerizer.cpp <https://reviews.apache.org/r/22852/#comment81805> Kill extra whitespace: s/s ./s./ - Ian Downes On June 20, 2014, 9:57 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22852/ > ----------------------------------------------------------- > > (Updated June 20, 2014, 9:57 p.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 > >
