mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request: Adding cpuset isolation to cgroups.
Date Tue, 20 Nov 2012 20:12:36 GMT


> On Nov. 20, 2012, 3:56 p.m., Vinod Kone wrote:
> > src/slave/cgroups_isolation_module.cpp, line 640
> > <https://reviews.apache.org/r/8108/diff/1-2/?file=191733#file191733line640>
> >
> >     why don't you pull info->cpuset.get() out into a variable. Looks like its
used a bunch here.
> >     
> >     some of the statements below might actually fit into one line then.
> 
> Ben Mahler wrote:
>     I thought about that but it's used non const, so what would that look like?
>     
>     CPUSet& cpuset = info->cpuset;
>     CPUSet* cpuset = &(info->cpuset);
>     
>     Both of which didn't feel consistent with mesos style.
>     Actually, they fit on one line now that I went from hashmap -> map.
> 
> Vinod Kone wrote:
>     we typically just store it in a variable (CPUSet cpuset = info->cpuset), irrespective
of constness. But, I will leave it upto you.

That would create a copy! I need to mutate the one inside info.


- Ben


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


On Nov. 20, 2012, 7:32 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8108/
> -----------------------------------------------------------
> 
> (Updated Nov. 20, 2012, 7:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This is the first pass at adding cpuset isolation for pinning cgroups to cpus.
> 
> We decided to start with a simplistic grow/shrink allocation technique, as such this
initial technique:
>   -Does not take cache locality into account.
>   -Does not actively fight fragmentation*, but does a good job at preventing it in many
cases, given it's simplicity.
>   -Note that when cpus resource requests are integral (non-fractional), then fragmentation
does not occur.
> 
> *By fragmentation, I'm referring to the case where we've spread a cgroup over more cpus
than necessary, due to other cgroups sharing the same cpus.
> High fragmentation would mean a lot of shared cpus across cgroups.
> No fragmentation would mean each cgroup has a unique set of cpus.
> 
> I've punted on documenting the pitfalls of this technique, wiring up the handler, and
adding tests for now.
> 
> Note that this is diffed off of benh's changes:
> https://reviews.apache.org/r/8058/
> https://reviews.apache.org/r/8059/
> 
> 
> Diffs
> -----
> 
>   src/linux/proc.hpp 27e15bf8695aa694b0d5bdb6881b9fa55a447528 
>   src/slave/cgroups_isolation_module.hpp 9f80fc5a969b959b34eaea4cac40700662d7f8b2 
>   src/slave/cgroups_isolation_module.cpp 8211618d7729350654e2d17946c5b912ed9dda6a 
>   third_party/libprocess/include/stout/stringify.hpp dcc5b95a54e6f34f93867e015d8c855fd7d6f950

>   third_party/libprocess/include/stout/strings.hpp 914c280a994733764957d19f37b48d151bb93778

> 
> Diff: https://reviews.apache.org/r/8108/diff/
> 
> 
> Testing
> -------
> 
> None as of yet.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


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