> On June 12, 2016, 10: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. > > haosdent huang wrote: > 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. > > Qian Zhang wrote: > My idea is, here we should call each subsystem's `prepare()` (the second > `for` loop in this method), and each subsystem's `prepare()` should call its > parent class's `prepare()` method (i.e., `Subsystem::prepare()`), and in > `Subsystem::prepare()`, we call the virtual method `name()` first to know > which subsystem that we are working on, and then ONLY create cgroup for that > subsytem. In this way, we keep the common cgroup operations (e.g., create > cgroup) only in parent class (e.g., `Subsystem::prepare()`), and we can still > do some subsystem specific works in child class (e.g., > `MemorySubsystem::prepare()`). > > Currently, `Subsystem::prepare()`, `Subsystem::isolate()` and > `Subsystem::cleanup()` are just no-op, I think we should fully utilize them. > > haosdent huang wrote: > Yes, but it would create/destroy the hierarchy multiple times if we put > these operations to `Subsystem`.
What did you mean about "create/destroy the hierachy"? Did you mean creating `/sys/fs/cgroup/<subsystem>/mesos`? If yes, I think it is a one-time operation which will be only done in `CgroupsIsolatorProcess::create()`, we do not need to call any methods of `Subsystem` at all. - Qian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46158/#review137145 ----------------------------------------------------------- On June 19, 2016, 11:55 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, 11:55 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 > >
