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:01 UTC
Review Request 13604: Refactored the ResourceMonitor to use TimeSeries
directly instead of Statistics.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13604/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Vinod Kone.
Bugs: MESOS-375
https://issues.apache.org/jira/browse/MESOS-375
Repository: mesos-git
Description
-------
Dumping data into Statistics leads to high memory consumption for various reasons. We had to encode the string values of the statistical information into a unique string key ("frameworkId/ExecutorId/statistic_name"), all of these combinations consume a lot of memory. This also made it clunky in terms of retrieving the latest information. This now uses TimeSeries<ResourceStatistics> to efficiently store the monitoring information.
My subsequent change in this chain of reviews returns immediate values to simplify the statistics.json endpoint.
This also uses JSON::Protobuf I recently added to simplify the JSON generation substantially!
Diffs
-----
src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578
src/slave/monitor.cpp 8e1eb353c55f9a04b07c382c10c669715a2d7ac2
src/tests/monitor_tests.cpp 31424160b0493ef37705be25cd97f74e027d85ef
Diff: https://reviews.apache.org/r/13604/diff/
Testing
-------
make check (no test modifications needed for this change as it preserves the API)
Thanks,
Ben Mahler
Re: Review Request 13604: Refactored the ResourceMonitor to use TimeSeries
directly instead of Statistics.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13604/
-----------------------------------------------------------
(Updated Jan. 21, 2014, 11:29 p.m.)
Review request for mesos, Benjamin Hindman and Jie Yu.
Changes
-------
Benh review.
Bugs: MESOS-375
https://issues.apache.org/jira/browse/MESOS-375
Repository: mesos-git
Description
-------
Dumping data into Statistics leads to high memory consumption for various reasons. We had to encode the string values of the statistical information into a unique string key ("frameworkId/ExecutorId/statistic_name"), all of these combinations consume a lot of memory. This also made it clunky in terms of retrieving the latest information. This now uses TimeSeries<ResourceStatistics> to efficiently store the monitoring information.
My subsequent change in this chain of reviews returns immediate values to simplify the statistics.json endpoint.
This also uses JSON::Protobuf I recently added to simplify the JSON generation substantially!
Diffs (updated)
-----
src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578
src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa
src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253
Diff: https://reviews.apache.org/r/13604/diff/
Testing
-------
make check (no test modifications needed for this change as it preserves the API)
Thanks,
Ben Mahler
Re: Review Request 13604: Refactored the ResourceMonitor to use TimeSeries
directly instead of Statistics.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13604/
-----------------------------------------------------------
(Updated Jan. 15, 2014, 2 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Forgot to remove the unneeded constants.
Bugs: MESOS-375
https://issues.apache.org/jira/browse/MESOS-375
Repository: mesos-git
Description
-------
Dumping data into Statistics leads to high memory consumption for various reasons. We had to encode the string values of the statistical information into a unique string key ("frameworkId/ExecutorId/statistic_name"), all of these combinations consume a lot of memory. This also made it clunky in terms of retrieving the latest information. This now uses TimeSeries<ResourceStatistics> to efficiently store the monitoring information.
My subsequent change in this chain of reviews returns immediate values to simplify the statistics.json endpoint.
This also uses JSON::Protobuf I recently added to simplify the JSON generation substantially!
Diffs (updated)
-----
src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578
src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa
src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253
Diff: https://reviews.apache.org/r/13604/diff/
Testing
-------
make check (no test modifications needed for this change as it preserves the API)
Thanks,
Ben Mahler
Re: Review Request 13604: Refactored the ResourceMonitor to use TimeSeries
directly instead of Statistics.
Posted by Ben Mahler <be...@gmail.com>.
> On Jan. 15, 2014, 1:50 a.m., Benjamin Hindman wrote:
> > src/slave/monitor.hpp, line 92
> > <https://reviews.apache.org/r/13604/diff/2/?file=422664#file422664line92>
> >
> > I think it's probably time for s/ResourceMonitor/Monitor/ ;).
If it's just 'Monitor', I'm not sure whether it's clear as to what 'thing' is being monitored.
> On Jan. 15, 2014, 1:50 a.m., Benjamin Hindman wrote:
> > src/slave/monitor.cpp, line 99
> > <https://reviews.apache.org/r/13604/diff/2/?file=422665#file422665line99>
> >
> > Why 'insert'?
Ah, this was an artifact of when I had make MonitoringInfo const, but I had loosened this to allow assignability and forgot to update this to use the [] operator, thanks!
> On Jan. 15, 2014, 1:50 a.m., Benjamin Hindman wrote:
> > src/tests/monitor_tests.cpp, lines 165-166
> > <https://reviews.apache.org/r/13604/diff/2/?file=422666#file422666line165>
> >
> > Could you wrap the entire JSON string in 'string(...)' instead?
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13604/#review31840
-----------------------------------------------------------
On Jan. 15, 2014, 2 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13604/
> -----------------------------------------------------------
>
> (Updated Jan. 15, 2014, 2 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Bugs: MESOS-375
> https://issues.apache.org/jira/browse/MESOS-375
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Dumping data into Statistics leads to high memory consumption for various reasons. We had to encode the string values of the statistical information into a unique string key ("frameworkId/ExecutorId/statistic_name"), all of these combinations consume a lot of memory. This also made it clunky in terms of retrieving the latest information. This now uses TimeSeries<ResourceStatistics> to efficiently store the monitoring information.
>
> My subsequent change in this chain of reviews returns immediate values to simplify the statistics.json endpoint.
>
> This also uses JSON::Protobuf I recently added to simplify the JSON generation substantially!
>
>
> Diffs
> -----
>
> src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578
> src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa
> src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253
>
> Diff: https://reviews.apache.org/r/13604/diff/
>
>
> Testing
> -------
>
> make check (no test modifications needed for this change as it preserves the API)
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 13604: Refactored the ResourceMonitor to use TimeSeries
directly instead of Statistics.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13604/#review31840
-----------------------------------------------------------
Ship it!
src/slave/monitor.hpp
<https://reviews.apache.org/r/13604/#comment60572>
I think it's probably time for s/ResourceMonitor/Monitor/ ;).
src/slave/monitor.hpp
<https://reviews.apache.org/r/13604/#comment60573>
Not needed?
src/slave/monitor.hpp
<https://reviews.apache.org/r/13604/#comment60574>
s/history/archive/?
src/slave/monitor.hpp
<https://reviews.apache.org/r/13604/#comment60575>
s/history.json/archive.json/? ;)
Also, you can now get path parameters for routes!
src/slave/monitor.cpp
<https://reviews.apache.org/r/13604/#comment60576>
For std::pair again?
src/slave/monitor.cpp
<https://reviews.apache.org/r/13604/#comment60577>
Revert? ;)
src/slave/monitor.cpp
<https://reviews.apache.org/r/13604/#comment60578>
Why 'insert'?
src/slave/monitor.cpp
<https://reviews.apache.org/r/13604/#comment60579>
s/Future<Nothing>::failed/Failure/
src/slave/monitor.cpp
<https://reviews.apache.org/r/13604/#comment60580>
Good riddance!
src/slave/monitor.cpp
<https://reviews.apache.org/r/13604/#comment60581>
entry.values["statistics"] = JSON::Protobuf(statistics);
src/tests/monitor_tests.cpp
<https://reviews.apache.org/r/13604/#comment60582>
Could you wrap the entire JSON string in 'string(...)' instead?
- Benjamin Hindman
On Jan. 14, 2014, 7:08 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13604/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2014, 7:08 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Jie Yu.
>
>
> Bugs: MESOS-375
> https://issues.apache.org/jira/browse/MESOS-375
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Dumping data into Statistics leads to high memory consumption for various reasons. We had to encode the string values of the statistical information into a unique string key ("frameworkId/ExecutorId/statistic_name"), all of these combinations consume a lot of memory. This also made it clunky in terms of retrieving the latest information. This now uses TimeSeries<ResourceStatistics> to efficiently store the monitoring information.
>
> My subsequent change in this chain of reviews returns immediate values to simplify the statistics.json endpoint.
>
> This also uses JSON::Protobuf I recently added to simplify the JSON generation substantially!
>
>
> Diffs
> -----
>
> src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578
> src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa
> src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253
>
> Diff: https://reviews.apache.org/r/13604/diff/
>
>
> Testing
> -------
>
> make check (no test modifications needed for this change as it preserves the API)
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 13604: Refactored the ResourceMonitor to use TimeSeries
directly instead of Statistics.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13604/
-----------------------------------------------------------
(Updated Jan. 14, 2014, 7:08 a.m.)
Review request for mesos, Benjamin Hindman and Jie Yu.
Changes
-------
Rebased.
Bugs: MESOS-375
https://issues.apache.org/jira/browse/MESOS-375
Repository: mesos-git
Description
-------
Dumping data into Statistics leads to high memory consumption for various reasons. We had to encode the string values of the statistical information into a unique string key ("frameworkId/ExecutorId/statistic_name"), all of these combinations consume a lot of memory. This also made it clunky in terms of retrieving the latest information. This now uses TimeSeries<ResourceStatistics> to efficiently store the monitoring information.
My subsequent change in this chain of reviews returns immediate values to simplify the statistics.json endpoint.
This also uses JSON::Protobuf I recently added to simplify the JSON generation substantially!
Diffs (updated)
-----
src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578
src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa
src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253
Diff: https://reviews.apache.org/r/13604/diff/
Testing
-------
make check (no test modifications needed for this change as it preserves the API)
Thanks,
Ben Mahler