----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9464/#review17308 -----------------------------------------------------------
Getting close! A few high level comments: 1. I don't think we want to split the handlers, I thought it was cleaner when you were doing all the cpu handling inside cpusChanged(). With this change there are now two handlers and one uses the other. I think to keep things intuitive let's just place the logic inside cpusChanged and guard it with a check of the flag. 2. What else do you anticipate using this flag for? I would consider it to be a temporary measure until we can notify schedulers of the level of cpu isolation available. Hence wanting it to be something simple like --cgroups_enable_cfs. We will then be killing this flag when we make resources first class: https://issues.apache.org/jira/browse/MESOS-338. - Ben Mahler On March 2, 2013, 12:49 a.m., David Mackey wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9464/ > ----------------------------------------------------------- > > (Updated March 2, 2013, 12:49 a.m.) > > > Review request for mesos, Benjamin Hindman, Vinod Kone, and Ben Mahler. > > > Description > ------- > > Initial support for CPU "hard limits" via CFS Bandwidth Control in cgroups. > > CFS is unique relative to existing Mesos cgroups support in that it is a > "subfeature" of an already supported cgroups subsystem, cpu. Also, there are > two "tunables" for configuring CFS bandwidth limiting. > > There are 4 approaches one could take: > 1) Use the CFS bandwidth limiting if the feature is present > 2) Expose as separate flag, eg "cpu,cfs,memory,freezer" > 3) Add feature flag support to subsystems via an additional delimiter, eg > "cpu+cfs,memory,freezer". > 4) Add an additional control flag via some other means > > Option 2's downside breaks the 1:1 mapping between cgroups subsystems and a > cgroups resource flag. > Option 3's downside is it greatly increases complexity of parsing cgroups > subsystem flags. > > This diff takes option 1. > > > This addresses bug MESOS-315. > https://issues.apache.org/jira/browse/MESOS-315 > > > Diffs > ----- > > src/slave/cgroups_isolation_module.hpp a04fc46 > src/slave/cgroups_isolation_module.cpp a779de8 > src/slave/flags.hpp d4aa045 > > Diff: https://reviews.apache.org/r/9464/diff/ > > > Testing > ------- > > make check + additional testing > > > Thanks, > > David Mackey > >
