> 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
>
>
