> On June 12, 2016, 2:51 p.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, lines 351-363 > > <https://reviews.apache.org/r/46158/diff/2/?file=1347650#file1347650line351> > > > > 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.
We delegate the create/destory operations of hierarchy to `CgroupsIsolatorProcess`. Because if we do the create/destroy operations of hierarchy in `Subsystem::prepare()` and `Subsystem::cleanup()`, it would create the hierarchy or destroy the hierarchy multiple times when differnt subsystems mount in same hierarchies. > On June 12, 2016, 2:51 p.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, lines 428-445 > > <https://reviews.apache.org/r/46158/diff/2/?file=1347650#file1347650line428> > > > > 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. Same reason as above, suppose we mount multiple subsystems into same hierarchy, isolate would be executed multiple times. > On June 12, 2016, 2:51 p.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, lines 592-600 > > <https://reviews.apache.org/r/46158/diff/2/?file=1347650#file1347650line592> > > > > Again I think destroying container's cgroup should be handled in > > `Subsystem::cleanup()` method. Same reason as above, suppose we mount multiple subsystems into same hierarchy, the hierarchy would be destroyed multiple times. - haosdent ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46158/#review137145 ----------------------------------------------------------- On June 19, 2016, 3:34 p.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46158/ > ----------------------------------------------------------- > > (Updated June 19, 2016, 3:34 p.m.) > > > Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, > Kevin Klues, and Qian Zhang. > > > 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 > >