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/08/16 00:14:07 UTC

Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

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

Review request for mesos, Benjamin Hindman and Vinod Kone.


Repository: mesos-git


Description
-------

This alters statistics.json to return instantaneous resource consumption information.

Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.


Diffs
-----

  src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
  src/slave/monitor.cpp 8e1eb353c55f9a04b07c382c10c669715a2d7ac2 
  src/tests/monitor_tests.cpp 31424160b0493ef37705be25cd97f74e027d85ef 

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


Testing
-------

Split the monitoring tests into two tests:

1. Tests that verify the periodic collection is working correctly.
2. Tests that use the instantaneous statistics.json endpoint.


Thanks,

Ben Mahler


Re: Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

Posted by Chi Zhang <ch...@gmail.com>.

> On Oct. 17, 2013, 11:15 p.m., Chi Zhang wrote:
> > src/slave/monitor.cpp, line 183
> > <https://reviews.apache.org/r/13605/diff/1/?file=341588#file341588line183>
> >
> >     What is the reason that __statisticsJSON is not declared as a member function?
> 
> Ben Mahler wrote:
>     Because it does not need to be a member function. (It does not access member variables or call member functions of ResourceMonitorProcess).

Hi Ben,

I don't think those facts make a strong argument in this case. We 'call out' in _statisticsJSON; 'call back' in __statisticsJSON. Together they form an async cycle and complete 'statisticJSON', a functionality of ResourceMonitorProcess. 

Not sure if this rb is still active at this point. I had the question while I was looking at process::await. 


- Chi


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


On Aug. 15, 2013, 10:14 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13605/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2013, 10:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This alters statistics.json to return instantaneous resource consumption information.
> 
> Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.
> 
> 
> Diffs
> -----
> 
>   src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
>   src/slave/monitor.cpp 8e1eb353c55f9a04b07c382c10c669715a2d7ac2 
>   src/tests/monitor_tests.cpp 31424160b0493ef37705be25cd97f74e027d85ef 
> 
> Diff: https://reviews.apache.org/r/13605/diff/
> 
> 
> Testing
> -------
> 
> Split the monitoring tests into two tests:
> 
> 1. Tests that verify the periodic collection is working correctly.
> 2. Tests that use the instantaneous statistics.json endpoint.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

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

> On Oct. 17, 2013, 11:15 p.m., Chi Zhang wrote:
> > src/slave/monitor.cpp, line 183
> > <https://reviews.apache.org/r/13605/diff/1/?file=341588#file341588line183>
> >
> >     What is the reason that __statisticsJSON is not declared as a member function?

Because it does not need to be a member function. (It does not access member variables or call member functions of ResourceMonitorProcess).


- Ben


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


On Aug. 15, 2013, 10:14 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13605/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2013, 10:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This alters statistics.json to return instantaneous resource consumption information.
> 
> Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.
> 
> 
> Diffs
> -----
> 
>   src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
>   src/slave/monitor.cpp 8e1eb353c55f9a04b07c382c10c669715a2d7ac2 
>   src/tests/monitor_tests.cpp 31424160b0493ef37705be25cd97f74e027d85ef 
> 
> Diff: https://reviews.apache.org/r/13605/diff/
> 
> 
> Testing
> -------
> 
> Split the monitoring tests into two tests:
> 
> 1. Tests that verify the periodic collection is working correctly.
> 2. Tests that use the instantaneous statistics.json endpoint.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

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

> On Oct. 17, 2013, 11:15 p.m., Chi Zhang wrote:
> > src/slave/monitor.cpp, line 183
> > <https://reviews.apache.org/r/13605/diff/1/?file=341588#file341588line183>
> >
> >     What is the reason that __statisticsJSON is not declared as a member function?
> 
> Ben Mahler wrote:
>     Because it does not need to be a member function. (It does not access member variables or call member functions of ResourceMonitorProcess).
> 
> Chi Zhang wrote:
>     Hi Ben,
>     
>     I don't think those facts make a strong argument in this case. We 'call out' in _statisticsJSON; 'call back' in __statisticsJSON. Together they form an async cycle and complete 'statisticJSON', a functionality of ResourceMonitorProcess. 
>     
>     Not sure if this rb is still active at this point. I had the question while I was looking at process::await.

