mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Qian Zhang <zhang...@cn.ibm.com>
Subject Re: Review Request 46158: Completed implementation of the cgroups unified isolator.
Date Tue, 21 Jun 2016 01:50:19 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message