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 45086: Enable cgroups unified isolator in isolation.
Date Wed, 01 Jun 2016 23:50:23 GMT


> On June 1, 2016, 11:09 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 242-245
> > <https://reviews.apache.org/r/45086/diff/13/?file=1347681#file1347681line242>
> >
> >     This seems not consistent with the design doc. In the design doc, I think we
are going to introduce a new isolation mechanism `cgroups/unified` and a new flag `--cgroups_subsystems`,
but here I see we reuse the existing cgroups isolation mechanisms. Maybe the design doc should
be updated?
> 
> haosdent huang wrote:
>     Yes, the design doc need to be updated. We would like to add this without any changes
when user upgrade.

Understood, actually I also like the idea to introduce this new feature without any changes
when user upgrades. However I am thinking how we are going to this case during upgrade: In
an old Mesos cluster, user has enabled `"cgroups/cpu"` for the agent which means internally
both `cpu` and `cpuacct` subsystems have been enabled, however after upgrading the cluster
with this new feature with the same flags to launch the agent, only `cpu` subsystem will be
enabled, right? So after upgrade we loose one subsystem (`cpuacct`)?


> On June 1, 2016, 11:09 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 150
> > <https://reviews.apache.org/r/45086/diff/13/?file=1347681#file1347681line150>
> >
> >     Not sure why you want to add `cgroups/cpuacct` here. Actually in the `creator`
hashmap below, I do not see `cgroups/cpuacc` as a key, i.e., no create() method corresponding
to `cgroups/cpuacc`. So I guess an `Unknown or unsupported isolator` error will happen when
`cgroups/cpuacc` is specified as the value of `--isolation`.
> 
> haosdent huang wrote:
>     Hi, @qianzhang Thanks a lot for your review! We need add cgroups/cpuacct here because
we split the original cpu isolator which contains cpu and cpuacct into two subsystems. To
compatiable old behaviour, we add it back at here.

@haosdent, you are welcome! I agree this is the right way to keep the backward compatibility.
However, as I mentioned in my comment above, the question is in the `creator` hashmap, there
is no create() method for `"cgroups/cpuacc"`, that means when agent is started with `"--isolation=cgroups/cpuacct"`
or `"--isolation=cgroups"`, we will not find the related create() method, so the `"Unknown
or unsupported isolator"` error will occur.


- Qian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45086/#review135801
-----------------------------------------------------------


On April 16, 2016, 6:29 p.m., haosdent huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45086/
> -----------------------------------------------------------
> 
> (Updated April 16, 2016, 6:29 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Alex Clemmer, Ian Downes, Jie Yu,
and Kevin Klues.
> 
> 
> Bugs: MESOS-5041
>     https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Enable cgroups unified isolator in isolation.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.cpp 1e1a36903f4377497bb72b69e4ead63675d453c0

> 
> Diff: https://reviews.apache.org/r/45086/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> haosdent huang
> 
>


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