----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46158/#review137145 -----------------------------------------------------------
src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 351 - 363) <https://reviews.apache.org/r/46158/#comment202301> Why do we prepare hierarchy here? I think preparing hierarchy for a specific container is a low-level work which should be handled in `Subsystem::prepare()` method rather than here. And in that way, we only need one `for` loop (the following one) rather than 2 here. src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 388 - 390) <https://reviews.apache.org/r/46158/#comment202314> I am wondering if we really need this method. I think If we introduce a static member `hashmap<ContainerID, process::Owned<Info>> infos` in the class `Subsystem` as I mentioned in this comment https://reviews.apache.org/r/46158/#comment201273, then we may not need this `CgroupsIsolatorProcess::_prepare()` method, because in `CgroupsIsolatorProcess::watch()`, we can directly watch the `limitation` of the Info object in that static `infos` hashmap, and in each Subsystem (e.g., in `MemorySubsystem::oom()`), we can directly trigger this `limiation`. src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 428 - 445) <https://reviews.apache.org/r/46158/#comment202319> Again I think assigning container to its own cgroup should be handled in `Subsystem::isolate()` method. In that way, we only need one `for` loop (the following one) rather than 2 here. src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 592 - 600) <https://reviews.apache.org/r/46158/#comment202324> Again I think destroying container's cgroup should be handled in `Subsystem::cleanup()` method. - Qian Zhang On April 16, 2016, 6:14 p.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46158/ > ----------------------------------------------------------- > > (Updated April 16, 2016, 6:14 p.m.) > > > Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and > Kevin Klues. > > > Bugs: MESOS-5041 > https://issues.apache.org/jira/browse/MESOS-5041 > > > Repository: mesos > > > Description > ------- > > Completed implementation of the cgroups unified isolator. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/46158/diff/ > > > Testing > ------- > > > Thanks, > > haosdent huang > >
