> On June 23, 2014, 11:02 a.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? > > Jie Yu wrote: > Added a TODO. Again, let's do the refactor in a new patch since this > patch is already quite complicated. > > Jie Yu wrote: > Sorry, for this, I don't understand why the checkpointing should be done > by the subprocess, not the slave?
We need to correctly handle the slave dying soon after forking the child, i.e., if the child process is running we must have the pid checkpointed so we know about it when we restart. The current code uses a pipe to synchronize. The child only execs after checkpointing is successful; if the slave exits the pipe is closed and the child exits. If we have the child checkpoint itself then we eliminate all of this. > On June 23, 2014, 11:02 a.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? > > Jie Yu wrote: > I am not able to get your first part? Can you elaborate? We're specifying command/cwd/user but inheriting the environment. I think making the environment explicit is preferred. - Ian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22852/#review46414 ----------------------------------------------------------- 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 > >
