----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33174/#review110547 -----------------------------------------------------------
Initial CFS parameters should be specified to Docker using the appropriate flags, not tacked onto the end of launch() where we don't yet know the cgroup. Subsequent updates done by Mesos in update(). src/slave/containerizer/docker.cpp (line 838) <https://reviews.apache.org/r/33174/#comment170483> Docker supports specifying the CFS period and quota to run a container with these flags {{--cpu-period=0}} and {{--cpu-quota=0}}. The correct solution is to set these using Docker itself when it runs the container. Subsequent updates will be done by Mesos in the update() call, as below in this review. src/slave/containerizer/docker.cpp (line 839) <https://reviews.apache.org/r/33174/#comment170480> FYI, it would be {{.then([]() { return true; });}} but this piece of code isn't needed, see previous comment. src/slave/containerizer/docker.cpp (line 976) <https://reviews.apache.org/r/33174/#comment170475> To reiterate my earlier comment: 1) We don't support changing the cfs period so it only needs to be set once, additional writes are unnecessary but aren't a problem. The MesosContainerizer isolator just writes it on every update rather than determining if it has already written it before. 2) The cfs quota changes depending on the CPU resource value so it definitely does need to be re-written at every update. Again, this is a straight copy-and-paste from the MC cpu isolator. This part of the code looks fine to me. @Tim, do you concur? - Ian Downes On April 14, 2015, 1:32 p.m., Steve Niemitz wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33174/ > ----------------------------------------------------------- > > (Updated April 14, 2015, 1:32 p.m.) > > > Review request for mesos, Ian Downes, Jie Yu, and Timothy Chen. > > > Bugs: MESOS-2617 > https://issues.apache.org/jira/browse/MESOS-2617 > > > Repository: mesos > > > Description > ------- > > Fix for docker containerizer not configuring CFS quotas correctly. > > It would be nice to refactor all this isolation code in a way that can be > shared between all containerizers, as this is basically just copied from the > CgroupsCpushareIsolator, but that's a much bigger undertaking. > > > Diffs > ----- > > src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7 > > Diff: https://reviews.apache.org/r/33174/diff/ > > > Testing > ------- > > > Thanks, > > Steve Niemitz > >
