mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Qian Zhang <zhq527...@gmail.com>
Subject Re: Review Request 71886: Set container's `cpu.cfs_quota_us` to its CPU resource limit.
Date Tue, 03 Mar 2020 14:50:23 GMT


> On Feb. 29, 2020, 2:20 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp
> > Lines 107 (patched)
> > <https://reviews.apache.org/r/71886/diff/4/?file=2212169#file2212169line108>
> >
> >     For executor containers, you have logic in `launchExecutor()` which inspects
the value of `flags.cgroups_enable_cfs`, and also sets the limits equal to the resource requests
when appropriate.
> >     
> >     It seems strange to me to have some of that logic in the agent code, and some
of that logic in the isolators. I would expect that we either put it all in the agent, or
all in the isolators.
> >     
> >     What about adding code to the `LaunchContainer` API handler which does this
for tasks, and then the isolator doesn't have to care about the semantics of the `cgroups_enable_cfs`
flag and the limits being set to the resource requests in some cases.

The logic in `launchExecutor()` (actually it is in `Slave::__run()` before `launchExecutor`)
which inspects the value of `--cgroups_enable_cfs` will ONLY be hit in the case that an executor
launches a task group, some tasks in the group have CPU limit set while some others have not.
So how do we handle the case like command task? For a command task, if it has no CPU limit
set, we still need to set its CFS quota to its CPU request if `--cgroups_enable_cfs` is true
which is currently handled in the isolator code. But if we remove such code from isolator,
the command task case will be missed since it cannot be handled in `Slave::__run()` and the
`LaunchContainer` API handler.


- Qian


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


On Feb. 26, 2020, 8:12 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71886/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2020, 8:12 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10047
>     https://issues.apache.org/jira/browse/MESOS-10047
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Set container's `cpu.cfs_quota_us` to its CPU resource limit.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp 960bd141430387e076a8fab1948d07719613ed90

> 
> 
> Diff: https://reviews.apache.org/r/71886/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


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