mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jie Yu" <yujie....@gmail.com>
Subject Re: Review Request 30545: cgroups: added support to listen on memory pressures.
Date Sat, 21 Feb 2015 01:27:02 GMT

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


The code looks great! Given that the tests are quite large. Can you split this patch and I'll
give a shipit for the code.

For tests, I need to do a pass next week. Thanks! THis is great, Chi!


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

    Any reason this is not a static method of pressure::Listener?



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

    Well, you don't need this friend declaration if 'create' is a static member funciton,
right?



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

    Look like only one place is using this constant. Sugguest killing it.



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

    I think we indent 2 spaces for switch 'case'.



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

    I think here should be able to do:
    ```
      : coutner_(0),
        error(None()),
        process(new event::Listener(
            hierarchy,
            cgroup,
            "memory.pressure_level",
            stringify(level))) {}
    ```



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

    Remove the tailing period.



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

    We don't use std:: prefix in cpp files. Please remove them (do a sweep in this file).


- Jie Yu


On Feb. 20, 2015, 9:37 p.m., Chi Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30545/
> -----------------------------------------------------------
> 
> (Updated Feb. 20, 2015, 9:37 p.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/Makefile.am d372404d84c9ac0bc49da7407ad366701b9586a6 
>   src/linux/cgroups.hpp e07772fec11b9bb73a44c54326f24d7ee1e8a64b 
>   src/linux/cgroups.cpp a307e27c5840270e28cced2d5aeb90c6679bff1d 
>   src/tests/cgroups_tests.cpp 9d50a47c939f5f136c85fdcc555459512c6f2259 
>   src/tests/memory_test_helper.hpp PRE-CREATION 
>   src/tests/memory_test_helper.cpp PRE-CREATION 
>   src/tests/memory_test_helper_main.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/30545/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>


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