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:15:35 UTC
Review Request: Resource Monitoring 5: Added the ability to archive
statistics.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9093/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Vinod Kone.
Description
-------
Some statistics will be ephemeral. (e.g. executor resource usage information).
It does not make sense to keep a value of these stats in the snapshot.
This addresses bug MESOS-324.
https://issues.apache.org/jira/browse/MESOS-324
Diffs
-----
third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
Diff: https://reviews.apache.org/r/9093/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request: Resource Monitoring 5: Added the ability to archive
statistics.
Posted by Ben Mahler <be...@gmail.com>.
> On Jan. 28, 2013, 8:44 p.m., Vinod Kone wrote:
> > third_party/libprocess/include/process/statistics.hpp, line 26
> > <https://reviews.apache.org/r/9093/diff/2/?file=251833#file251833line26>
> >
> > s/sparsifying/aggregating/ ?
What I mean here is instead of always deleting the oldest value, we would sparsify older values.
E.g.
Within 1 minute ago: Keep all values
Within 1-2 minutes ago: Keep half the values
Within 2-4 minutes ago: Keep 1/4 the values
Within 4-8 minutes ago: Keep 1/8 the values
etc..
There are many techniques for sparsifying the older values, I'm not sure which would be best as of yet.
> On Jan. 28, 2013, 8:44 p.m., Vinod Kone wrote:
> > third_party/libprocess/include/process/statistics.hpp, line 27
> > <https://reviews.apache.org/r/9093/diff/2/?file=251833#file251833line27>
> >
> > s/oldest/older/
Indeed I mean oldest.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9093/#review15767
-----------------------------------------------------------
On Jan. 24, 2013, 11:19 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9093/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2013, 11:19 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> Some statistics will be ephemeral. (e.g. executor resource usage information).
> It does not make sense to keep a value of these stats in the snapshot.
>
>
> This addresses bug MESOS-324.
> https://issues.apache.org/jira/browse/MESOS-324
>
>
> Diffs
> -----
>
> third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
> third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
>
> Diff: https://reviews.apache.org/r/9093/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Resource Monitoring 5: Added the ability to archive
statistics.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9093/#review15767
-----------------------------------------------------------
third_party/libprocess/include/process/statistics.hpp
<https://reviews.apache.org/r/9093/#comment33938>
s/sparsifying/aggregating/ ?
third_party/libprocess/include/process/statistics.hpp
<https://reviews.apache.org/r/9093/#comment33939>
s/oldest/older/
third_party/libprocess/include/process/statistics.hpp
<https://reviews.apache.org/r/9093/#comment33940>
s/two/three/ :)
third_party/libprocess/include/process/statistics.hpp
<https://reviews.apache.org/r/9093/#comment33941>
s/for/associated with/
third_party/libprocess/src/statistics.cpp
<https://reviews.apache.org/r/9093/#comment33943>
unless it is archived?
third_party/libprocess/src/statistics.cpp
<https://reviews.apache.org/r/9093/#comment33942>
An explanation for this mapping would be great.
third_party/libprocess/src/statistics.cpp
<https://reviews.apache.org/r/9093/#comment33944>
pull the 5 minutes out as a constant.
also, doc this behavior in its declaration
- Vinod Kone
On Jan. 24, 2013, 11:19 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9093/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2013, 11:19 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> Some statistics will be ephemeral. (e.g. executor resource usage information).
> It does not make sense to keep a value of these stats in the snapshot.
>
>
> This addresses bug MESOS-324.
> https://issues.apache.org/jira/browse/MESOS-324
>
>
> Diffs
> -----
>
> third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
> third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
>
> Diff: https://reviews.apache.org/r/9093/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Resource Monitoring 5: Added the ability to archive
statistics.
Posted by Ben Mahler <be...@gmail.com>.
> On Jan. 30, 2013, 2 a.m., Vinod Kone wrote:
> > third_party/libprocess/src/statistics.cpp, lines 51-54
> > <https://reviews.apache.org/r/9093/diff/2-3/?file=251834#file251834line51>
> >
> > Why are these virtual?
I hesitated to mix virtual / non-virtual methods, but fair enough. Fixed in part 4.
> On Jan. 30, 2013, 2 a.m., Vinod Kone wrote:
> > third_party/libprocess/src/statistics.cpp, line 145
> > <https://reviews.apache.org/r/9093/diff/2-3/?file=251834#file251834line145>
> >
> > s/perpetually based on/periodically every/ ?
done
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9093/#review15815
-----------------------------------------------------------
On Jan. 29, 2013, 12:45 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9093/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2013, 12:45 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> Some statistics will be ephemeral. (e.g. executor resource usage information).
> It does not make sense to keep a value of these stats in the snapshot.
>
>
> This addresses bug MESOS-324.
> https://issues.apache.org/jira/browse/MESOS-324
>
>
> Diffs
> -----
>
> third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
> third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
>
> Diff: https://reviews.apache.org/r/9093/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Resource Monitoring 5: Added the ability to archive
statistics.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9093/#review15815
-----------------------------------------------------------
third_party/libprocess/src/statistics.cpp
<https://reviews.apache.org/r/9093/#comment34066>
Why are these virtual?
third_party/libprocess/src/statistics.cpp
<https://reviews.apache.org/r/9093/#comment34067>
s/perpetually based on/periodically every/ ?
- Vinod Kone
On Jan. 29, 2013, 12:45 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9093/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2013, 12:45 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> Some statistics will be ephemeral. (e.g. executor resource usage information).
> It does not make sense to keep a value of these stats in the snapshot.
>
>
> This addresses bug MESOS-324.
> https://issues.apache.org/jira/browse/MESOS-324
>
>
> Diffs
> -----
>
> third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
> third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
>
> Diff: https://reviews.apache.org/r/9093/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Resource Monitoring 5: Added the ability to archive
statistics.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9093/#review15949
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
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/9093/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2013, 6:17 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> Some statistics will be ephemeral. (e.g. executor resource usage information).
> It does not make sense to keep a value of these stats in the snapshot.
>
>
> This addresses bug MESOS-324.
> https://issues.apache.org/jira/browse/MESOS-324
>
>
> Diffs
> -----
>
> third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
> third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
>
> Diff: https://reviews.apache.org/r/9093/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Resource Monitoring 5: Added the ability to archive
statistics.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9093/#review16813
-----------------------------------------------------------
third_party/libprocess/src/tests/statistics_tests.cpp
<https://reviews.apache.org/r/9093/#comment35723>
implicit double
- Ben Mahler
On Feb. 13, 2013, 2:12 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9093/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2013, 2:12 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> Some statistics will be ephemeral. (e.g. executor resource usage information).
> It does not make sense to keep a value of these stats in the snapshot.
>
>
> This addresses bug MESOS-324.
> https://issues.apache.org/jira/browse/MESOS-324
>
>
> Diffs
> -----
>
> third_party/libprocess/include/process/clock.hpp 1a98c6a8b50fdad5308321413121f86bea135d37
> 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/9093/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Resource Monitoring 5: Added the ability to archive
statistics.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9093/#review16812
-----------------------------------------------------------
Ship it!
third_party/libprocess/src/statistics.cpp
<https://reviews.apache.org/r/9093/#comment35722>
Old habits die hard. Kill them more quickly.
third_party/libprocess/src/statistics.cpp
<https://reviews.apache.org/r/9093/#comment35724>
s/data/values/
third_party/libprocess/src/statistics.cpp
<https://reviews.apache.org/r/9093/#comment35725>
const.
- Benjamin Hindman
On Feb. 13, 2013, 2:12 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9093/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2013, 2:12 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> Some statistics will be ephemeral. (e.g. executor resource usage information).
> It does not make sense to keep a value of these stats in the snapshot.
>
>
> This addresses bug MESOS-324.
> https://issues.apache.org/jira/browse/MESOS-324
>
>
> Diffs
> -----
>
> third_party/libprocess/include/process/clock.hpp 1a98c6a8b50fdad5308321413121f86bea135d37
> 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/9093/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Resource Monitoring 5: Added the ability to archive
statistics.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9093/
-----------------------------------------------------------
(Updated Feb. 22, 2013, 12:31 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Benh review.
Rebased off trunk.
Description
-------
Some statistics will be ephemeral. (e.g. executor resource usage information).
It does not make sense to keep a value of these stats in the snapshot.
This addresses bug MESOS-324.
https://issues.apache.org/jira/browse/MESOS-324
Diffs (updated)
-----
third_party/libprocess/include/process/clock.hpp 1a98c6a8b50fdad5308321413121f86bea135d37
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/9093/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request: Resource Monitoring 5: Added the ability to archive
statistics.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9093/
-----------------------------------------------------------
(Updated Feb. 13, 2013, 2:12 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Fixed a bug with empty time series archival.
Description
-------
Some statistics will be ephemeral. (e.g. executor resource usage information).
It does not make sense to keep a value of these stats in the snapshot.
This addresses bug MESOS-324.
https://issues.apache.org/jira/browse/MESOS-324
Diffs (updated)
-----
third_party/libprocess/include/process/clock.hpp 1a98c6a8b50fdad5308321413121f86bea135d37
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/9093/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request: Resource Monitoring 5: Added the ability to archive
statistics.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9093/
-----------------------------------------------------------
(Updated Feb. 7, 2013, 12:39 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
-benh review
-Added a TimeSeries struct.
Description
-------
Some statistics will be ephemeral. (e.g. executor resource usage information).
It does not make sense to keep a value of these stats in the snapshot.
This addresses bug MESOS-324.
https://issues.apache.org/jira/browse/MESOS-324
Diffs (updated)
-----
third_party/libprocess/include/process/clock.hpp 1a98c6a8b50fdad5308321413121f86bea135d37
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/9093/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request: Resource Monitoring 5: Added the ability to archive
statistics.
Posted by Ben Mahler <be...@gmail.com>.
> On Feb. 5, 2013, 6:26 p.m., Benjamin Hindman wrote:
> > My high level thought for this review:
> >
> > What about adding a Statistic struct that contains two fields, (1) the 'map<Seconds, double> values' and a 'bool archived'. The one thing I like about that is that we don't need to keep logic around for deciding when to clean up the archived map.
I held off on a struct but after thinking about it, I think adding a TimeSeries struct would be nice! Done, and it's much cleaner.
I can also expand on this struct later to isolate the TimeSeries functionality:
-Truncation logic: this will get more complex when adding coarseness.
-Set, Get, etc if needed.
> On Feb. 5, 2013, 6:26 p.m., Benjamin Hindman wrote:
> > third_party/libprocess/include/process/clock.hpp, line 9
> > <https://reviews.apache.org/r/9093/diff/5/?file=254415#file254415line9>
> >
> > When we changed most 'double' values to use Seconds or Duration I did this here, but there was some mess and never got that part fully committed. I still have an old branch for it though, so ping me if you take this on and want to do less work. ;)
Feel free to email me with your partial patch.
I'm going to punt for resource monitoring, I may get to after in an independent change.
> On Feb. 5, 2013, 6:26 p.m., Benjamin Hindman wrote:
> > third_party/libprocess/include/process/statistics.hpp, line 19
> > <https://reviews.apache.org/r/9093/diff/5/?file=254416#file254416line19>
> >
> > Eventually we'll want to make these "configurable" via environment variables (that's been our standard for libprocess).
I only see:
$ grep -R getenv third_party/libprocess/src
third_party/libprocess/src/process.cpp: value = getenv("LIBPROCESS_IP");
third_party/libprocess/src/process.cpp: value = getenv("LIBPROCESS_PORT");
I can add this now if you like: "LIBPROCESS_STATISTICS_TRUNCATION_INTERVAL", "LIBPROCESS_STATS_TRUNCATION"?
> On Feb. 5, 2013, 6:26 p.m., Benjamin Hindman wrote:
> > third_party/libprocess/include/process/statistics.hpp, line 25
> > <https://reviews.apache.org/r/9093/diff/5/?file=254416#file254416line25>
> >
> > As the bottom of your comment alludes, the original intent for window was to coarsen the granularity of data over time. That way, for high frequency statistics, there are a lot of _recent_ data points (fine granularity), and less _older_ data points (coarse granularity). I could see adding a tunable which decides how many total data points to keep around which informs how often to delete old data points while still keeping a window worth of information. This is much more valuable than just keeping N data points around because N data points might not give you enough history (over some window of time) to understand the general shape of the data.
Right, I'm on the same page, I was focused on providing determinism on the upper bound to guarantee that this won't blow up in memory (the tunable you mentioned), and possibly being intelligent. I'll rewrite this comment to describe the ideal design.
> On Feb. 5, 2013, 6:26 p.m., Benjamin Hindman wrote:
> > third_party/libprocess/include/process/statistics.hpp, line 67
> > <https://reviews.apache.org/r/9093/diff/5/?file=254416#file254416line67>
> >
> > As in, it will be part of the snapshot until the window expires?
Nope: // 1. The statistic will no longer be part of the snapshot.
But it will remain part of the timeseries.json.
> On Feb. 5, 2013, 6:26 p.m., Benjamin Hindman wrote:
> > third_party/libprocess/src/statistics.cpp, line 120
> > <https://reviews.apache.org/r/9093/diff/5/?file=254417#file254417line120>
> >
> > Would one ever want to archive an entire context? Seems like a helper that loops through all statistics for a name and calls this version of the function might be very useful (and avoid people having to remember all fo the statistics they might have created!).
I don't see contexts being used ephemerally, but I could be convinced otherwise with a good example.
This is why I did not include such a method.
> On Feb. 5, 2013, 6:26 p.m., Benjamin Hindman wrote:
> > third_party/libprocess/src/statistics.cpp, line 132
> > <https://reviews.apache.org/r/9093/diff/5/?file=254417#file254417line132>
> >
> > First, I didn't understand this comment and I think making it more explicit would be helpful, i.e.,: "We wait 'window' time until trying to do our first truncation across all statistics because no values will get truncated until after window passes". Second, I don't agree with this since someone could set values outside the window.
Good point!
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9093/#review16124
-----------------------------------------------------------
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/9093/
> -----------------------------------------------------------
>
> (Updated Feb. 1, 2013, 1:12 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> Some statistics will be ephemeral. (e.g. executor resource usage information).
> It does not make sense to keep a value of these stats in the snapshot.
>
>
> This addresses bug MESOS-324.
> https://issues.apache.org/jira/browse/MESOS-324
>
>
> Diffs
> -----
>
> third_party/libprocess/include/process/clock.hpp 1a98c6a8b50fdad5308321413121f86bea135d37
> 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/9093/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Resource Monitoring 5: Added the ability to archive
statistics.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9093/#review16124
-----------------------------------------------------------
My high level thought for this review:
What about adding a Statistic struct that contains two fields, (1) the 'map<Seconds, double> values' and a 'bool archived'. The one thing I like about that is that we don't need to keep logic around for deciding when to clean up the archived map.
third_party/libprocess/include/process/clock.hpp
<https://reviews.apache.org/r/9093/#comment34459>
When we changed most 'double' values to use Seconds or Duration I did this here, but there was some mess and never got that part fully committed. I still have an old branch for it though, so ping me if you take this on and want to do less work. ;)
third_party/libprocess/include/process/statistics.hpp
<https://reviews.apache.org/r/9093/#comment34466>
Eventually we'll want to make these "configurable" via environment variables (that's been our standard for libprocess).
third_party/libprocess/include/process/statistics.hpp
<https://reviews.apache.org/r/9093/#comment34460>
As the bottom of your comment alludes, the original intent for window was to coarsen the granularity of data over time. That way, for high frequency statistics, there are a lot of _recent_ data points (fine granularity), and less _older_ data points (coarse granularity). I could see adding a tunable which decides how many total data points to keep around which informs how often to delete old data points while still keeping a window worth of information. This is much more valuable than just keeping N data points around because N data points might not give you enough history (over some window of time) to understand the general shape of the data.
third_party/libprocess/include/process/statistics.hpp
<https://reviews.apache.org/r/9093/#comment34463>
s/timeseries/time series/
third_party/libprocess/include/process/statistics.hpp
<https://reviews.apache.org/r/9093/#comment34464>
As in, it will be part of the snapshot until the window expires?
third_party/libprocess/include/process/statistics.hpp
<https://reviews.apache.org/r/9093/#comment34462>
This line seems redundant.
third_party/libprocess/src/statistics.cpp
<https://reviews.apache.org/r/9093/#comment34467>
Would one ever want to archive an entire context? Seems like a helper that loops through all statistics for a name and calls this version of the function might be very useful (and avoid people having to remember all fo the statistics they might have created!).
third_party/libprocess/src/statistics.cpp
<https://reviews.apache.org/r/9093/#comment34468>
First, I didn't understand this comment and I think making it more explicit would be helpful, i.e.,: "We wait 'window' time until trying to do our first truncation across all statistics because no values will get truncated until after window passes". Second, I don't agree with this since someone could set values outside the window.
third_party/libprocess/src/statistics.cpp
<https://reviews.apache.org/r/9093/#comment34465>
s/timeseries/time series/g
Also, let's lean towards sentences that leave less ambiguity or require thought: 'Returns true if the time series is empty."
- 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/9093/
> -----------------------------------------------------------
>
> (Updated Feb. 1, 2013, 1:12 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> Some statistics will be ephemeral. (e.g. executor resource usage information).
> It does not make sense to keep a value of these stats in the snapshot.
>
>
> This addresses bug MESOS-324.
> https://issues.apache.org/jira/browse/MESOS-324
>
>
> Diffs
> -----
>
> third_party/libprocess/include/process/clock.hpp 1a98c6a8b50fdad5308321413121f86bea135d37
> 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/9093/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Resource Monitoring 5: Added the ability to archive
statistics.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9093/
-----------------------------------------------------------
(Updated Feb. 1, 2013, 1:12 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Tests + bug fix.
Description
-------
Some statistics will be ephemeral. (e.g. executor resource usage information).
It does not make sense to keep a value of these stats in the snapshot.
This addresses bug MESOS-324.
https://issues.apache.org/jira/browse/MESOS-324
Diffs (updated)
-----
third_party/libprocess/include/process/clock.hpp 1a98c6a8b50fdad5308321413121f86bea135d37
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/9093/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request: Resource Monitoring 5: Added the ability to archive
statistics.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9093/
-----------------------------------------------------------
(Updated Jan. 31, 2013, 6:17 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Vinod's review.
Description
-------
Some statistics will be ephemeral. (e.g. executor resource usage information).
It does not make sense to keep a value of these stats in the snapshot.
This addresses bug MESOS-324.
https://issues.apache.org/jira/browse/MESOS-324
Diffs (updated)
-----
third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
Diff: https://reviews.apache.org/r/9093/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request: Resource Monitoring 5: Added the ability to archive
statistics.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9093/
-----------------------------------------------------------
(Updated Jan. 29, 2013, 12:45 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Vinod's review.
Description
-------
Some statistics will be ephemeral. (e.g. executor resource usage information).
It does not make sense to keep a value of these stats in the snapshot.
This addresses bug MESOS-324.
https://issues.apache.org/jira/browse/MESOS-324
Diffs (updated)
-----
third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
Diff: https://reviews.apache.org/r/9093/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request: Resource Monitoring 5: Added the ability to archive
statistics.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9093/
-----------------------------------------------------------
(Updated Jan. 24, 2013, 11:19 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Fixed the archival logic to correctly handle Statistics that get updated after archival.
Description
-------
Some statistics will be ephemeral. (e.g. executor resource usage information).
It does not make sense to keep a value of these stats in the snapshot.
This addresses bug MESOS-324.
https://issues.apache.org/jira/browse/MESOS-324
Diffs (updated)
-----
third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
Diff: https://reviews.apache.org/r/9093/diff/
Testing
-------
make check
Thanks,
Ben Mahler