----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56362/#review164575 -----------------------------------------------------------
src/slave/containerizer/mesos/windows_launcher.hpp (lines 48 - 55) <https://reviews.apache.org/r/56362/#comment236341> Definitely update this comment :) src/slave/containerizer/mesos/windows_launcher.cpp (lines 92 - 98) <https://reviews.apache.org/r/56362/#comment236380> Unless there's a good reason, you should just return an Error here if either of these arguments are `Some`. src/slave/containerizer/mesos/windows_launcher.cpp (lines 105 - 106) <https://reviews.apache.org/r/56362/#comment236381> We're definitely not on systemd... on Windows :D src/slave/containerizer/mesos/windows_launcher.cpp (lines 109 - 127) <https://reviews.apache.org/r/56362/#comment236382> Why are you removing the CREATE_JOB parent hook (https://reviews.apache.org/r/56368/ ) instead of using it here? We may want to use JobObjects for other subprocesses in general, rather than just the launcher. In which case, having a CREATE_JOB helper would be useful. src/slave/containerizer/mesos/windows_launcher.cpp (line 139) <https://reviews.apache.org/r/56362/#comment236383> This is a no-op on Windows. The intended behavior is to prevent the child process from exiting if the parent exits. On Windows, we may eventually want to introduce the opposite type of hook (having the child exit if the parent exits). src/slave/containerizer/mesos/windows_launcher.cpp (lines 155 - 156) <https://reviews.apache.org/r/56362/#comment236384> The reason for this forward declaration is that `_destroy` is not declared in the WindowsLauncher header. You should just declare it. src/slave/containerizer/mesos/windows_launcher.cpp (lines 203 - 207) <https://reviews.apache.org/r/56362/#comment236385> One newline here. - Joseph Wu On Feb. 7, 2017, 10:54 a.m., Andrew Schwartzmeyer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56362/ > ----------------------------------------------------------- > > (Updated Feb. 7, 2017, 10:54 a.m.) > > > Review request for mesos, Alex Clemmer and Joseph Wu. > > > Bugs: MESOS-6892 > https://issues.apache.org/jira/browse/MESOS-6892 > > > Repository: mesos > > > Description > ------- > > Instead of deriving `WindowsLauncher` from `PosixLauncher`, > this commit implements a separate `WindowsLauncher` derived > from `Launcher` parallel to the `PosixLauncher`. > > The purpose of this is to setup for refactoring the `WindowsLauncher` > into a `Launcher` that uses Windows' "Job Objects," > which are similar to Linux's cgroups, and enable us to reason > about a group of processes. It is necessary to do this in the > `WindowsLaucher` because the current implementation uses the POSIX > idea of a process hierarchy to list and kill all processes > associated with a task. We have found this model to not work on > Windows, and the solution is to use Job Objects. > > Because this is preparation work, it is a straight copy of the > current "working" implementation. That is, the `PosixLauncher` > has been copied to create the `WindowsLauncher`, instead of > the latter deriving the former. > > > Diffs > ----- > > src/CMakeLists.txt c09bcde3c14a73ab0b296c400f79f6a775da2470 > src/slave/containerizer/mesos/containerizer.cpp > d2b4f75a55dbe4746bc2dfc180335fa831a554ef > src/slave/containerizer/mesos/launcher.hpp > 79f6eea0ee8e564e90b36208672df150dbc5d540 > src/slave/containerizer/mesos/launcher.cpp > 5114c130efbfb252dde1e85c081f5174e66f57af > src/slave/containerizer/mesos/windows_launcher.hpp PRE-CREATION > src/slave/containerizer/mesos/windows_launcher.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/56362/diff/ > > > Testing > ------- > > > Thanks, > > Andrew Schwartzmeyer > >
