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



src/linux/cgroups.hpp
<https://reviews.apache.org/r/30545/#comment116666>

    meta note: cgroups.hpp should probably be split into one file per 
class/namespace.



src/linux/cgroups.hpp
<https://reviews.apache.org/r/30545/#comment116657>

    could this return a Try<Owned<PressureListener>> instead?



src/linux/cgroups.hpp
<https://reviews.apache.org/r/30545/#comment116665>

    there's a static method that creates a PressureListener, and a constructor 
to create a Process, and they take similar arguments.
    
    this is a confusing API.
    
    as an end user of this system, what should I be calling? should the 
PressureListener be the public class and the PressureListenerProcess be a 
private implementation detail?



src/linux/cgroups.hpp
<https://reviews.apache.org/r/30545/#comment116655>

    this sentence is odd. error if there is any what?



src/linux/cgroups.hpp
<https://reviews.apache.org/r/30545/#comment116661>

    s/data/count/



src/linux/cgroups.hpp
<https://reviews.apache.org/r/30545/#comment116660>

    why is there a trailing underscore?



src/linux/cgroups.cpp
<https://reviews.apache.org/r/30545/#comment116656>

    const char PressureListenerProcess::CONTROL[] = "memory.pressure_level";
    
    no non-pod statics please.



src/linux/cgroups.cpp
<https://reviews.apache.org/r/30545/#comment116662>

    how do you know it isn't more than sizeof(data)?



src/linux/cgroups.cpp
<https://reviews.apache.org/r/30545/#comment116663>

    you don't want a future here.. you want a Try, i think.


- Dominic Hamon


On Feb. 4, 2015, 11:50 a.m., Chi Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30545/
> -----------------------------------------------------------
> 
> (Updated Feb. 4, 2015, 11:50 a.m.)
> 
> 
> Review request for mesos, Dominic Hamon, Ian Downes, and Jie Yu.
> 
> 
> Bugs: MESOS-2136
>     https://issues.apache.org/jira/browse/MESOS-2136
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> cgroups: added support to listen on memory pressures.
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.hpp abf31df 
>   src/linux/cgroups.cpp 0b136e1 
> 
> Diff: https://reviews.apache.org/r/30545/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Reply via email to