You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Alex Degtiar <al...@gmail.com> on 2011/12/07 08:08:06 UTC
Review Request: Create utilities to collect information from the proc
filesystem
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3050/
-----------------------------------------------------------
Review request for mesos.
Summary
-------
The first of several patches related to resource usage monitoring. This patch provides a collection of utilities for use on Linux for reading stats from proc. It is used by both the lxc and proc resource collectors.
This addresses bug MESOS-89.
https://issues.apache.org/jira/browse/MESOS-89
Diffs
-----
src/tests/Makefile.in ea943f7
src/tests/proc_utils_tests.cpp PRE-CREATION
src/Makefile.in 516f128
src/monitoring/proc_utils.hpp PRE-CREATION
src/monitoring/proc_utils.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/3050/diff
Testing
-------
Sanity tests have been written in src/tests/proc_utils_tests.cpp for all utility functions, and functions have been tested ad hoc.
Thanks,
Alex
Re: Review Request: Create utilities to collect information from the proc
filesystem
Posted by Charles Reiss <wo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3050/#review4708
-----------------------------------------------------------
- Charles
On 2011-12-24 04:13:40, Alex Degtiar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3050/
> -----------------------------------------------------------
>
> (Updated 2011-12-24 04:13:40)
>
>
> Review request for mesos.
>
>
> Summary
> -------
>
> The first of several patches related to resource usage monitoring. This patch provides a collection of utilities for use on Linux for reading stats from proc. It is used by both the lxc and proc resource collectors.
>
>
> This addresses bug MESOS-89.
> https://issues.apache.org/jira/browse/MESOS-89
>
>
> Diffs
> -----
>
> src/monitoring/linux/proc_utils.cpp PRE-CREATION
> src/tests/Makefile.in ea943f7
> src/tests/proc_utils_tests.cpp PRE-CREATION
> src/Makefile.in 516f128
> src/monitoring/linux/proc_utils.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3050/diff
>
>
> Testing
> -------
>
> Sanity tests have been written in src/tests/proc_utils_tests.cpp for all utility functions, and functions have been tested ad hoc.
>
>
> Thanks,
>
> Alex
>
>
Re: Review Request: Create utilities to collect information from the proc
filesystem
Posted by Sam Whitlock <ph...@gmail.com>.
> On 2012-02-07 18:14:30, Charles Reiss wrote:
> >
>
> Sam Whitlock wrote:
> What target should these go into in Makefile.am? libmesos_no_third_party_la_SOURCES seems to be the one that all the source files are being added to.
>
> Charles Reiss wrote:
> libmesos_no_third_party_la would be fine for the non-test targets. (The test ones probably go into the mesos_tests_SOURCES.)
>
Because some of the monitoring stuff is linux-specific, should it be conditionally added to libmesos_no_third_party_la_SOURCES in the same way lxc_isolation_module.cpp is?
It seems like all the hpp files are unconditionally added to libmesos_no_third_party and linux-specific cpp files should only be added if OS_LINUX is true. Is this correct?
- Sam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3050/#review4868
-----------------------------------------------------------
On 2011-12-24 04:13:40, Alex Degtiar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3050/
> -----------------------------------------------------------
>
> (Updated 2011-12-24 04:13:40)
>
>
> Review request for mesos.
>
>
> Summary
> -------
>
> The first of several patches related to resource usage monitoring. This patch provides a collection of utilities for use on Linux for reading stats from proc. It is used by both the lxc and proc resource collectors.
>
>
> This addresses bug MESOS-89.
> https://issues.apache.org/jira/browse/MESOS-89
>
>
> Diffs
> -----
>
> src/monitoring/linux/proc_utils.cpp PRE-CREATION
> src/tests/Makefile.in ea943f7
> src/tests/proc_utils_tests.cpp PRE-CREATION
> src/Makefile.in 516f128
> src/monitoring/linux/proc_utils.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3050/diff
>
>
> Testing
> -------
>
> Sanity tests have been written in src/tests/proc_utils_tests.cpp for all utility functions, and functions have been tested ad hoc.
>
>
> Thanks,
>
> Alex
>
>
Re: Review Request: Create utilities to collect information from the proc
filesystem
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On 2012-02-07 18:14:30, Charles Reiss wrote:
> >
>
> Sam Whitlock wrote:
> What target should these go into in Makefile.am? libmesos_no_third_party_la_SOURCES seems to be the one that all the source files are being added to.
>
> Charles Reiss wrote:
> libmesos_no_third_party_la would be fine for the non-test targets. (The test ones probably go into the mesos_tests_SOURCES.)
>
>
> Sam Whitlock wrote:
> Because some of the monitoring stuff is linux-specific, should it be conditionally added to libmesos_no_third_party_la_SOURCES in the same way lxc_isolation_module.cpp is?
>
> It seems like all the hpp files are unconditionally added to libmesos_no_third_party and linux-specific cpp files should only be added if OS_LINUX is true. Is this correct?
Yes.
Yes, but don't forget to add it to EXTRA_DIST just ast in the lxc_isolation_module.cpp.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3050/#review4868
-----------------------------------------------------------
On 2011-12-24 04:13:40, Alex Degtiar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3050/
> -----------------------------------------------------------
>
> (Updated 2011-12-24 04:13:40)
>
>
> Review request for mesos.
>
>
> Summary
> -------
>
> The first of several patches related to resource usage monitoring. This patch provides a collection of utilities for use on Linux for reading stats from proc. It is used by both the lxc and proc resource collectors.
>
>
> This addresses bug MESOS-89.
> https://issues.apache.org/jira/browse/MESOS-89
>
>
> Diffs
> -----
>
> src/monitoring/linux/proc_utils.cpp PRE-CREATION
> src/tests/Makefile.in ea943f7
> src/tests/proc_utils_tests.cpp PRE-CREATION
> src/Makefile.in 516f128
> src/monitoring/linux/proc_utils.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3050/diff
>
>
> Testing
> -------
>
> Sanity tests have been written in src/tests/proc_utils_tests.cpp for all utility functions, and functions have been tested ad hoc.
>
>
> Thanks,
>
> Alex
>
>
Re: Review Request: Create utilities to collect information from the proc
filesystem
Posted by Sam Whitlock <ph...@gmail.com>.
> On 2012-02-07 18:14:30, Charles Reiss wrote:
> >
What target should these go into in Makefile.am? libmesos_no_third_party_la_SOURCES seems to be the one that all the source files are being added to.
- Sam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3050/#review4868
-----------------------------------------------------------
On 2011-12-24 04:13:40, Alex Degtiar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3050/
> -----------------------------------------------------------
>
> (Updated 2011-12-24 04:13:40)
>
>
> Review request for mesos.
>
>
> Summary
> -------
>
> The first of several patches related to resource usage monitoring. This patch provides a collection of utilities for use on Linux for reading stats from proc. It is used by both the lxc and proc resource collectors.
>
>
> This addresses bug MESOS-89.
> https://issues.apache.org/jira/browse/MESOS-89
>
>
> Diffs
> -----
>
> src/monitoring/linux/proc_utils.cpp PRE-CREATION
> src/tests/Makefile.in ea943f7
> src/tests/proc_utils_tests.cpp PRE-CREATION
> src/Makefile.in 516f128
> src/monitoring/linux/proc_utils.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3050/diff
>
>
> Testing
> -------
>
> Sanity tests have been written in src/tests/proc_utils_tests.cpp for all utility functions, and functions have been tested ad hoc.
>
>
> Thanks,
>
> Alex
>
>
Re: Review Request: Create utilities to collect information from the proc
filesystem
Posted by Charles Reiss <wo...@gmail.com>.
> On 2012-02-07 18:14:30, Charles Reiss wrote:
> >
>
> Sam Whitlock wrote:
> What target should these go into in Makefile.am? libmesos_no_third_party_la_SOURCES seems to be the one that all the source files are being added to.
libmesos_no_third_party_la would be fine for the non-test targets. (The test ones probably go into the mesos_tests_SOURCES.)
- Charles
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3050/#review4868
-----------------------------------------------------------
On 2011-12-24 04:13:40, Alex Degtiar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3050/
> -----------------------------------------------------------
>
> (Updated 2011-12-24 04:13:40)
>
>
> Review request for mesos.
>
>
> Summary
> -------
>
> The first of several patches related to resource usage monitoring. This patch provides a collection of utilities for use on Linux for reading stats from proc. It is used by both the lxc and proc resource collectors.
>
>
> This addresses bug MESOS-89.
> https://issues.apache.org/jira/browse/MESOS-89
>
>
> Diffs
> -----
>
> src/monitoring/linux/proc_utils.cpp PRE-CREATION
> src/tests/Makefile.in ea943f7
> src/tests/proc_utils_tests.cpp PRE-CREATION
> src/Makefile.in 516f128
> src/monitoring/linux/proc_utils.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3050/diff
>
>
> Testing
> -------
>
> Sanity tests have been written in src/tests/proc_utils_tests.cpp for all utility functions, and functions have been tested ad hoc.
>
>
> Thanks,
>
> Alex
>
>
Re: Review Request: Create utilities to collect information from the proc
filesystem
Posted by Charles Reiss <wo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3050/#review4868
-----------------------------------------------------------
src/Makefile.in
<https://reviews.apache.org/r/3050/#comment10719>
This needs to be merged into src/Makefile.am now.
src/tests/Makefile.in
<https://reviews.apache.org/r/3050/#comment10721>
This needs to be merged into src/Makefile.am
- Charles
On 2011-12-24 04:13:40, Alex Degtiar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3050/
> -----------------------------------------------------------
>
> (Updated 2011-12-24 04:13:40)
>
>
> Review request for mesos.
>
>
> Summary
> -------
>
> The first of several patches related to resource usage monitoring. This patch provides a collection of utilities for use on Linux for reading stats from proc. It is used by both the lxc and proc resource collectors.
>
>
> This addresses bug MESOS-89.
> https://issues.apache.org/jira/browse/MESOS-89
>
>
> Diffs
> -----
>
> src/monitoring/linux/proc_utils.cpp PRE-CREATION
> src/tests/Makefile.in ea943f7
> src/tests/proc_utils_tests.cpp PRE-CREATION
> src/Makefile.in 516f128
> src/monitoring/linux/proc_utils.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3050/diff
>
>
> Testing
> -------
>
> Sanity tests have been written in src/tests/proc_utils_tests.cpp for all utility functions, and functions have been tested ad hoc.
>
>
> Thanks,
>
> Alex
>
>
Re: Review Request: Create utilities to collect information from the proc
filesystem
Posted by Alex Degtiar <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3050/
-----------------------------------------------------------
(Updated 2011-12-24 04:13:40.408094)
Review request for mesos.
Changes
-------
I noticed a couple of unused imports. Fixed.
Summary
-------
The first of several patches related to resource usage monitoring. This patch provides a collection of utilities for use on Linux for reading stats from proc. It is used by both the lxc and proc resource collectors.
This addresses bug MESOS-89.
https://issues.apache.org/jira/browse/MESOS-89
Diffs (updated)
-----
src/monitoring/linux/proc_utils.cpp PRE-CREATION
src/tests/Makefile.in ea943f7
src/tests/proc_utils_tests.cpp PRE-CREATION
src/Makefile.in 516f128
src/monitoring/linux/proc_utils.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/3050/diff
Testing
-------
Sanity tests have been written in src/tests/proc_utils_tests.cpp for all utility functions, and functions have been tested ad hoc.
Thanks,
Alex
Re: Review Request: Create utilities to collect information from the proc
filesystem
Posted by Alex Degtiar <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3050/
-----------------------------------------------------------
(Updated 2011-12-21 03:24:51.902604)
Review request for mesos.
Changes
-------
Thanks for the feedback. I've made the requested changes. Please take another look.
Summary
-------
The first of several patches related to resource usage monitoring. This patch provides a collection of utilities for use on Linux for reading stats from proc. It is used by both the lxc and proc resource collectors.
This addresses bug MESOS-89.
https://issues.apache.org/jira/browse/MESOS-89
Diffs (updated)
-----
src/Makefile.in 516f128
src/monitoring/linux/proc_utils.hpp PRE-CREATION
src/monitoring/linux/proc_utils.cpp PRE-CREATION
src/tests/Makefile.in ea943f7
src/tests/proc_utils_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/3050/diff
Testing
-------
Sanity tests have been written in src/tests/proc_utils_tests.cpp for all utility functions, and functions have been tested ad hoc.
Thanks,
Alex
Re: Review Request: Create utilities to collect information from the proc
filesystem
Posted by Alex Degtiar <al...@gmail.com>.
> On 2011-12-08 18:48:30, Benjamin Hindman wrote:
> >
Also, as discussed, I created a monitoring/linux folder to put all linux-specific source files.
> On 2011-12-08 18:48:30, Benjamin Hindman wrote:
> > src/monitoring/proc_utils.hpp, line 33
> > <https://reviews.apache.org/r/3050/diff/2/?file=63110#file63110line33>
> >
> > Brace on newline.
Done.
> On 2011-12-08 18:48:30, Benjamin Hindman wrote:
> > src/monitoring/proc_utils.cpp, line 43
> > <https://reviews.apache.org/r/3050/diff/2/?file=63111#file63111line43>
> >
> > Brace on newline.
Done.
> On 2011-12-08 18:48:30, Benjamin Hindman wrote:
> > src/monitoring/proc_utils.cpp, line 60
> > <https://reviews.apache.org/r/3050/diff/2/?file=63111#file63111line60>
> >
> > Double newline between functions.
Done.
> On 2011-12-08 18:48:30, Benjamin Hindman wrote:
> > src/monitoring/proc_utils.hpp, lines 38-39
> > <https://reviews.apache.org/r/3050/diff/2/?file=63110#file63110line38>
> >
> > Use seconds or milliseconds _abstraction_ from common/seconds.hpp.
Done.
> On 2011-12-08 18:48:30, Benjamin Hindman wrote:
> > src/monitoring/proc_utils.hpp, line 53
> > <https://reviews.apache.org/r/3050/diff/2/?file=63110#file63110line53>
> >
> > Seconds or milliseconds type, here and everywhere else.
Done. Went with seconds for consistency with rest of Mesos.
> On 2011-12-08 18:48:30, Benjamin Hindman wrote:
> > src/monitoring/proc_utils.cpp, line 76
> > <https://reviews.apache.org/r/3050/diff/2/?file=63111#file63111line76>
> >
> > Add more spaces between logical blocks.
Done.
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3050/#review3745
-----------------------------------------------------------
On 2011-12-08 00:00:32, Alex Degtiar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3050/
> -----------------------------------------------------------
>
> (Updated 2011-12-08 00:00:32)
>
>
> Review request for mesos.
>
>
> Summary
> -------
>
> The first of several patches related to resource usage monitoring. This patch provides a collection of utilities for use on Linux for reading stats from proc. It is used by both the lxc and proc resource collectors.
>
>
> This addresses bug MESOS-89.
> https://issues.apache.org/jira/browse/MESOS-89
>
>
> Diffs
> -----
>
> src/tests/Makefile.in ea943f7
> src/tests/proc_utils_tests.cpp PRE-CREATION
> src/monitoring/proc_utils.cpp PRE-CREATION
> src/Makefile.in 516f128
> src/monitoring/proc_utils.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3050/diff
>
>
> Testing
> -------
>
> Sanity tests have been written in src/tests/proc_utils_tests.cpp for all utility functions, and functions have been tested ad hoc.
>
>
> Thanks,
>
> Alex
>
>
Re: Review Request: Create utilities to collect information from the proc
filesystem
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3050/#review3745
-----------------------------------------------------------
src/monitoring/proc_utils.hpp
<https://reviews.apache.org/r/3050/#comment8438>
Brace on newline.
src/monitoring/proc_utils.hpp
<https://reviews.apache.org/r/3050/#comment8439>
Use seconds or milliseconds _abstraction_ from common/seconds.hpp.
src/monitoring/proc_utils.hpp
<https://reviews.apache.org/r/3050/#comment8440>
Seconds or milliseconds type, here and everywhere else.
src/monitoring/proc_utils.cpp
<https://reviews.apache.org/r/3050/#comment8443>
Brace on newline.
src/monitoring/proc_utils.cpp
<https://reviews.apache.org/r/3050/#comment8444>
Double newline between functions.
src/monitoring/proc_utils.cpp
<https://reviews.apache.org/r/3050/#comment8445>
Add more spaces between logical blocks.
- Benjamin
On 2011-12-08 00:00:32, Alex Degtiar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3050/
> -----------------------------------------------------------
>
> (Updated 2011-12-08 00:00:32)
>
>
> Review request for mesos.
>
>
> Summary
> -------
>
> The first of several patches related to resource usage monitoring. This patch provides a collection of utilities for use on Linux for reading stats from proc. It is used by both the lxc and proc resource collectors.
>
>
> This addresses bug MESOS-89.
> https://issues.apache.org/jira/browse/MESOS-89
>
>
> Diffs
> -----
>
> src/tests/Makefile.in ea943f7
> src/tests/proc_utils_tests.cpp PRE-CREATION
> src/monitoring/proc_utils.cpp PRE-CREATION
> src/Makefile.in 516f128
> src/monitoring/proc_utils.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3050/diff
>
>
> Testing
> -------
>
> Sanity tests have been written in src/tests/proc_utils_tests.cpp for all utility functions, and functions have been tested ad hoc.
>
>
> Thanks,
>
> Alex
>
>
Re: Review Request: Create utilities to collect information from the proc
filesystem
Posted by Alex Degtiar <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3050/
-----------------------------------------------------------
(Updated 2011-12-08 00:00:32.699023)
Review request for mesos.
Changes
-------
I've address the comments and updated the diff. PTAL.
Summary
-------
The first of several patches related to resource usage monitoring. This patch provides a collection of utilities for use on Linux for reading stats from proc. It is used by both the lxc and proc resource collectors.
This addresses bug MESOS-89.
https://issues.apache.org/jira/browse/MESOS-89
Diffs (updated)
-----
src/tests/Makefile.in ea943f7
src/tests/proc_utils_tests.cpp PRE-CREATION
src/monitoring/proc_utils.cpp PRE-CREATION
src/Makefile.in 516f128
src/monitoring/proc_utils.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/3050/diff
Testing
-------
Sanity tests have been written in src/tests/proc_utils_tests.cpp for all utility functions, and functions have been tested ad hoc.
Thanks,
Alex
Re: Review Request: Create utilities to collect information from the proc
filesystem
Posted by Alex Degtiar <al...@gmail.com>.
> On 2011-12-07 21:10:13, Charles Reiss wrote:
> > src/Makefile.in, line 137
> > <https://reviews.apache.org/r/3050/diff/1/?file=62806#file62806line137>
> >
> > MONITORING_OBJ should be set to empty string unconditionally first.
Done.
> On 2011-12-07 21:10:13, Charles Reiss wrote:
> > src/monitoring/proc_utils.hpp, line 39
> > <https://reviews.apache.org/r/3050/diff/1/?file=62807#file62807line39>
> >
> > Please state whether RSS or VSIZE or something else.
Done.
> On 2011-12-07 21:10:13, Charles Reiss wrote:
> > src/monitoring/proc_utils.cpp, line 22
> > <https://reviews.apache.org/r/3050/diff/1/?file=62808#file62808line22>
> >
> > include all system .h headers either before or after standard C++ headers.
Done.
> On 2011-12-07 21:10:13, Charles Reiss wrote:
> > src/monitoring/proc_utils.cpp, line 61
> > <https://reviews.apache.org/r/3050/diff/1/?file=62808#file62808line61>
> >
> > Please make these file-scoped (static or anonymous namespace) or put them in the header file.
Done.
> On 2011-12-07 21:10:13, Charles Reiss wrote:
> > src/monitoring/proc_utils.cpp, line 81
> > <https://reviews.apache.org/r/3050/diff/1/?file=62808#file62808line81>
> >
> > O is a pretty bad variable name.
Done. I was using outdated documentation where this field was a hard-coded 0 in proc. It is now num_threads.
> On 2011-12-07 21:10:13, Charles Reiss wrote:
> > src/monitoring/proc_utils.cpp, line 85
> > <https://reviews.apache.org/r/3050/diff/1/?file=62808#file62808line85>
> >
> > Check whether this read succeeds.
Done.
> On 2011-12-07 21:10:13, Charles Reiss wrote:
> > src/monitoring/proc_utils.hpp, line 37
> > <https://reviews.apache.org/r/3050/diff/1/?file=62807#file62807line37>
> >
> > Why milliseconds? libprocess uses seconds since some time, and I think USER_HZ usually (often?) isn't 1000.
Sam and I decided on milliseconds (since epoch when appropriate) for all measured times because it was the granularity closest to the times in various sources, and having one unit used consistently seemed cleaner. Would it make it more consistent with the rest of Mesos if we scale it up to seconds?
For reference, granularity of various times we used:
process start time (used for duration of initial read)- jiffies (4ms +/- depending on system HZ)
boot time (used for start time to make it since epoch)- seconds
current time (used for duration) - nanoseconds/milliseconds since epoch (depends on system)
cpu time (proc)- clock ticks (10ms +/- depending on system SC_CLK_TCK)
cpu time (lxc)- nanoseconds (unit returned in, not sure of actual granularity)
Period of measurement - potential lower bound on a single machine is probably in the millisecond range.
> On 2011-12-07 21:10:13, Charles Reiss wrote:
> > src/monitoring/proc_utils.hpp, line 33
> > <https://reviews.apache.org/r/3050/diff/1/?file=62807#file62807line33>
> >
> > Why strings? (and elsewhere)
> >
I was deciding between using unsigned integers, something like the pid_t type, and strings. I ultimately decided on strings because they supported special non-integer 'pids' proc lets you query (i.e. "self"). This was convenient for testing.
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3050/#review3712
-----------------------------------------------------------
On 2011-12-08 00:00:32, Alex Degtiar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3050/
> -----------------------------------------------------------
>
> (Updated 2011-12-08 00:00:32)
>
>
> Review request for mesos.
>
>
> Summary
> -------
>
> The first of several patches related to resource usage monitoring. This patch provides a collection of utilities for use on Linux for reading stats from proc. It is used by both the lxc and proc resource collectors.
>
>
> This addresses bug MESOS-89.
> https://issues.apache.org/jira/browse/MESOS-89
>
>
> Diffs
> -----
>
> src/tests/Makefile.in ea943f7
> src/tests/proc_utils_tests.cpp PRE-CREATION
> src/monitoring/proc_utils.cpp PRE-CREATION
> src/Makefile.in 516f128
> src/monitoring/proc_utils.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3050/diff
>
>
> Testing
> -------
>
> Sanity tests have been written in src/tests/proc_utils_tests.cpp for all utility functions, and functions have been tested ad hoc.
>
>
> Thanks,
>
> Alex
>
>
Re: Review Request: Create utilities to collect information from the proc
filesystem
Posted by Alex Degtiar <al...@gmail.com>.
> On 2011-12-07 21:10:13, Charles Reiss wrote:
> > src/monitoring/proc_utils.hpp, line 37
> > <https://reviews.apache.org/r/3050/diff/1/?file=62807#file62807line37>
> >
> > Why milliseconds? libprocess uses seconds since some time, and I think USER_HZ usually (often?) isn't 1000.
>
> Alex Degtiar wrote:
> Sam and I decided on milliseconds (since epoch when appropriate) for all measured times because it was the granularity closest to the times in various sources, and having one unit used consistently seemed cleaner. Would it make it more consistent with the rest of Mesos if we scale it up to seconds?
>
> For reference, granularity of various times we used:
> process start time (used for duration of initial read)- jiffies (4ms +/- depending on system HZ)
> boot time (used for start time to make it since epoch)- seconds
> current time (used for duration) - nanoseconds/milliseconds since epoch (depends on system)
> cpu time (proc)- clock ticks (10ms +/- depending on system SC_CLK_TCK)
> cpu time (lxc)- nanoseconds (unit returned in, not sure of actual granularity)
> Period of measurement - potential lower bound on a single machine is probably in the millisecond range.
>
> Charles Reiss wrote:
> Assuming process::Clock::now() can be counted on to be in time since the epoch (I don't actually know if this is an API gaurentee), then I think you should have that replace getCurrentTime() and thus use seconds since the epoch for everything.
>
Done.
> On 2011-12-07 21:10:13, Charles Reiss wrote:
> > src/monitoring/proc_utils.hpp, line 33
> > <https://reviews.apache.org/r/3050/diff/1/?file=62807#file62807line33>
> >
> > Why strings? (and elsewhere)
> >
>
> Alex Degtiar wrote:
> I was deciding between using unsigned integers, something like the pid_t type, and strings. I ultimately decided on strings because they supported special non-integer 'pids' proc lets you query (i.e. "self"). This was convenient for testing.
>
> Charles Reiss wrote:
> I don't think there's any special "PID" other than "self", and that's easy replaced with getpid() (which your test already needs).
Replaced with pid_t and getpid().
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3050/#review3712
-----------------------------------------------------------
On 2011-12-08 00:00:32, Alex Degtiar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3050/
> -----------------------------------------------------------
>
> (Updated 2011-12-08 00:00:32)
>
>
> Review request for mesos.
>
>
> Summary
> -------
>
> The first of several patches related to resource usage monitoring. This patch provides a collection of utilities for use on Linux for reading stats from proc. It is used by both the lxc and proc resource collectors.
>
>
> This addresses bug MESOS-89.
> https://issues.apache.org/jira/browse/MESOS-89
>
>
> Diffs
> -----
>
> src/tests/Makefile.in ea943f7
> src/tests/proc_utils_tests.cpp PRE-CREATION
> src/monitoring/proc_utils.cpp PRE-CREATION
> src/Makefile.in 516f128
> src/monitoring/proc_utils.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3050/diff
>
>
> Testing
> -------
>
> Sanity tests have been written in src/tests/proc_utils_tests.cpp for all utility functions, and functions have been tested ad hoc.
>
>
> Thanks,
>
> Alex
>
>
Re: Review Request: Create utilities to collect information from the proc
filesystem
Posted by Charles Reiss <wo...@gmail.com>.
> On 2011-12-07 21:10:13, Charles Reiss wrote:
> > src/monitoring/proc_utils.hpp, line 33
> > <https://reviews.apache.org/r/3050/diff/1/?file=62807#file62807line33>
> >
> > Why strings? (and elsewhere)
> >
>
> Alex Degtiar wrote:
> I was deciding between using unsigned integers, something like the pid_t type, and strings. I ultimately decided on strings because they supported special non-integer 'pids' proc lets you query (i.e. "self"). This was convenient for testing.
I don't think there's any special "PID" other than "self", and that's easy replaced with getpid() (which your test already needs).
> On 2011-12-07 21:10:13, Charles Reiss wrote:
> > src/monitoring/proc_utils.hpp, line 37
> > <https://reviews.apache.org/r/3050/diff/1/?file=62807#file62807line37>
> >
> > Why milliseconds? libprocess uses seconds since some time, and I think USER_HZ usually (often?) isn't 1000.
>
> Alex Degtiar wrote:
> Sam and I decided on milliseconds (since epoch when appropriate) for all measured times because it was the granularity closest to the times in various sources, and having one unit used consistently seemed cleaner. Would it make it more consistent with the rest of Mesos if we scale it up to seconds?
>
> For reference, granularity of various times we used:
> process start time (used for duration of initial read)- jiffies (4ms +/- depending on system HZ)
> boot time (used for start time to make it since epoch)- seconds
> current time (used for duration) - nanoseconds/milliseconds since epoch (depends on system)
> cpu time (proc)- clock ticks (10ms +/- depending on system SC_CLK_TCK)
> cpu time (lxc)- nanoseconds (unit returned in, not sure of actual granularity)
> Period of measurement - potential lower bound on a single machine is probably in the millisecond range.
Assuming process::Clock::now() can be counted on to be in time since the epoch (I don't actually know if this is an API gaurentee), then I think you should have that replace getCurrentTime() and thus use seconds since the epoch for everything.
- Charles
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3050/#review3712
-----------------------------------------------------------
On 2011-12-08 00:00:32, Alex Degtiar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3050/
> -----------------------------------------------------------
>
> (Updated 2011-12-08 00:00:32)
>
>
> Review request for mesos.
>
>
> Summary
> -------
>
> The first of several patches related to resource usage monitoring. This patch provides a collection of utilities for use on Linux for reading stats from proc. It is used by both the lxc and proc resource collectors.
>
>
> This addresses bug MESOS-89.
> https://issues.apache.org/jira/browse/MESOS-89
>
>
> Diffs
> -----
>
> src/tests/Makefile.in ea943f7
> src/tests/proc_utils_tests.cpp PRE-CREATION
> src/monitoring/proc_utils.cpp PRE-CREATION
> src/Makefile.in 516f128
> src/monitoring/proc_utils.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3050/diff
>
>
> Testing
> -------
>
> Sanity tests have been written in src/tests/proc_utils_tests.cpp for all utility functions, and functions have been tested ad hoc.
>
>
> Thanks,
>
> Alex
>
>
Re: Review Request: Create utilities to collect information from the proc
filesystem
Posted by Charles Reiss <wo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3050/#review3712
-----------------------------------------------------------
src/Makefile.in
<https://reviews.apache.org/r/3050/#comment8323>
MONITORING_OBJ should be set to empty string unconditionally first.
src/monitoring/proc_utils.hpp
<https://reviews.apache.org/r/3050/#comment8319>
Why strings? (and elsewhere)
src/monitoring/proc_utils.hpp
<https://reviews.apache.org/r/3050/#comment8309>
Why milliseconds? libprocess uses seconds since some time, and I think USER_HZ usually (often?) isn't 1000.
src/monitoring/proc_utils.hpp
<https://reviews.apache.org/r/3050/#comment8320>
Please state whether RSS or VSIZE or something else.
src/monitoring/proc_utils.cpp
<https://reviews.apache.org/r/3050/#comment8316>
include all system .h headers either before or after standard C++ headers.
src/monitoring/proc_utils.cpp
<https://reviews.apache.org/r/3050/#comment8315>
Please make these file-scoped (static or anonymous namespace) or put them in the header file.
src/monitoring/proc_utils.cpp
<https://reviews.apache.org/r/3050/#comment8311>
O is a pretty bad variable name.
src/monitoring/proc_utils.cpp
<https://reviews.apache.org/r/3050/#comment8313>
Check whether this read succeeds.
- Charles
On 2011-12-07 07:08:06, Alex Degtiar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3050/
> -----------------------------------------------------------
>
> (Updated 2011-12-07 07:08:06)
>
>
> Review request for mesos.
>
>
> Summary
> -------
>
> The first of several patches related to resource usage monitoring. This patch provides a collection of utilities for use on Linux for reading stats from proc. It is used by both the lxc and proc resource collectors.
>
>
> This addresses bug MESOS-89.
> https://issues.apache.org/jira/browse/MESOS-89
>
>
> Diffs
> -----
>
> src/tests/Makefile.in ea943f7
> src/tests/proc_utils_tests.cpp PRE-CREATION
> src/Makefile.in 516f128
> src/monitoring/proc_utils.hpp PRE-CREATION
> src/monitoring/proc_utils.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3050/diff
>
>
> Testing
> -------
>
> Sanity tests have been written in src/tests/proc_utils_tests.cpp for all utility functions, and functions have been tested ad hoc.
>
>
> Thanks,
>
> Alex
>
>