----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20429/#review40969 -----------------------------------------------------------
src/slave/containerizer/cgroups_launcher.cpp <https://reviews.apache.org/r/20429/#comment74292> properly align " for container" src/slave/containerizer/cgroups_launcher.cpp <https://reviews.apache.org/r/20429/#comment74304> How about something like this? // pseudo code. foreach (orphan, orphans) { Future<bool> future = cgroups::freeze(orphan) .then(&CgroupsLauncher::reap, ...) .then(cgroups::destroy, ...) futures.push_back(future); } collect(futures) .onAny(_recover, ....) Where CgroupsLauncher::reap() does a snapshot of processes and collects the future of the reaps. Alternatively, you can stick the reaping into cgroups::destroy which will simplify this code quite a bit and also could be taken advantage of by our test teardown methods. src/slave/containerizer/cgroups_launcher.cpp <https://reviews.apache.org/r/20429/#comment74295> Hmm. What happens if a process forks after we take a snapshot? How about taking a snapshot after you call cgroups::freeze? src/slave/containerizer/cgroups_launcher.cpp <https://reviews.apache.org/r/20429/#comment74293> properly align "processes.error()" src/slave/containerizer/cgroups_launcher.cpp <https://reviews.apache.org/r/20429/#comment74300> Why 5s timeout? What if it takes longer? Also, why .then instead of .any? src/slave/containerizer/mesos_containerizer.cpp <https://reviews.apache.org/r/20429/#comment74305> What happens if launcher's recover fails? - Vinod Kone On April 16, 2014, 10:11 p.m., Ian Downes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20429/ > ----------------------------------------------------------- > > (Updated April 16, 2014, 10:11 p.m.) > > > Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone. > > > Repository: mesos-git > > > Description > ------- > > A process that is killed will move out of a cgroup, so cgroups::destroy will > see the cgroup as empty and complete, but the process may not have been > reaped. This patch ensures that all processes are reaped before the launcher > declares a container destroyed. > > > Diffs > ----- > > src/slave/containerizer/cgroups_launcher.hpp > db61107141f60dd13cfa13278a7daab16f0d108c > src/slave/containerizer/cgroups_launcher.cpp > 39f0e4c3baaed3403da160ba63dada4a53d9af09 > src/slave/containerizer/launcher.hpp > dee526f254d56a7a974a70522cd0ca059814fb6d > src/slave/containerizer/launcher.cpp > c83327b2f13f0511c8e4e0e9902bc6a1e1328283 > src/slave/containerizer/mesos_containerizer.hpp > ee1fd3010b4a178a6abcbd813af5eba4d564be00 > src/slave/containerizer/mesos_containerizer.cpp > 1ce41d71eb13582b46d240d48f0508bdc3e7ef10 > > Diff: https://reviews.apache.org/r/20429/diff/ > > > Testing > ------- > > make check # Linux > > > Thanks, > > Ian Downes > >