Check out _read() in files.cpp for another example of this kind of continuation, I'm not sure I understand what you mean by "async cycle" or completing statisticsJSON, can you clarify?


- Ben


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


On Aug. 15, 2013, 10:14 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13605/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2013, 10:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This alters statistics.json to return instantaneous resource consumption information.
> 
> Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.
> 
> 
> Diffs
> -----
> 
>   src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
>   src/slave/monitor.cpp 8e1eb353c55f9a04b07c382c10c669715a2d7ac2 
>   src/tests/monitor_tests.cpp 31424160b0493ef37705be25cd97f74e027d85ef 
> 
> Diff: https://reviews.apache.org/r/13605/diff/
> 
> 
> Testing
> -------
> 
> Split the monitoring tests into two tests:
> 
> 1. Tests that verify the periodic collection is working correctly.
> 2. Tests that use the instantaneous statistics.json endpoint.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13605/#review27156
-----------------------------------------------------------



src/slave/monitor.cpp
<https://reviews.apache.org/r/13605/#comment52850>

    What is the reason that __statisticsJSON is not declared as a member function?


- Chi Zhang


On Aug. 15, 2013, 10:14 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13605/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2013, 10:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This alters statistics.json to return instantaneous resource consumption information.
> 
> Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.
> 
> 
> Diffs
> -----
> 
>   src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
>   src/slave/monitor.cpp 8e1eb353c55f9a04b07c382c10c669715a2d7ac2 
>   src/tests/monitor_tests.cpp 31424160b0493ef37705be25cd97f74e027d85ef 
> 
> Diff: https://reviews.apache.org/r/13605/diff/
> 
> 
> Testing
> -------
> 
> Split the monitoring tests into two tests:
> 
> 1. Tests that verify the periodic collection is working correctly.
> 2. Tests that use the instantaneous statistics.json endpoint.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13605/
-----------------------------------------------------------

