> On Sept. 25, 2016, 10:32 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/linux_launcher.cpp, line 614 > > <https://reviews.apache.org/r/51278/diff/3/?file=1509809#file1509809line614> > > > > using executor_pid here is weird. I think we should rethink how we > > should `ContainerStatus` when we have nested containers. Should we rename > > it to `pid`? Sounds like 'executor_pid' is not very precise. > > > > Another to consider is: in the future, user might want to get the pid > > of it's actual process, not the 'init' pid we forked. What should we call > > that?
I'm going to drop this for now, let's iterate on this together in a subsequent review. > On Sept. 25, 2016, 10:32 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/linux_launcher.cpp, lines 565-571 > > <https://reviews.apache.org/r/51278/diff/3/?file=1509809#file1509809line565> > > > > I won't worry about this case because the containerizer will make sure > > cleanup/destroy is only called once for launcher/isolator/provisioner. > > > > But I like the fact we remove the container from `containers` map so > > that other calls to launcher will be properly handled (e.g., status). I improved the comment but still mentioned this because we're missing proper separation of concerns by assuming that we know how the caller is calling this function. I didn't write that code, someone else did, I'm just exposing this functionality through an interface! - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51278/#review150361 ----------------------------------------------------------- On Sept. 25, 2016, 4:08 p.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51278/ > ----------------------------------------------------------- > > (Updated Sept. 25, 2016, 4:08 p.m.) > > > Review request for mesos, Gilbert Song, Jie Yu, and Kevin Klues. > > > Repository: mesos > > > Description > ------- > > Refactored LinuxLauncher to be nested container aware. > > > Diffs > ----- > > src/slave/containerizer/mesos/launch.hpp > 859990cb85e9e8c06400397256cfc512f0811800 > src/slave/containerizer/mesos/linux_launcher.hpp > 2cd1e4d4b6d9e656c2447a9d3ab8e1f49ba5fffa > src/slave/containerizer/mesos/linux_launcher.cpp > f27757032cc28be0f0067bed045536431dde4d6c > > Diff: https://reviews.apache.org/r/51278/diff/ > > > Testing > ------- > > > Thanks, > > Benjamin Hindman > >
