mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ian Downes" <ian.dow...@gmail.com>
Subject Re: Review Request 33174: Fix for docker not configuring CFS quotas correctly
Date Tue, 15 Dec 2015 19:36:50 GMT

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


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