(Updated Jan. 21, 2014, 11:30 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
-------

Benh review.


Repository: mesos-git


Description
-------

This alters statistics.json to return instantaneous resource consumption information.

Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.


Diffs (updated)
-----

  src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
  src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa 
  src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253 

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


Testing
-------

Split the monitoring tests into two tests:

1. Tests that verify the periodic collection is working correctly.
2. Tests that use the instantaneous statistics.json endpoint.


Thanks,

Ben Mahler


Re: Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

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

> On Jan. 21, 2014, 7:18 p.m., Benjamin Hindman wrote:
> > src/slave/monitor.cpp, line 230
> > <https://reviews.apache.org/r/13605/diff/4/?file=425942#file425942line230>
> >
> >     Seems like it would be nice to capture this to the log ...

Done!


> On Jan. 21, 2014, 7:18 p.m., Benjamin Hindman wrote:
> > src/slave/monitor.cpp, line 210
> > <https://reviews.apache.org/r/13605/diff/4/?file=425942#file425942line210>
> >
> >     Alternatively you could have a 'list<Usage> usages' where each Usage has a Future<ResourceStatistics> and still do process::await on those futures but just pass in 'usages' rather than 'lambda::_1' for '__statistics' as you do now.

Done!


> On Jan. 21, 2014, 7:18 p.m., Benjamin Hindman wrote:
> > src/slave/monitor.cpp, line 214
> > <https://reviews.apache.org/r/13605/diff/4/?file=425942#file425942line214>
> >
> >     Is there any reason to eventually make batch usage calls to the Isolator/Containerizer? Maybe a TODO that motivates that here?

Added a TODO.


- Ben


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


On Jan. 17, 2014, 4:04 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13605/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2014, 4:04 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This alters statistics.json to return instantaneous resource consumption information.
> 
> Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.
> 
> 
> Diffs
> -----
> 
>   src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
>   src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa 
>   src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253 
> 
> Diff: https://reviews.apache.org/r/13605/diff/
> 
> 
> Testing
> -------
> 
> Split the monitoring tests into two tests:
> 
> 1. Tests that verify the periodic collection is working correctly.
> 2. Tests that use the instantaneous statistics.json endpoint.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13605/#review32396
-----------------------------------------------------------

Ship it!



src/slave/monitor.cpp
<https://reviews.apache.org/r/13605/#comment61190>

    Alternatively you could have a 'list<Usage> usages' where each Usage has a Future<ResourceStatistics> and still do process::await on those futures but just pass in 'usages' rather than 'lambda::_1' for '__statistics' as you do now.



src/slave/monitor.cpp
<https://reviews.apache.org/r/13605/#comment61182>

    Is there any reason to eventually make batch usage calls to the Isolator/Containerizer? Maybe a TODO that motivates that here?



src/slave/monitor.cpp
<https://reviews.apache.org/r/13605/#comment61186>

    Seems like it would be nice to capture this to the log ...



src/slave/monitor.cpp
<https://reviews.apache.org/r/13605/#comment61180>

    What about calling the Future<Usage> above 'future' and then just calling this 'usage'? It probably makes sense to s/usages/futures/ then too.



src/slave/monitor.cpp
<https://reviews.apache.org/r/13605/#comment61188>

    What's happening here? Did you want to strings::format this or something?


- Benjamin Hindman


On Jan. 17, 2014, 4:04 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13605/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2014, 4:04 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This alters statistics.json to return instantaneous resource consumption information.
> 
> Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.
> 
> 
> Diffs
> -----
> 
>   src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
>   src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa 
>   src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253 
> 
> Diff: https://reviews.apache.org/r/13605/diff/
> 
> 
> Testing
> -------
> 
> Split the monitoring tests into two tests:
> 
> 1. Tests that verify the periodic collection is working correctly.
> 2. Tests that use the instantaneous statistics.json endpoint.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13605/
-----------------------------------------------------------

(Updated Jan. 17, 2014, 4:04 a.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Code cleanup per benh review.


Repository: mesos-git


Description
-------

This alters statistics.json to return instantaneous resource consumption information.

Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.


Diffs (updated)
-----

  src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
  src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa 
  src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253 

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


Testing
-------

Split the monitoring tests into two tests:

1. Tests that verify the periodic collection is working correctly.
2. Tests that use the instantaneous statistics.json endpoint.


Thanks,

Ben Mahler


Re: Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13605/
-----------------------------------------------------------

(Updated Jan. 15, 2014, 2:06 a.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Rebase.


Repository: mesos-git


Description
-------

This alters statistics.json to return instantaneous resource consumption information.

Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.


Diffs (updated)
-----

  src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
  src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa 
  src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253 

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


Testing
-------

Split the monitoring tests into two tests:

1. Tests that verify the periodic collection is working correctly.
2. Tests that use the instantaneous statistics.json endpoint.


Thanks,

Ben Mahler


Re: Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

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

> On Jan. 15, 2014, 2:04 a.m., Benjamin Hindman wrote:
> > src/slave/monitor.cpp, line 231
> > <https://reviews.apache.org/r/13605/diff/2/?file=422668#file422668line231>
> >
> >     Can we clean this up at all? What about a function, which takes framework ID and executor info and returns a future which contains all of FrameworkID, ExecutorInfo, and ResourceStatistics (perhaps as a struct?) that is satisfied when the underlying future returned from the isolator is satisfied?

I've added a Usage struct that wraps these and a usage() call to do the wrapping. It's a little more code but the logic should be more straightforward.

The collect() call could also make use of usage() to be more consistent, but the logic in _collect() becomes slightly more complex, so I opted against it.


- Ben


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


On Jan. 15, 2014, 2:06 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13605/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2014, 2:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This alters statistics.json to return instantaneous resource consumption information.
> 
> Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.
> 
> 
> Diffs
> -----
> 
>   src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
>   src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa 
>   src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253 
> 
> Diff: https://reviews.apache.org/r/13605/diff/
> 
> 
> Testing
> -------
> 
> Split the monitoring tests into two tests:
> 
> 1. Tests that verify the periodic collection is working correctly.
> 2. Tests that use the instantaneous statistics.json endpoint.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

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

> On Jan. 15, 2014, 2:04 a.m., Benjamin Hindman wrote:
> > Does it still make sense to continue to collect statistics at an interval or only drive it completely by the REST endpoint? Perhaps you want other endpoints, like archive or time series?
> 
> Ben Mahler wrote:
>     The only reason we periodically collected was to eventually show monitoring history in the webui. If we wanted to show history and the collection was driven by the REST endpoint, we would not have any historical data to show, unless something is hitting the endpoint periodically.
>     
>     Since we're not close to showing monitoring history in the webui, I would be ok with removing the periodic collection and the historical monitoring information until we know how we're going to use this data in the webui. What do you think?

These are the two choices I see for this:

1. Removing historical data, and driving collection only by the endpoint. Or,
2. Keeping historical data, and doing our own periodic collection.


- Ben


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


On Jan. 15, 2014, 2:06 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13605/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2014, 2:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This alters statistics.json to return instantaneous resource consumption information.
> 
> Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.
> 
> 
> Diffs
> -----
> 
>   src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
>   src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa 
>   src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253 
> 
> Diff: https://reviews.apache.org/r/13605/diff/
> 
> 
> Testing
> -------
> 
> Split the monitoring tests into two tests:
> 
> 1. Tests that verify the periodic collection is working correctly.
> 2. Tests that use the instantaneous statistics.json endpoint.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

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

> On Jan. 15, 2014, 2:04 a.m., Benjamin Hindman wrote:
> > Does it still make sense to continue to collect statistics at an interval or only drive it completely by the REST endpoint? Perhaps you want other endpoints, like archive or time series?

The only reason we periodically collected was to eventually show monitoring history in the webui. If we wanted to show history and the collection was driven by the REST endpoint, we would not have any historical data to show, unless something is hitting the endpoint periodically.

Since we're not close to showing monitoring history in the webui, I would be ok with removing the periodic collection and the historical monitoring information until we know how we're going to use this data in the webui. What do you think?


- Ben


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


On Jan. 15, 2014, 2:06 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13605/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2014, 2:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This alters statistics.json to return instantaneous resource consumption information.
> 
> Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.
> 
> 
> Diffs
> -----
> 
>   src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
>   src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa 
>   src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253 
> 
> Diff: https://reviews.apache.org/r/13605/diff/
> 
> 
> Testing
> -------
> 
> Split the monitoring tests into two tests:
> 
> 1. Tests that verify the periodic collection is working correctly.
> 2. Tests that use the instantaneous statistics.json endpoint.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

Posted by Jie Yu <yu...@gmail.com>.

> On Jan. 15, 2014, 2:04 a.m., Benjamin Hindman wrote:
> > Does it still make sense to continue to collect statistics at an interval or only drive it completely by the REST endpoint? Perhaps you want other endpoints, like archive or time series?
> 
> Ben Mahler wrote:
>     The only reason we periodically collected was to eventually show monitoring history in the webui. If we wanted to show history and the collection was driven by the REST endpoint, we would not have any historical data to show, unless something is hitting the endpoint periodically.
>     
>     Since we're not close to showing monitoring history in the webui, I would be ok with removing the periodic collection and the historical monitoring information until we know how we're going to use this data in the webui. What do you think?
> 
> Ben Mahler wrote:
>     These are the two choices I see for this:
>     
>     1. Removing historical data, and driving collection only by the endpoint. Or,
>     2. Keeping historical data, and doing our own periodic collection.

I prefer 2 because 1 is not reliable if there is a network problem.


- Jie


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


On Jan. 15, 2014, 2:06 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13605/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2014, 2:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This alters statistics.json to return instantaneous resource consumption information.
> 
> Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.
> 
> 
> Diffs
> -----
> 
>   src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
>   src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa 
>   src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253 
> 
> Diff: https://reviews.apache.org/r/13605/diff/
> 
> 
> Testing
> -------
> 
> Split the monitoring tests into two tests:
> 
> 1. Tests that verify the periodic collection is working correctly.
> 2. Tests that use the instantaneous statistics.json endpoint.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13605/#review31842
-----------------------------------------------------------


Does it still make sense to continue to collect statistics at an interval or only drive it completely by the REST endpoint? Perhaps you want other endpoints, like archive or time series?


src/slave/monitor.cpp
<https://reviews.apache.org/r/13605/#comment60588>

    Another newline here please.



src/slave/monitor.cpp
<https://reviews.apache.org/r/13605/#comment60587>

    Can we clean this up at all? What about a function, which takes framework ID and executor info and returns a future which contains all of FrameworkID, ExecutorInfo, and ResourceStatistics (perhaps as a struct?) that is satisfied when the underlying future returned from the isolator is satisfied?


- Benjamin Hindman


On Jan. 14, 2014, 7:09 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13605/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2014, 7:09 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This alters statistics.json to return instantaneous resource consumption information.
> 
> Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.
> 
> 
> Diffs
> -----
> 
>   src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
>   src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa 
>   src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253 
> 
> Diff: https://reviews.apache.org/r/13605/diff/
> 
> 
> Testing
> -------
> 
> Split the monitoring tests into two tests:
> 
> 1. Tests that verify the periodic collection is working correctly.
> 2. Tests that use the instantaneous statistics.json endpoint.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

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

> On Jan. 14, 2014, 7:10 a.m., Ben Mahler wrote:
> > src/slave/monitor.hpp, line 138
> > <https://reviews.apache.org/r/13605/diff/2/?file=422667#file422667line138>
> >
> >     This should use an inner Http class as was done in the master / slave for HTTP endpoints to clean this up and avoid this clunky function name.

This has some complexity due to the fact that the endpoints here are asynchronous and the ones in the Slave and Master are not. One cannot defer to a Process execution context and execute an arbitrary function at the current time. With C++11 required, we can re-visit this.

The http function names have now been simplified instead.


- Ben


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


On Jan. 15, 2014, 2:06 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13605/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2014, 2:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This alters statistics.json to return instantaneous resource consumption information.
> 
> Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.
> 
> 
> Diffs
> -----
> 
>   src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
>   src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa 
>   src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253 
> 
> Diff: https://reviews.apache.org/r/13605/diff/
> 
> 
> Testing
> -------
> 
> Split the monitoring tests into two tests:
> 
> 1. Tests that verify the periodic collection is working correctly.
> 2. Tests that use the instantaneous statistics.json endpoint.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13605/#review31709
-----------------------------------------------------------



src/slave/monitor.hpp
<https://reviews.apache.org/r/13605/#comment60450>

    This should use an inner Http class as was done in the master / slave for HTTP endpoints to clean this up and avoid this clunky function name.


- Ben Mahler


On Jan. 14, 2014, 7:09 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13605/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2014, 7:09 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This alters statistics.json to return instantaneous resource consumption information.
> 
> Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.
> 
> 
> Diffs
> -----
> 
>   src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
>   src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa 
>   src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253 
> 
> Diff: https://reviews.apache.org/r/13605/diff/
> 
> 
> Testing
> -------
> 
> Split the monitoring tests into two tests:
> 
> 1. Tests that verify the periodic collection is working correctly.
> 2. Tests that use the instantaneous statistics.json endpoint.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 13605: Updated the monitoring endpoint to return instantaneous values.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13605/
-----------------------------------------------------------

(Updated Jan. 14, 2014, 7:09 a.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
-------

Rebased. Will be pairing with Ross tomorrow to update https://reviews.apache.org/r/13606/ .


Repository: mesos-git


Description
-------

This alters statistics.json to return instantaneous resource consumption information.

Previously, one could receive information that is stale by up to 5 seconds (or the RESOURCE_MONITORING_INTERVAL). This allows one to build Top like utilities using the monitoring endpoint, this is done by the subsequent change in this chain of reviews.


Diffs (updated)
-----

  src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
  src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa 
  src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253 

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


Testing
-------

Split the monitoring tests into two tests:

1. Tests that verify the periodic collection is working correctly.
2. Tests that use the instantaneous statistics.json endpoint.


Thanks,

Ben Mahler