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