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


Great!


include/mesos/mesos.proto
<https://reviews.apache.org/r/13868/#comment50288>

    Can you move this right below the other cpu related stats? Just so we can 
visually group related stats in the protobuf.
    
    Would be nice to have a unit on cpus_throttled_time.
    
    How about either:
    optional double cpus_throttled_time_secs;
    optional uint64 cpus_throttled_time_ns;



src/slave/cgroups_isolator.cpp
<https://reviews.apache.org/r/13868/#comment50290>

    Please kill the trailing whitespace here and elsewhere.



src/slave/cgroups_isolator.cpp
<https://reviews.apache.org/r/13868/#comment50298>

    How about:
    
    // Add the cpu.stat information.
    
    No need to list them since the code lists them as well :)



src/slave/constants.hpp
<https://reviews.apache.org/r/13868/#comment50291>

    Why the move?



src/slave/constants.cpp
<https://reviews.apache.org/r/13868/#comment50295>

    You can kill this comment.
    
    Can we collect this information once per second with little performance 
impact?



src/slave/monitor.cpp
<https://reviews.apache.org/r/13868/#comment50297>

    s/TODO/ TODO/


- Ben Mahler


On Aug. 29, 2013, 11:21 p.m., Christina Delimitrou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13868/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2013, 11:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Ian Downes, and Jie 
> Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Adding fields to record the cpu.stat statistics per cgroup. These include 
> nr_periods, nr_throttled and throttled_time. 
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto bbf1d31 
>   src/slave/cgroups_isolator.cpp 676768e 
>   src/slave/constants.hpp bbbbfd3 
>   src/slave/constants.cpp 8c74c00 
>   src/slave/monitor.cpp 8e1eb35 
>   src/slave/process_isolator.cpp fa80293 
> 
> Diff: https://reviews.apache.org/r/13868/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Christina Delimitrou
> 
>

Reply via email to