> 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. > > 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`. > > Qian Zhang wrote: > 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.
Oh, not. `/sys/fs/cgroup/<subsystem>/mesos` is the root hierachy. Suppose we mount both `cpu` and `cpuacct` into `/sys/fs/cgroup/cpu,cpuacct/mesos`. If we delegate the create operation to `Subsystem`, then we would create `/sys/fs/cgroup/cpu,cpuacct/mesos/${container_id}` when `CpuSubsystem::prepare()` and `CpuacctSubsystem::prepare()`. This would bring problems. - haosdent ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46158/#review137145 ----------------------------------------------------------- On June 19, 2016, 3: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, 3: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 > >