You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2013/01/24 10:21:23 UTC

Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

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

Review request for mesos, Benjamin Hindman and Vinod Kone.


Description
-------

This is the meat of the resource monitoring design.

-Added a ResourceUsage protobuf.
-Added a ResourceMonitor process that periodically hits the isolation module for usage information.
-Usage is exported to the Statistics process.
-Usage is available via a JSON endpoint.

Implementation of the isolation module code will follow in subsequent reviews.


This addresses bug MESOS-324.
    https://issues.apache.org/jira/browse/MESOS-324


Diffs
-----

  include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
  include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
  src/Makefile.am 0ab59b75b2955c532d0833f132bdaffe323838d0 
  src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
  src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
  src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
  src/slave/constants.hpp ddf02570caf3793106b3c48e158a5bb48c1ae80c 
  src/slave/constants.cpp 1735a6b55a93e6537a5a119e5345961f3d84a000 
  src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
  src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
  src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
  src/slave/monitor.hpp PRE-CREATION 
  src/slave/monitor.cpp PRE-CREATION 
  src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
  src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
  src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
  src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
  src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
  third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
  third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
  third_party/libprocess/src/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 

Diff: https://reviews.apache.org/r/9094/diff/


Testing
-------

make check

Although I didn't add tests, I've manually tested end-to-end with my future reviews.


Thanks,

Ben Mahler

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Ben Mahler <be...@gmail.com>.


> On Jan. 30, 2013, 2:13 a.m., Vinod Kone wrote:
> > third_party/libprocess/src/statistics.cpp, line 106
> > <https://reviews.apache.org/r/9094/diff/1/?file=251555#file251555line106>
> >
> >     I don't like the fact that you are using Future for something that is not asynchronous.

Me neither.. (see my comment on the last review of this you did), I could return an Option, but then it will come out of Statistics as Future<Option<double>>, which is not so nice.. any other ideas?


- Ben


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


