You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Chi Zhang <ch...@gmail.com> on 2013/10/18 01:15:18 UTC

Re: 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/#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 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
> 
>