On Jan. 30, 2013, 4:01 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9094/
> -----------------------------------------------------------
> 
> (Updated Jan. 30, 2013, 4:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This is the meat of the resource monitoring design.
> 
> -Added a ResourceUsage protobuf.
> -Added a ResourceMonitor process that periodically hits the isolation module for usage information.
> -Usage is exported to the Statistics process.
> -Usage is available via a JSON endpoint.
> 
> Implementation of the isolation module code will follow in subsequent reviews.
> 
> 
> This addresses bug MESOS-324.
>     https://issues.apache.org/jira/browse/MESOS-324
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
>   include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
>   src/Makefile.am 0ab59b75b2955c532d0833f132bdaffe323838d0 
>   src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
>   src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
>   src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
>   src/slave/constants.hpp ddf02570caf3793106b3c48e158a5bb48c1ae80c 
>   src/slave/constants.cpp 1735a6b55a93e6537a5a119e5345961f3d84a000 
>   src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
>   src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
>   src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
>   src/slave/monitor.hpp PRE-CREATION 
>   src/slave/monitor.cpp PRE-CREATION 
>   src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
>   src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
>   src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
>   src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
>   src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
>   third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
>   third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
>   third_party/libprocess/src/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 
> 
> Diff: https://reviews.apache.org/r/9094/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Although I didn't add tests, I've manually tested end-to-end with my future reviews.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Ben Mahler <be...@gmail.com>.


> On Jan. 30, 2013, 2:13 a.m., Vinod Kone wrote:
> > third_party/libprocess/src/statistics.cpp, line 106
> > <https://reviews.apache.org/r/9094/diff/1/?file=251555#file251555line106>
> >
> >     I don't like the fact that you are using Future for something that is not asynchronous.
> 
> Ben Mahler wrote:
>     Me neither.. (see my comment on the last review of this you did), I could return an Option, but then it will come out of Statistics as Future<Option<double>>, which is not so nice.. any other ideas?
> 
> Vinod Kone wrote:
>     I think its perfectly reasonable to return Future<Option<double>> from Statistics. I think of it this way,
>     
>     if the user uses the other get() to get context wide stats, he still needs to do map.contains(stat) if he wants to know if a particular stat is present.
>     
>     similarly, for the stat specific get(), a user should check if should a stat existed by doing option.isSome().
>     
>     I think thats a well-understood behavior.

Fixed!


- Ben


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


On Jan. 31, 2013, 6:17 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9094/
> -----------------------------------------------------------
> 
> (Updated Jan. 31, 2013, 6:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This is the meat of the resource monitoring design.
> 
> -Added a ResourceUsage protobuf.
> -Added a ResourceMonitor process that periodically hits the isolation module for usage information.
> -Usage is exported to the Statistics process.
> -Usage is available via a JSON endpoint.
> 
> Implementation of the isolation module code will follow in subsequent reviews.
> 
> 
> This addresses bug MESOS-324.
>     https://issues.apache.org/jira/browse/MESOS-324
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
>   include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
>   src/Makefile.am 0ab59b75b2955c532d0833f132bdaffe323838d0 
>   src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
>   src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
>   src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
>   src/slave/constants.hpp ddf02570caf3793106b3c48e158a5bb48c1ae80c 
>   src/slave/constants.cpp 1735a6b55a93e6537a5a119e5345961f3d84a000 
>   src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
>   src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
>   src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
>   src/slave/monitor.hpp PRE-CREATION 
>   src/slave/monitor.cpp PRE-CREATION 
>   src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
>   src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
>   src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
>   src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
>   src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
>   third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
>   third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
>   third_party/libprocess/src/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 
> 
> Diff: https://reviews.apache.org/r/9094/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Although I didn't add tests, I've manually tested end-to-end with my future reviews.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Vinod Kone <vi...@gmail.com>.


> On Jan. 30, 2013, 2:13 a.m., Vinod Kone wrote:
> > third_party/libprocess/src/statistics.cpp, line 106
> > <https://reviews.apache.org/r/9094/diff/1/?file=251555#file251555line106>
> >
> >     I don't like the fact that you are using Future for something that is not asynchronous.
> 
> Ben Mahler wrote:
>     Me neither.. (see my comment on the last review of this you did), I could return an Option, but then it will come out of Statistics as Future<Option<double>>, which is not so nice.. any other ideas?

I think its perfectly reasonable to return Future<Option<double>> from Statistics. I think of it this way,

if the user uses the other get() to get context wide stats, he still needs to do map.contains(stat) if he wants to know if a particular stat is present.

similarly, for the stat specific get(), a user should check if should a stat existed by doing option.isSome().

I think thats a well-understood behavior.


- Vinod


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


On Jan. 31, 2013, 6:17 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9094/
> -----------------------------------------------------------
> 
> (Updated Jan. 31, 2013, 6:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This is the meat of the resource monitoring design.
> 
> -Added a ResourceUsage protobuf.
> -Added a ResourceMonitor process that periodically hits the isolation module for usage information.
> -Usage is exported to the Statistics process.
> -Usage is available via a JSON endpoint.
> 
> Implementation of the isolation module code will follow in subsequent reviews.
> 
> 
> This addresses bug MESOS-324.
>     https://issues.apache.org/jira/browse/MESOS-324
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
>   include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
>   src/Makefile.am 0ab59b75b2955c532d0833f132bdaffe323838d0 
>   src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
>   src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
>   src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
>   src/slave/constants.hpp ddf02570caf3793106b3c48e158a5bb48c1ae80c 
>   src/slave/constants.cpp 1735a6b55a93e6537a5a119e5345961f3d84a000 
>   src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
>   src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
>   src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
>   src/slave/monitor.hpp PRE-CREATION 
>   src/slave/monitor.cpp PRE-CREATION 
>   src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
>   src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
>   src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
>   src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
>   src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
>   third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
>   third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
>   third_party/libprocess/src/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 
> 
> Diff: https://reviews.apache.org/r/9094/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Although I didn't add tests, I've manually tested end-to-end with my future reviews.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Vinod Kone <vi...@gmail.com>.

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



third_party/libprocess/src/statistics.cpp
<https://reviews.apache.org/r/9094/#comment34073>

    I don't like the fact that you are using Future for something that is not asynchronous.


- Vinod Kone


On Jan. 29, 2013, 1:38 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9094/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2013, 1:38 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This is the meat of the resource monitoring design.
> 
> -Added a ResourceUsage protobuf.
> -Added a ResourceMonitor process that periodically hits the isolation module for usage information.
> -Usage is exported to the Statistics process.
> -Usage is available via a JSON endpoint.
> 
> Implementation of the isolation module code will follow in subsequent reviews.
> 
> 
> This addresses bug MESOS-324.
>     https://issues.apache.org/jira/browse/MESOS-324
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
>   include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
>   src/Makefile.am 0ab59b75b2955c532d0833f132bdaffe323838d0 
>   src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
>   src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
>   src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
>   src/slave/constants.hpp ddf02570caf3793106b3c48e158a5bb48c1ae80c 
>   src/slave/constants.cpp 1735a6b55a93e6537a5a119e5345961f3d84a000 
>   src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
>   src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
>   src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
>   src/slave/monitor.hpp PRE-CREATION 
>   src/slave/monitor.cpp PRE-CREATION 
>   src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
>   src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
>   src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
>   src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
>   src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
>   third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
>   third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
>   third_party/libprocess/src/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 
> 
> Diff: https://reviews.apache.org/r/9094/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Although I didn't add tests, I've manually tested end-to-end with my future reviews.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Ben Mahler <be...@gmail.com>.

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

(Updated Feb. 25, 2013, 7:17 p.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Rebased off trunk.


Description
-------

This is the meat of the resource monitoring design.

-Added a ResourceUsage protobuf.
-Added a ResourceMonitor process that periodically hits the isolation module for usage information.
-Usage is exported to the Statistics process.
-Usage is available via a JSON endpoint.

Implementation of the isolation module code will follow in subsequent reviews.


This addresses bug MESOS-324.
    https://issues.apache.org/jira/browse/MESOS-324


Diffs (updated)
-----

  include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
  include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
  src/Makefile.am c94736df660a25b58dc47c07d9c56c3c26152a66 
  src/files/files.cpp 837de67aa8a9c0926080b503eb2666d9f036ae66 
  src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
  src/slave/cgroups_isolation_module.cpp a2eba6f96f5d8a4b1257571aa29e37c5682aab8d 
  src/slave/flags.hpp 39e57f4104ee7a1538436ebbb9493581e28c99dd 
  src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
  src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
  src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
  src/slave/monitor.hpp PRE-CREATION 
  src/slave/monitor.cpp PRE-CREATION 
  src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
  src/slave/process_based_isolation_module.cpp 12a579cba56cd3dac384bc7919b0d5537b0e429d 
  src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
  src/slave/slave.cpp d4721c3eb51db87278d05f6fbe2eadb8a3a9b4dd 
  src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
  third_party/libprocess/include/process/statistics.hpp faefad2e374e08e4e0f88750c4c4eee74bce62d7 
  third_party/libprocess/src/statistics.cpp 29d08f05c6ab7b69d587a44aab860b4dfe645b89 
  third_party/libprocess/src/tests/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 

Diff: https://reviews.apache.org/r/9094/diff/


Testing
-------

make check

Although I didn't add tests, I've manually tested end-to-end with my future reviews.


Thanks,

Ben Mahler

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Ben Mahler <be...@gmail.com>.

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

(Updated Feb. 22, 2013, 12:32 a.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Benh review.
Rebased off trunk.


Description
-------

This is the meat of the resource monitoring design.

-Added a ResourceUsage protobuf.
-Added a ResourceMonitor process that periodically hits the isolation module for usage information.
-Usage is exported to the Statistics process.
-Usage is available via a JSON endpoint.

Implementation of the isolation module code will follow in subsequent reviews.


This addresses bug MESOS-324.
    https://issues.apache.org/jira/browse/MESOS-324


Diffs (updated)
-----

  include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
  include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
  src/Makefile.am c94736df660a25b58dc47c07d9c56c3c26152a66 
  src/files/files.cpp 84082ce6dab6c70bb21cd290119cbdd0169be29d 
  src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
  src/slave/cgroups_isolation_module.cpp a2eba6f96f5d8a4b1257571aa29e37c5682aab8d 
  src/slave/flags.hpp 39e57f4104ee7a1538436ebbb9493581e28c99dd 
  src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
  src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
  src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
  src/slave/monitor.hpp PRE-CREATION 
  src/slave/monitor.cpp PRE-CREATION 
  src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
  src/slave/process_based_isolation_module.cpp 12a579cba56cd3dac384bc7919b0d5537b0e429d 
  src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
  src/slave/slave.cpp d4721c3eb51db87278d05f6fbe2eadb8a3a9b4dd 
  src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
  third_party/libprocess/include/process/statistics.hpp faefad2e374e08e4e0f88750c4c4eee74bce62d7 
  third_party/libprocess/src/statistics.cpp 29d08f05c6ab7b69d587a44aab860b4dfe645b89 
  third_party/libprocess/src/tests/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 

Diff: https://reviews.apache.org/r/9094/diff/


Testing
-------

make check

Although I didn't add tests, I've manually tested end-to-end with my future reviews.


Thanks,

Ben Mahler

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Ben Mahler <be...@gmail.com>.

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



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment35730>

    s/usage/statistics here and elswhere


- Ben Mahler


On Feb. 13, 2013, 2:44 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9094/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2013, 2:44 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This is the meat of the resource monitoring design.
> 
> -Added a ResourceUsage protobuf.
> -Added a ResourceMonitor process that periodically hits the isolation module for usage information.
> -Usage is exported to the Statistics process.
> -Usage is available via a JSON endpoint.
> 
> Implementation of the isolation module code will follow in subsequent reviews.
> 
> 
> This addresses bug MESOS-324.
>     https://issues.apache.org/jira/browse/MESOS-324
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
>   include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
>   src/Makefile.am c94736df660a25b58dc47c07d9c56c3c26152a66 
>   src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
>   src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
>   src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
>   src/slave/flags.hpp 39e57f4104ee7a1538436ebbb9493581e28c99dd 
>   src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
>   src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
>   src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
>   src/slave/monitor.hpp PRE-CREATION 
>   src/slave/monitor.cpp PRE-CREATION 
>   src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
>   src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
>   src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
>   src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
>   src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
>   third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
>   third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
>   third_party/libprocess/src/tests/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 
> 
> Diff: https://reviews.apache.org/r/9094/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Although I didn't add tests, I've manually tested end-to-end with my future reviews.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Benjamin Hindman <be...@berkeley.edu>.

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

Ship it!



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment35729>

    &?



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment35728>

    You could always delay before you do anything else with a comment saying "we should always collect future statistics even if this one failed".


- Benjamin Hindman


On Feb. 13, 2013, 2:44 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9094/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2013, 2:44 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This is the meat of the resource monitoring design.
> 
> -Added a ResourceUsage protobuf.
> -Added a ResourceMonitor process that periodically hits the isolation module for usage information.
> -Usage is exported to the Statistics process.
> -Usage is available via a JSON endpoint.
> 
> Implementation of the isolation module code will follow in subsequent reviews.
> 
> 
> This addresses bug MESOS-324.
>     https://issues.apache.org/jira/browse/MESOS-324
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
>   include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
>   src/Makefile.am c94736df660a25b58dc47c07d9c56c3c26152a66 
>   src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
>   src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
>   src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
>   src/slave/flags.hpp 39e57f4104ee7a1538436ebbb9493581e28c99dd 
>   src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
>   src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
>   src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
>   src/slave/monitor.hpp PRE-CREATION 
>   src/slave/monitor.cpp PRE-CREATION 
>   src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
>   src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
>   src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
>   src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
>   src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
>   third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
>   third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
>   third_party/libprocess/src/tests/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 
> 
> Diff: https://reviews.apache.org/r/9094/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Although I didn't add tests, I've manually tested end-to-end with my future reviews.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Benjamin Hindman <be...@berkeley.edu>.

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



third_party/libprocess/src/tests/statistics_tests.cpp
<https://reviews.apache.org/r/9094/#comment35805>

    s/timeseries/time series/g


- Benjamin Hindman


On Feb. 13, 2013, 2:44 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9094/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2013, 2:44 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This is the meat of the resource monitoring design.
> 
> -Added a ResourceUsage protobuf.
> -Added a ResourceMonitor process that periodically hits the isolation module for usage information.
> -Usage is exported to the Statistics process.
> -Usage is available via a JSON endpoint.
> 
> Implementation of the isolation module code will follow in subsequent reviews.
> 
> 
> This addresses bug MESOS-324.
>     https://issues.apache.org/jira/browse/MESOS-324
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
>   include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
>   src/Makefile.am c94736df660a25b58dc47c07d9c56c3c26152a66 
>   src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
>   src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
>   src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
>   src/slave/flags.hpp 39e57f4104ee7a1538436ebbb9493581e28c99dd 
>   src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
>   src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
>   src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
>   src/slave/monitor.hpp PRE-CREATION 
>   src/slave/monitor.cpp PRE-CREATION 
>   src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
>   src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
>   src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
>   src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
>   src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
>   third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
>   third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
>   third_party/libprocess/src/tests/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 
> 
> Diff: https://reviews.apache.org/r/9094/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Although I didn't add tests, I've manually tested end-to-end with my future reviews.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Ben Mahler <be...@gmail.com>.

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

(Updated Feb. 13, 2013, 2:44 a.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

This is updated to be more conducive to resource usage propagating to the master / scheduler.
  -the protos have been re-structured
  -the monitor now watches / unwatches (allows the one-time setup of the cpu usage meter and per-executor collection)
  -the isolation module returns usage for a particular executor


Description
-------

This is the meat of the resource monitoring design.

-Added a ResourceUsage protobuf.
-Added a ResourceMonitor process that periodically hits the isolation module for usage information.
-Usage is exported to the Statistics process.
-Usage is available via a JSON endpoint.

Implementation of the isolation module code will follow in subsequent reviews.


This addresses bug MESOS-324.
    https://issues.apache.org/jira/browse/MESOS-324


Diffs (updated)
-----

  include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
  include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
  src/Makefile.am c94736df660a25b58dc47c07d9c56c3c26152a66 
  src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
  src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
  src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
  src/slave/flags.hpp 39e57f4104ee7a1538436ebbb9493581e28c99dd 
  src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
  src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
  src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
  src/slave/monitor.hpp PRE-CREATION 
  src/slave/monitor.cpp PRE-CREATION 
  src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
  src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
  src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
  src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
  src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
  third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
  third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
  third_party/libprocess/src/tests/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 

Diff: https://reviews.apache.org/r/9094/diff/


Testing
-------

make check

Although I didn't add tests, I've manually tested end-to-end with my future reviews.


Thanks,

Ben Mahler

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Ben Mahler <be...@gmail.com>.


> On Feb. 7, 2013, 10:39 p.m., Benjamin Hindman wrote:
> > include/mesos/mesos.proto, lines 251-252
> > <https://reviews.apache.org/r/9094/diff/5/?file=254441#file254441line251>
> >
> >     Are these total times or since the last snapshot? If so, what's the interval or do I need to have a previous ResourceUsage to see it's timestamp?

These are the total times, I'll document that so that it's clear.


> On Feb. 7, 2013, 10:39 p.m., Benjamin Hindman wrote:
> > src/files/files.cpp, line 104
> > <https://reviews.apache.org/r/9094/diff/5/?file=254443#file254443line104>
> >
> >     Yeah, I don't think we want this to propagate.

I'll remove the CHECKs since we can think about this later, but we should definitely be looking at the result of route!


> On Feb. 7, 2013, 10:39 p.m., Benjamin Hindman wrote:
> > src/slave/isolation_module.hpp, line 22
> > <https://reviews.apache.org/r/9094/diff/5/?file=254448#file254448line22>
> >
> >     Just curious, what did you need this for? Maybe a comment?

Artifact.


> On Feb. 7, 2013, 10:39 p.m., Benjamin Hindman wrote:
> > src/slave/monitor.cpp, line 83
> > <https://reviews.apache.org/r/9094/diff/5/?file=254452#file254452line83>
> >
> >     Maybe it should propagate until we fix it?

I'm not going to propagate it through this review, but it's definitely something we should figure out.


> On Feb. 7, 2013, 10:39 p.m., Benjamin Hindman wrote:
> > third_party/libprocess/src/statistics.cpp, lines 343-348
> > <https://reviews.apache.org/r/9094/diff/5/?file=254459#file254459line343>
> >
> >     How does the added output help us debug? IIUC, if one of these CHECKs fires there is a serious bug, regardless of the context and name. Am I missing something?

Removed. I had added these when trying to fix a bug uncovered by my tests.


> On Feb. 7, 2013, 10:39 p.m., Benjamin Hindman wrote:
> > src/slave/monitor.cpp, lines 161-162
> > <https://reviews.apache.org/r/9094/diff/5/?file=254452#file254452line161>
> >
> >     A comment on why you're not using const& here would be nice.

I'm going to just bite the bullet and use const& here at the cost of the code being a bit uglier.


> On Feb. 7, 2013, 10:39 p.m., Benjamin Hindman wrote:
> > src/slave/slave.cpp, line 318
> > <https://reviews.apache.org/r/9094/diff/5/?file=254456#file254456line318>
> >
> >     This needs to be a flag (you can keep the constant if you like, but we should really only be referencing 'flag.resource_collection_interval'). In the long term we probably don't need to put constants that are only being referenced by flags in constants.hpp/cpp since it might be a source of confusion.

Done, this is now a slave flag, and the constant is limited to the flag file itself rather than constants.hpp.


> On Feb. 7, 2013, 10:39 p.m., Benjamin Hindman wrote:
> > src/slave/monitor.cpp, line 160
> > <https://reviews.apache.org/r/9094/diff/5/?file=254452#file254452line160>
> >
> >     So I get that you don't need this to be a method of ResourceMonitorProcess, but I've tended to like putting the continuation function immediately after the other function for readability reasons. That might mean having a declaration at the top of the file in order to reference it in ResourceMonitorProcess::usage.


> On Feb. 7, 2013, 10:39 p.m., Benjamin Hindman wrote:
> > src/slave/monitor.cpp, line 65
> > <https://reviews.apache.org/r/9094/diff/5/?file=254452#file254452line65>
> >
> >     I like the notion of the 'expose' verb, but I feel like I read it better as 'export'. The former makes it seem like I'm trying to show something, the latter makes it seem like I'm trying to ship it somewhere (as in, ship it over to statistics). Also, this is kind of like a macro, and the implementation doesn't look like it needs to be a method but could just be a function.

I agree and originally named this 'export' as well, but 'export' is a reserved keyword! So expose was the best synonym I could think of.

Made this a function instead.


- Ben


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


On Feb. 1, 2013, 1:12 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9094/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2013, 1:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This is the meat of the resource monitoring design.
> 
> -Added a ResourceUsage protobuf.
> -Added a ResourceMonitor process that periodically hits the isolation module for usage information.
> -Usage is exported to the Statistics process.
> -Usage is available via a JSON endpoint.
> 
> Implementation of the isolation module code will follow in subsequent reviews.
> 
> 
> This addresses bug MESOS-324.
>     https://issues.apache.org/jira/browse/MESOS-324
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
>   include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
>   src/Makefile.am c94736df660a25b58dc47c07d9c56c3c26152a66 
>   src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
>   src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
>   src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
>   src/slave/constants.hpp ddf02570caf3793106b3c48e158a5bb48c1ae80c 
>   src/slave/constants.cpp 1735a6b55a93e6537a5a119e5345961f3d84a000 
>   src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
>   src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
>   src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
>   src/slave/monitor.hpp PRE-CREATION 
>   src/slave/monitor.cpp PRE-CREATION 
>   src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
>   src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
>   src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
>   src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
>   src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
>   third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
>   third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
>   third_party/libprocess/src/tests/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 
> 
> Diff: https://reviews.apache.org/r/9094/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Although I didn't add tests, I've manually tested end-to-end with my future reviews.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Benjamin Hindman <be...@berkeley.edu>.


> On Feb. 7, 2013, 10:39 p.m., Benjamin Hindman wrote:
> > src/slave/monitor.cpp, line 65
> > <https://reviews.apache.org/r/9094/diff/5/?file=254452#file254452line65>
> >
> >     I like the notion of the 'expose' verb, but I feel like I read it better as 'export'. The former makes it seem like I'm trying to show something, the latter makes it seem like I'm trying to ship it somewhere (as in, ship it over to statistics). Also, this is kind of like a macro, and the implementation doesn't look like it needs to be a method but could just be a function.
> 
> Ben Mahler wrote:
>     I agree and originally named this 'export' as well, but 'export' is a reserved keyword! So expose was the best synonym I could think of.
>     
>     Made this a function instead.

What about publish!


- Benjamin


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


On Feb. 13, 2013, 2:44 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9094/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2013, 2:44 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This is the meat of the resource monitoring design.
> 
> -Added a ResourceUsage protobuf.
> -Added a ResourceMonitor process that periodically hits the isolation module for usage information.
> -Usage is exported to the Statistics process.
> -Usage is available via a JSON endpoint.
> 
> Implementation of the isolation module code will follow in subsequent reviews.
> 
> 
> This addresses bug MESOS-324.
>     https://issues.apache.org/jira/browse/MESOS-324
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
>   include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
>   src/Makefile.am c94736df660a25b58dc47c07d9c56c3c26152a66 
>   src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
>   src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
>   src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
>   src/slave/flags.hpp 39e57f4104ee7a1538436ebbb9493581e28c99dd 
>   src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
>   src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
>   src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
>   src/slave/monitor.hpp PRE-CREATION 
>   src/slave/monitor.cpp PRE-CREATION 
>   src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
>   src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
>   src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
>   src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
>   src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
>   third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
>   third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
>   third_party/libprocess/src/tests/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 
> 
> Diff: https://reviews.apache.org/r/9094/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Although I didn't add tests, I've manually tested end-to-end with my future reviews.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Ben Mahler <be...@gmail.com>.


> On Feb. 7, 2013, 10:39 p.m., Benjamin Hindman wrote:
> > src/slave/monitor.cpp, line 65
> > <https://reviews.apache.org/r/9094/diff/5/?file=254452#file254452line65>
> >
> >     I like the notion of the 'expose' verb, but I feel like I read it better as 'export'. The former makes it seem like I'm trying to show something, the latter makes it seem like I'm trying to ship it somewhere (as in, ship it over to statistics). Also, this is kind of like a macro, and the implementation doesn't look like it needs to be a method but could just be a function.
> 
> Ben Mahler wrote:
>     I agree and originally named this 'export' as well, but 'export' is a reserved keyword! So expose was the best synonym I could think of.
>     
>     Made this a function instead.
> 
> Benjamin Hindman wrote:
>     What about publish!

publish it is!


- Ben


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


On Feb. 13, 2013, 2:44 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9094/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2013, 2:44 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This is the meat of the resource monitoring design.
> 
> -Added a ResourceUsage protobuf.
> -Added a ResourceMonitor process that periodically hits the isolation module for usage information.
> -Usage is exported to the Statistics process.
> -Usage is available via a JSON endpoint.
> 
> Implementation of the isolation module code will follow in subsequent reviews.
> 
> 
> This addresses bug MESOS-324.
>     https://issues.apache.org/jira/browse/MESOS-324
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
>   include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
>   src/Makefile.am c94736df660a25b58dc47c07d9c56c3c26152a66 
>   src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
>   src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
>   src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
>   src/slave/flags.hpp 39e57f4104ee7a1538436ebbb9493581e28c99dd 
>   src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
>   src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
>   src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
>   src/slave/monitor.hpp PRE-CREATION 
>   src/slave/monitor.cpp PRE-CREATION 
>   src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
>   src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
>   src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
>   src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
>   src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
>   third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
>   third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
>   third_party/libprocess/src/tests/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 
> 
> Diff: https://reviews.apache.org/r/9094/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Although I didn't add tests, I've manually tested end-to-end with my future reviews.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Benjamin Hindman <be...@berkeley.edu>.

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



include/mesos/mesos.proto
<https://reviews.apache.org/r/9094/#comment34743>

    ... since the Epoch.



include/mesos/mesos.proto
<https://reviews.apache.org/r/9094/#comment34744>

    Are these total times or since the last snapshot? If so, what's the interval or do I need to have a previous ResourceUsage to see it's timestamp?



src/files/files.cpp
<https://reviews.apache.org/r/9094/#comment34745>

    Yeah, I don't think we want this to propagate.



src/slave/isolation_module.hpp
<https://reviews.apache.org/r/9094/#comment34748>

    Just curious, what did you need this for? Maybe a comment?



src/slave/isolation_module.hpp
<https://reviews.apache.org/r/9094/#comment34749>

    Formatting.



src/slave/monitor.hpp
<https://reviews.apache.org/r/9094/#comment34753>

    const&



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment34759>

    const&



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment34760>

    Yeah const&! ;)



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment34763>

    I like the notion of the 'expose' verb, but I feel like I read it better as 'export'. The former makes it seem like I'm trying to show something, the latter makes it seem like I'm trying to ship it somewhere (as in, ship it over to statistics). Also, this is kind of like a macro, and the implementation doesn't look like it needs to be a method but could just be a function.



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment34755>

    Maybe it should propagate until we fix it?



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment34762>

    So I get that you don't need this to be a method of ResourceMonitorProcess, but I've tended to like putting the continuation function immediately after the other function for readability reasons. That might mean having a declaration at the top of the file in order to reference it in ResourceMonitorProcess::usage.



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment34761>

    A comment on why you're not using const& here would be nice.



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment34764>

    I wonder if it makes sense to have the names in ResourceUsage map here? Perhaps we change the protobuf to have a 'memory_rss' field rather than just an 'rss' field?



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment34757>

    s/buildJson/_usage/
    
    Given our style of the prefixed '_' for continuations, this reads more clearly below that this is the continuation.
    
    Also, you used lambda earlier, and now std::tr1?



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment34758>

    Ditto, lambda::_1 early and now std::tr1::placeholders::_1?



src/slave/slave.cpp
<https://reviews.apache.org/r/9094/#comment34747>

    This needs to be a flag (you can keep the constant if you like, but we should really only be referencing 'flag.resource_collection_interval'). In the long term we probably don't need to put constants that are only being referenced by flags in constants.hpp/cpp since it might be a source of confusion.



third_party/libprocess/src/statistics.cpp
<https://reviews.apache.org/r/9094/#comment34766>

    How does the added output help us debug? IIUC, if one of these CHECKs fires there is a serious bug, regardless of the context and name. Am I missing something?


- Benjamin Hindman


On Feb. 1, 2013, 1:12 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9094/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2013, 1:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This is the meat of the resource monitoring design.
> 
> -Added a ResourceUsage protobuf.
> -Added a ResourceMonitor process that periodically hits the isolation module for usage information.
> -Usage is exported to the Statistics process.
> -Usage is available via a JSON endpoint.
> 
> Implementation of the isolation module code will follow in subsequent reviews.
> 
> 
> This addresses bug MESOS-324.
>     https://issues.apache.org/jira/browse/MESOS-324
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
>   include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
>   src/Makefile.am c94736df660a25b58dc47c07d9c56c3c26152a66 
>   src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
>   src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
>   src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
>   src/slave/constants.hpp ddf02570caf3793106b3c48e158a5bb48c1ae80c 
>   src/slave/constants.cpp 1735a6b55a93e6537a5a119e5345961f3d84a000 
>   src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
>   src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
>   src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
>   src/slave/monitor.hpp PRE-CREATION 
>   src/slave/monitor.cpp PRE-CREATION 
>   src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
>   src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
>   src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
>   src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
>   src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
>   third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
>   third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
>   third_party/libprocess/src/tests/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 
> 
> Diff: https://reviews.apache.org/r/9094/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Although I didn't add tests, I've manually tested end-to-end with my future reviews.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Ben Mahler <be...@gmail.com>.

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

(Updated Feb. 1, 2013, 1:12 a.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Vinod's review.


Description
-------

This is the meat of the resource monitoring design.

-Added a ResourceUsage protobuf.
-Added a ResourceMonitor process that periodically hits the isolation module for usage information.
-Usage is exported to the Statistics process.
-Usage is available via a JSON endpoint.

Implementation of the isolation module code will follow in subsequent reviews.


This addresses bug MESOS-324.
    https://issues.apache.org/jira/browse/MESOS-324


Diffs (updated)
-----

  include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
  include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
  src/Makefile.am c94736df660a25b58dc47c07d9c56c3c26152a66 
  src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
  src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
  src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
  src/slave/constants.hpp ddf02570caf3793106b3c48e158a5bb48c1ae80c 
  src/slave/constants.cpp 1735a6b55a93e6537a5a119e5345961f3d84a000 
  src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
  src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
  src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
  src/slave/monitor.hpp PRE-CREATION 
  src/slave/monitor.cpp PRE-CREATION 
  src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
  src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
  src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
  src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
  src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
  third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
  third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
  third_party/libprocess/src/tests/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 

Diff: https://reviews.apache.org/r/9094/diff/


Testing
-------

make check

Although I didn't add tests, I've manually tested end-to-end with my future reviews.


Thanks,

Ben Mahler

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Ben Mahler <be...@gmail.com>.

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

(Updated Jan. 31, 2013, 6:17 p.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Vinod's review.


Description
-------

This is the meat of the resource monitoring design.

-Added a ResourceUsage protobuf.
-Added a ResourceMonitor process that periodically hits the isolation module for usage information.
-Usage is exported to the Statistics process.
-Usage is available via a JSON endpoint.

Implementation of the isolation module code will follow in subsequent reviews.


This addresses bug MESOS-324.
    https://issues.apache.org/jira/browse/MESOS-324


Diffs (updated)
-----

  include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
  include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
  src/Makefile.am 0ab59b75b2955c532d0833f132bdaffe323838d0 
  src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
  src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
  src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
  src/slave/constants.hpp ddf02570caf3793106b3c48e158a5bb48c1ae80c 
  src/slave/constants.cpp 1735a6b55a93e6537a5a119e5345961f3d84a000 
  src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
  src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
  src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
  src/slave/monitor.hpp PRE-CREATION 
  src/slave/monitor.cpp PRE-CREATION 
  src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
  src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
  src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
  src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
  src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
  third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
  third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
  third_party/libprocess/src/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 

Diff: https://reviews.apache.org/r/9094/diff/


Testing
-------

make check

Although I didn't add tests, I've manually tested end-to-end with my future reviews.


Thanks,

Ben Mahler

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Ben Mahler <be...@gmail.com>.

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

(Updated Jan. 30, 2013, 4:01 a.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Fixed some strange issues with foreachkey not iterating over the executors.


Description
-------

This is the meat of the resource monitoring design.

-Added a ResourceUsage protobuf.
-Added a ResourceMonitor process that periodically hits the isolation module for usage information.
-Usage is exported to the Statistics process.
-Usage is available via a JSON endpoint.

Implementation of the isolation module code will follow in subsequent reviews.


This addresses bug MESOS-324.
    https://issues.apache.org/jira/browse/MESOS-324


Diffs (updated)
-----

  include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
  include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
  src/Makefile.am 0ab59b75b2955c532d0833f132bdaffe323838d0 
  src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
  src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
  src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
  src/slave/constants.hpp ddf02570caf3793106b3c48e158a5bb48c1ae80c 
  src/slave/constants.cpp 1735a6b55a93e6537a5a119e5345961f3d84a000 
  src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
  src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
  src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
  src/slave/monitor.hpp PRE-CREATION 
  src/slave/monitor.cpp PRE-CREATION 
  src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
  src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
  src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
  src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
  src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
  third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
  third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
  third_party/libprocess/src/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 

Diff: https://reviews.apache.org/r/9094/diff/


Testing
-------

make check

Although I didn't add tests, I've manually tested end-to-end with my future reviews.


Thanks,

Ben Mahler

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Ben Mahler <be...@gmail.com>.

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

(Updated Jan. 29, 2013, 1:38 a.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Vinod's review.


Description
-------

This is the meat of the resource monitoring design.

-Added a ResourceUsage protobuf.
-Added a ResourceMonitor process that periodically hits the isolation module for usage information.
-Usage is exported to the Statistics process.
-Usage is available via a JSON endpoint.

Implementation of the isolation module code will follow in subsequent reviews.


This addresses bug MESOS-324.
    https://issues.apache.org/jira/browse/MESOS-324


Diffs (updated)
-----

  include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
  include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
  src/Makefile.am 0ab59b75b2955c532d0833f132bdaffe323838d0 
  src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
  src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
  src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
  src/slave/constants.hpp ddf02570caf3793106b3c48e158a5bb48c1ae80c 
  src/slave/constants.cpp 1735a6b55a93e6537a5a119e5345961f3d84a000 
  src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
  src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
  src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
  src/slave/monitor.hpp PRE-CREATION 
  src/slave/monitor.cpp PRE-CREATION 
  src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
  src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
  src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
  src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
  src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
  third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
  third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
  third_party/libprocess/src/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 

Diff: https://reviews.apache.org/r/9094/diff/


Testing
-------

make check

Although I didn't add tests, I've manually tested end-to-end with my future reviews.


Thanks,

Ben Mahler

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Vinod Kone <vi...@gmail.com>.


> On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote:
> > src/slave/monitor.cpp, lines 142-149
> > <https://reviews.apache.org/r/9094/diff/1/?file=251548#file251548line142>
> >
> >     Swap these two to match the doc?
> 
> Ben Mahler wrote:
>     I first apply the meter to track the stat, so that the first value is given to the meter.
>     If I meter afterwards, then the meter won't apply until the second value arrives.

This subtlety wasn't obvious. May be a comment here.

More importantly, what semantics do you want when someone adds a meter to a stat? Would it be preferable to meter to the whole time series rather than just the future values? I'm not sure, but good to think about.


> On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote:
> > src/slave/monitor.cpp, lines 154-155
> > <https://reviews.apache.org/r/9094/diff/1/?file=251548#file251548line154>
> >
> >     const &
> 
> Ben Mahler wrote:
>     They can't be const& because I'm using the [] operators, I tried to avoid the [] operators but the resulting code is considerably more clunky.

i guess you are going to use const now.


> On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote:
> > src/slave/monitor.cpp, lines 167-169
> > <https://reviews.apache.org/r/9094/diff/1/?file=251548#file251548line167>
> >
> >     Why are the "="s aligned here and not below?
> 
> Ben Mahler wrote:
>     Hm.. I think because the right hand sides are so closely related: all pulling from statistics, the similar prefix operation.
>     
>     I tried aligning the other bit below but it doesn't seem to help much, since the right hand sides below are not related.

definitely do not align the "+"s?


> On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote:
> > src/slave/monitor.cpp, line 185
> > <https://reviews.apache.org/r/9094/diff/1/?file=251548#file251548line185>
> >
> >     I would put this function before _usage().
> 
> Ben Mahler wrote:
>     That would require a prototype of the function since usage needs _usage.

aha. i see that _usage is not a method on the class.


- Vinod


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


On Jan. 29, 2013, 1:38 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9094/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2013, 1:38 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This is the meat of the resource monitoring design.
> 
> -Added a ResourceUsage protobuf.
> -Added a ResourceMonitor process that periodically hits the isolation module for usage information.
> -Usage is exported to the Statistics process.
> -Usage is available via a JSON endpoint.
> 
> Implementation of the isolation module code will follow in subsequent reviews.
> 
> 
> This addresses bug MESOS-324.
>     https://issues.apache.org/jira/browse/MESOS-324
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
>   include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
>   src/Makefile.am 0ab59b75b2955c532d0833f132bdaffe323838d0 
>   src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
>   src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
>   src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
>   src/slave/constants.hpp ddf02570caf3793106b3c48e158a5bb48c1ae80c 
>   src/slave/constants.cpp 1735a6b55a93e6537a5a119e5345961f3d84a000 
>   src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
>   src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
>   src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
>   src/slave/monitor.hpp PRE-CREATION 
>   src/slave/monitor.cpp PRE-CREATION 
>   src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
>   src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
>   src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
>   src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
>   src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
>   third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
>   third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
>   third_party/libprocess/src/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 
> 
> Diff: https://reviews.apache.org/r/9094/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Although I didn't add tests, I've manually tested end-to-end with my future reviews.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Ben Mahler <be...@gmail.com>.


> On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote:
> > src/slave/monitor.cpp, lines 109-110
> > <https://reviews.apache.org/r/9094/diff/1/?file=251548#file251548line109>
> >
> >     Why not use foreachpair() above?

I wanted to, but it seems theres an issue with the macro, when dealing with commas in templates.


> On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote:
> > src/slave/monitor.cpp, lines 167-169
> > <https://reviews.apache.org/r/9094/diff/1/?file=251548#file251548line167>
> >
> >     Why are the "="s aligned here and not below?

Hm.. I think because the right hand sides are so closely related: all pulling from statistics, the similar prefix operation.

I tried aligning the other bit below but it doesn't seem to help much, since the right hand sides below are not related.


> On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote:
> > src/slave/monitor.cpp, line 185
> > <https://reviews.apache.org/r/9094/diff/1/?file=251548#file251548line185>
> >
> >     I would put this function before _usage().

That would require a prototype of the function since usage needs _usage.


> On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote:
> > src/slave/slave.cpp, line 75
> > <https://reviews.apache.org/r/9094/diff/1/?file=251552#file251552line75>
> >
> >     woah..magic!

;)


> On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote:
> > third_party/libprocess/src/statistics.cpp, line 106
> > <https://reviews.apache.org/r/9094/diff/1/?file=251555#file251555line106>
> >
> >     I'm confused. Why does this get() return a Future, but not the next get() and timeseries()?

Because I needed to represent the missing case.

Here when stats[context][name] is empty I return a failed future.
Whereas since the others return maps, an empty map is equivalent to the stats[context] or stats[context][name] being empty.

The alternative here was to return an Option<double> instead of a Future<double>, but that would bubble up to returning a Future<Option<double>> instead of just a Future<double>.


> On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote:
> > src/slave/monitor.cpp, lines 154-155
> > <https://reviews.apache.org/r/9094/diff/1/?file=251548#file251548line154>
> >
> >     const &

They can't be const& because I'm using the [] operators, I tried to avoid the [] operators but the resulting code is considerably more clunky.


> On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote:
> > src/slave/monitor.cpp, lines 142-149
> > <https://reviews.apache.org/r/9094/diff/1/?file=251548#file251548line142>
> >
> >     Swap these two to match the doc?

I first apply the meter to track the stat, so that the first value is given to the meter.
If I meter afterwards, then the meter won't apply until the second value arrives.


- Ben


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


On Jan. 24, 2013, 9:21 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9094/
> -----------------------------------------------------------
> 
> (Updated Jan. 24, 2013, 9:21 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This is the meat of the resource monitoring design.
> 
> -Added a ResourceUsage protobuf.
> -Added a ResourceMonitor process that periodically hits the isolation module for usage information.
> -Usage is exported to the Statistics process.
> -Usage is available via a JSON endpoint.
> 
> Implementation of the isolation module code will follow in subsequent reviews.
> 
> 
> This addresses bug MESOS-324.
>     https://issues.apache.org/jira/browse/MESOS-324
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
>   include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
>   src/Makefile.am 0ab59b75b2955c532d0833f132bdaffe323838d0 
>   src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
>   src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
>   src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
>   src/slave/constants.hpp ddf02570caf3793106b3c48e158a5bb48c1ae80c 
>   src/slave/constants.cpp 1735a6b55a93e6537a5a119e5345961f3d84a000 
>   src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
>   src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
>   src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
>   src/slave/monitor.hpp PRE-CREATION 
>   src/slave/monitor.cpp PRE-CREATION 
>   src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
>   src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
>   src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
>   src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
>   src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
>   third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
>   third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
>   third_party/libprocess/src/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 
> 
> Diff: https://reviews.apache.org/r/9094/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Although I didn't add tests, I've manually tested end-to-end with my future reviews.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Ben Mahler <be...@gmail.com>.


> On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote:
> > src/slave/monitor.cpp, lines 142-149
> > <https://reviews.apache.org/r/9094/diff/1/?file=251548#file251548line142>
> >
> >     Swap these two to match the doc?
> 
> Ben Mahler wrote:
>     I first apply the meter to track the stat, so that the first value is given to the meter.
>     If I meter afterwards, then the meter won't apply until the second value arrives.
> 
> Vinod Kone wrote:
>     This subtlety wasn't obvious. May be a comment here.
>     
>     More importantly, what semantics do you want when someone adds a meter to a stat? Would it be preferable to meter to the whole time series rather than just the future values? I'm not sure, but good to think about.

Added a comment.

Yeah this is definitely a design decision, metering the whole timeseries:
  -Makes meter() potentially a heavy call, whereas I wanted to keep most statistics calls very lightweight (given how I'd like to plumb stats into libprocess).

I think for that reason I avoided metering previous values (especially since right now we store unlimited values inside of the time window!).


> On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote:
> > src/slave/monitor.cpp, lines 154-155
> > <https://reviews.apache.org/r/9094/diff/1/?file=251548#file251548line154>
> >
> >     const &
> 
> Ben Mahler wrote:
>     They can't be const& because I'm using the [] operators, I tried to avoid the [] operators but the resulting code is considerably more clunky.
> 
> Vinod Kone wrote:
>     i guess you are going to use const now.

yep ;)


- Ben


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


On Jan. 30, 2013, 4:01 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9094/
> -----------------------------------------------------------
> 
> (Updated Jan. 30, 2013, 4:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This is the meat of the resource monitoring design.
> 
> -Added a ResourceUsage protobuf.
> -Added a ResourceMonitor process that periodically hits the isolation module for usage information.
> -Usage is exported to the Statistics process.
> -Usage is available via a JSON endpoint.
> 
> Implementation of the isolation module code will follow in subsequent reviews.
> 
> 
> This addresses bug MESOS-324.
>     https://issues.apache.org/jira/browse/MESOS-324
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
>   include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
>   src/Makefile.am 0ab59b75b2955c532d0833f132bdaffe323838d0 
>   src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
>   src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
>   src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
>   src/slave/constants.hpp ddf02570caf3793106b3c48e158a5bb48c1ae80c 
>   src/slave/constants.cpp 1735a6b55a93e6537a5a119e5345961f3d84a000 
>   src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
>   src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
>   src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
>   src/slave/monitor.hpp PRE-CREATION 
>   src/slave/monitor.cpp PRE-CREATION 
>   src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
>   src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
>   src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
>   src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
>   src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
>   third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
>   third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
>   third_party/libprocess/src/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 
> 
> Diff: https://reviews.apache.org/r/9094/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Although I didn't add tests, I've manually tested end-to-end with my future reviews.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request: Resource Monitoring 6: Added ResourceMonitor and updated the isolation API.

Posted by Vinod Kone <vi...@gmail.com>.

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



include/mesos/mesos.proto
<https://reviews.apache.org/r/9094/#comment33948>

    s/(s)//



include/mesos/mesos.proto
<https://reviews.apache.org/r/9094/#comment33947>

    s/;/./



include/mesos/mesos.proto
<https://reviews.apache.org/r/9094/#comment33946>

    s/. Akin/, akin/



src/slave/isolation_module.hpp
<https://reviews.apache.org/r/9094/#comment33949>

    unused?



src/slave/monitor.hpp
<https://reviews.apache.org/r/9094/#comment33950>

    new line



src/slave/monitor.hpp
<https://reviews.apache.org/r/9094/#comment33951>

    reorder



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment33954>

    Why not use foreachpair() above?



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment33956>

    Swap these two to match the doc?



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment33958>

    const &



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment33959>

    Why are the "="s aligned here and not below?



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment33960>

    I would put this function before _usage().



src/slave/monitor.cpp
<https://reviews.apache.org/r/9094/#comment33962>

    reorder



src/slave/slave.cpp
<https://reviews.apache.org/r/9094/#comment33963>

    woah..magic!



third_party/libprocess/src/statistics.cpp
<https://reviews.apache.org/r/9094/#comment33964>

    I'm confused. Why does this get() return a Future, but not the next get() and timeseries()?


- Vinod Kone


On Jan. 24, 2013, 9:21 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9094/
> -----------------------------------------------------------
> 
> (Updated Jan. 24, 2013, 9:21 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This is the meat of the resource monitoring design.
> 
> -Added a ResourceUsage protobuf.
> -Added a ResourceMonitor process that periodically hits the isolation module for usage information.
> -Usage is exported to the Statistics process.
> -Usage is available via a JSON endpoint.
> 
> Implementation of the isolation module code will follow in subsequent reviews.
> 
> 
> This addresses bug MESOS-324.
>     https://issues.apache.org/jira/browse/MESOS-324
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 
>   include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 
>   src/Makefile.am 0ab59b75b2955c532d0833f132bdaffe323838d0 
>   src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
>   src/slave/cgroups_isolation_module.hpp 669efa14ba2603764aa68ae19a44e79dbfdec192 
>   src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36 
>   src/slave/constants.hpp ddf02570caf3793106b3c48e158a5bb48c1ae80c 
>   src/slave/constants.cpp 1735a6b55a93e6537a5a119e5345961f3d84a000 
>   src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 
>   src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 
>   src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c 
>   src/slave/monitor.hpp PRE-CREATION 
>   src/slave/monitor.cpp PRE-CREATION 
>   src/slave/process_based_isolation_module.hpp f1817192582e3646f8dcf17934ba7998829e8fd6 
>   src/slave/process_based_isolation_module.cpp 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 
>   src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 
>   src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 
>   src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 
>   third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 
>   third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f 
>   third_party/libprocess/src/statistics_tests.cpp 0aaab3526618171c7cfbd11d40d614344bcbfd0a 
> 
> Diff: https://reviews.apache.org/r/9094/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Although I didn't add tests, I've manually tested end-to-end with my future reviews.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>