You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Niklas Nielsen <ni...@qni.dk> on 2014/01/24 20:04:25 UTC
Review Request 17325: Added new /system HTTP handler and /system/stats.json
end-point.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17325/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
Bugs: MESOS-581
https://issues.apache.org/jira/browse/MESOS-581
Repository: mesos-git
Description
-------
Skipped #16631 and let this RR take over.
This patch adds a /system HTTP handler to libprocess for node/system centric metrics and tracing.
The first metric exposed is average system load (usual uptime(1) style;1, 5 and
15 minutes averages) along with local cpu count. The fields are named
cpus_total, avg_load_XXmin.
Further more, help text should explain fields in /system/stats.json
Diffs
-----
3rdparty/libprocess/Makefile.am 40f01a7
3rdparty/libprocess/include/process/system.hpp PRE-CREATION
3rdparty/libprocess/src/process.cpp bc7a1c5
Diff: https://reviews.apache.org/r/17325/diff/
Testing
-------
make check and functional testing of end-point.
Thanks,
Niklas Nielsen
Re: Review Request 17325: Added new /system HTTP handler and
/system/stats.json end-point.
Posted by Niklas Nielsen <ni...@qni.dk>.
> On Feb. 3, 2014, 3:34 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/include/process/system.hpp, lines 54-66
> > <https://reviews.apache.org/r/17325/diff/5/?file=463622#file463622line54>
> >
> > This might get bitten by the same bug that is referenced in constants.hpp, statistics.hpp, timeseries.hpp in which the constants were not being initialized when compiled with gcc-4.1.2.
> >
> > Perhaps this is immune to the bug since it's a static _member_. This won't matter once we have C++11 required and can remove support for older compilers.
> >
> > In duration.hpp we made max, min static methods to avoid this issue.
Yeah, I did think about the static keyword but that would AFAIK generate a compile error for a static member variable.
Should we revert to having this in process.cpp instead?
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17325/#review33529
-----------------------------------------------------------
On Feb. 3, 2014, 2:10 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17325/
> -----------------------------------------------------------
>
> (Updated Feb. 3, 2014, 2:10 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-581
> https://issues.apache.org/jira/browse/MESOS-581
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Skipped #16631 and let this RR take over.
>
> This patch adds a /system HTTP handler to libprocess for node/system centric metrics and tracing.
>
> The first metric exposed is average system load (usual uptime(1) style;1, 5 and
> 15 minutes averages) along with local cpu count. The fields are named
> cpus_total, avg_load_XXmin.
>
> Further more, help text should explain fields in /system/stats.json
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am bbd17cc
> 3rdparty/libprocess/include/process/system.hpp PRE-CREATION
> 3rdparty/libprocess/src/process.cpp 1083a35
>
> Diff: https://reviews.apache.org/r/17325/diff/
>
>
> Testing
> -------
>
> make check and functional testing of end-point.
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 17325: Added new /system HTTP handler and
/system/stats.json end-point.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17325/#review33529
-----------------------------------------------------------
3rdparty/libprocess/include/process/system.hpp
<https://reviews.apache.org/r/17325/#comment63042>
This might get bitten by the same bug that is referenced in constants.hpp, statistics.hpp, timeseries.hpp in which the constants were not being initialized when compiled with gcc-4.1.2.
Perhaps this is immune to the bug since it's a static _member_. This won't matter once we have C++11 required and can remove support for older compilers.
In duration.hpp we made max, min static methods to avoid this issue.
- Ben Mahler
On Feb. 3, 2014, 10:10 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17325/
> -----------------------------------------------------------
>
> (Updated Feb. 3, 2014, 10:10 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-581
> https://issues.apache.org/jira/browse/MESOS-581
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Skipped #16631 and let this RR take over.
>
> This patch adds a /system HTTP handler to libprocess for node/system centric metrics and tracing.
>
> The first metric exposed is average system load (usual uptime(1) style;1, 5 and
> 15 minutes averages) along with local cpu count. The fields are named
> cpus_total, avg_load_XXmin.
>
> Further more, help text should explain fields in /system/stats.json
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am bbd17cc
> 3rdparty/libprocess/include/process/system.hpp PRE-CREATION
> 3rdparty/libprocess/src/process.cpp 1083a35
>
> Diff: https://reviews.apache.org/r/17325/diff/
>
>
> Testing
> -------
>
> make check and functional testing of end-point.
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 17325: Added new /system HTTP handler and
/system/stats.json end-point.
Posted by Ben Mahler <be...@gmail.com>.
> On Feb. 4, 2014, 12:10 a.m., Benjamin Hindman wrote:
> > One high level comment, I think we need to decide if we want to "reserve" these names in libprocess. It might make sense to name these things __system__ and __profiler__ similar to how __processes__ and __gc__ was done.
SGTM
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17325/#review33535
-----------------------------------------------------------
On Feb. 3, 2014, 10:10 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17325/
> -----------------------------------------------------------
>
> (Updated Feb. 3, 2014, 10:10 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-581
> https://issues.apache.org/jira/browse/MESOS-581
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Skipped #16631 and let this RR take over.
>
> This patch adds a /system HTTP handler to libprocess for node/system centric metrics and tracing.
>
> The first metric exposed is average system load (usual uptime(1) style;1, 5 and
> 15 minutes averages) along with local cpu count. The fields are named
> cpus_total, avg_load_XXmin.
>
> Further more, help text should explain fields in /system/stats.json
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am bbd17cc
> 3rdparty/libprocess/include/process/system.hpp PRE-CREATION
> 3rdparty/libprocess/src/process.cpp 1083a35
>
> Diff: https://reviews.apache.org/r/17325/diff/
>
>
> Testing
> -------
>
> make check and functional testing of end-point.
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 17325: Added new /system HTTP handler and
/system/stats.json end-point.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17325/#review33535
-----------------------------------------------------------
One high level comment, I think we need to decide if we want to "reserve" these names in libprocess. It might make sense to name these things __system__ and __profiler__ similar to how __processes__ and __gc__ was done.
- Benjamin Hindman
On Feb. 3, 2014, 10:10 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17325/
> -----------------------------------------------------------
>
> (Updated Feb. 3, 2014, 10:10 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-581
> https://issues.apache.org/jira/browse/MESOS-581
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Skipped #16631 and let this RR take over.
>
> This patch adds a /system HTTP handler to libprocess for node/system centric metrics and tracing.
>
> The first metric exposed is average system load (usual uptime(1) style;1, 5 and
> 15 minutes averages) along with local cpu count. The fields are named
> cpus_total, avg_load_XXmin.
>
> Further more, help text should explain fields in /system/stats.json
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am bbd17cc
> 3rdparty/libprocess/include/process/system.hpp PRE-CREATION
> 3rdparty/libprocess/src/process.cpp 1083a35
>
> Diff: https://reviews.apache.org/r/17325/diff/
>
>
> Testing
> -------
>
> make check and functional testing of end-point.
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 17325: Added new /system HTTP handler and
/system/stats.json end-point.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17325/
-----------------------------------------------------------
(Updated Feb. 3, 2014, 2:10 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
Changes
-------
Addressed BenM's comments.
Bugs: MESOS-581
https://issues.apache.org/jira/browse/MESOS-581
Repository: mesos-git
Description
-------
Skipped #16631 and let this RR take over.
This patch adds a /system HTTP handler to libprocess for node/system centric metrics and tracing.
The first metric exposed is average system load (usual uptime(1) style;1, 5 and
15 minutes averages) along with local cpu count. The fields are named
cpus_total, avg_load_XXmin.
Further more, help text should explain fields in /system/stats.json
Diffs (updated)
-----
3rdparty/libprocess/Makefile.am bbd17cc
3rdparty/libprocess/include/process/system.hpp PRE-CREATION
3rdparty/libprocess/src/process.cpp 1083a35
Diff: https://reviews.apache.org/r/17325/diff/
Testing
-------
make check and functional testing of end-point.
Thanks,
Niklas Nielsen
Re: Review Request 17325: Added new /system HTTP handler and
/system/stats.json end-point.
Posted by Ben Mahler <be...@gmail.com>.
> On Feb. 3, 2014, 7:51 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/include/process/system.hpp, line 35
> > <https://reviews.apache.org/r/17325/diff/4/?file=457110#file457110line35>
> >
> > Consider a static function as used in Duration to move the help string to the header file.
(See Duration::min() and Duration::max(), we used static functions instead of static members to avoid the need for definitions in .cpp files)
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17325/#review33456
-----------------------------------------------------------
On Jan. 30, 2014, 11:46 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17325/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2014, 11:46 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-581
> https://issues.apache.org/jira/browse/MESOS-581
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Skipped #16631 and let this RR take over.
>
> This patch adds a /system HTTP handler to libprocess for node/system centric metrics and tracing.
>
> The first metric exposed is average system load (usual uptime(1) style;1, 5 and
> 15 minutes averages) along with local cpu count. The fields are named
> cpus_total, avg_load_XXmin.
>
> Further more, help text should explain fields in /system/stats.json
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am bbd17cc
> 3rdparty/libprocess/include/process/system.hpp PRE-CREATION
> 3rdparty/libprocess/src/process.cpp 1083a35
>
> Diff: https://reviews.apache.org/r/17325/diff/
>
>
> Testing
> -------
>
> make check and functional testing of end-point.
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 17325: Added new /system HTTP handler and
/system/stats.json end-point.
Posted by Niklas Nielsen <ni...@qni.dk>.
> On Feb. 3, 2014, 11:51 a.m., Ben Mahler wrote:
> >
Thanks for the reviews Ben! I will go ahead and commit this.
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17325/#review33456
-----------------------------------------------------------
On Feb. 3, 2014, 2:10 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17325/
> -----------------------------------------------------------
>
> (Updated Feb. 3, 2014, 2:10 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-581
> https://issues.apache.org/jira/browse/MESOS-581
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Skipped #16631 and let this RR take over.
>
> This patch adds a /system HTTP handler to libprocess for node/system centric metrics and tracing.
>
> The first metric exposed is average system load (usual uptime(1) style;1, 5 and
> 15 minutes averages) along with local cpu count. The fields are named
> cpus_total, avg_load_XXmin.
>
> Further more, help text should explain fields in /system/stats.json
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am bbd17cc
> 3rdparty/libprocess/include/process/system.hpp PRE-CREATION
> 3rdparty/libprocess/src/process.cpp 1083a35
>
> Diff: https://reviews.apache.org/r/17325/diff/
>
>
> Testing
> -------
>
> make check and functional testing of end-point.
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 17325: Added new /system HTTP handler and
/system/stats.json end-point.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17325/#review33456
-----------------------------------------------------------
Ship it!
3rdparty/libprocess/include/process/system.hpp
<https://reviews.apache.org/r/17325/#comment62891>
kill this line
3rdparty/libprocess/include/process/system.hpp
<https://reviews.apache.org/r/17325/#comment62892>
kill this line
3rdparty/libprocess/include/process/system.hpp
<https://reviews.apache.org/r/17325/#comment62929>
How about this slightly different version:
The System process provides HTTP endpoints for retrieving system metrics, such as CPU load and memory usage. This is started by default during the initialization of libprocess.
3rdparty/libprocess/include/process/system.hpp
<https://reviews.apache.org/r/17325/#comment62930>
Consider a static function as used in Duration to move the help string to the header file.
- Ben Mahler
On Jan. 30, 2014, 11:46 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17325/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2014, 11:46 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-581
> https://issues.apache.org/jira/browse/MESOS-581
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Skipped #16631 and let this RR take over.
>
> This patch adds a /system HTTP handler to libprocess for node/system centric metrics and tracing.
>
> The first metric exposed is average system load (usual uptime(1) style;1, 5 and
> 15 minutes averages) along with local cpu count. The fields are named
> cpus_total, avg_load_XXmin.
>
> Further more, help text should explain fields in /system/stats.json
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am bbd17cc
> 3rdparty/libprocess/include/process/system.hpp PRE-CREATION
> 3rdparty/libprocess/src/process.cpp 1083a35
>
> Diff: https://reviews.apache.org/r/17325/diff/
>
>
> Testing
> -------
>
> make check and functional testing of end-point.
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 17325: Added new /system HTTP handler and
/system/stats.json end-point.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17325/
-----------------------------------------------------------
(Updated Jan. 30, 2014, 3:46 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
Changes
-------
Addressed BenM's comments.
Bugs: MESOS-581
https://issues.apache.org/jira/browse/MESOS-581
Repository: mesos-git
Description
-------
Skipped #16631 and let this RR take over.
This patch adds a /system HTTP handler to libprocess for node/system centric metrics and tracing.
The first metric exposed is average system load (usual uptime(1) style;1, 5 and
15 minutes averages) along with local cpu count. The fields are named
cpus_total, avg_load_XXmin.
Further more, help text should explain fields in /system/stats.json
Diffs (updated)
-----
3rdparty/libprocess/Makefile.am bbd17cc
3rdparty/libprocess/include/process/system.hpp PRE-CREATION
3rdparty/libprocess/src/process.cpp 1083a35
Diff: https://reviews.apache.org/r/17325/diff/
Testing
-------
make check and functional testing of end-point.
Thanks,
Niklas Nielsen
Re: Review Request 17325: Added new /system HTTP handler and
/system/stats.json end-point.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17325/#review33244
-----------------------------------------------------------
Looks good!
3rdparty/libprocess/include/process/system.hpp
<https://reviews.apache.org/r/17325/#comment62630>
Did you need this?
3rdparty/libprocess/include/process/system.hpp
<https://reviews.apache.org/r/17325/#comment62631>
Did you need this?
3rdparty/libprocess/include/process/system.hpp
<https://reviews.apache.org/r/17325/#comment62626>
Some documentation here would be helpful for those looking at this header and trying to figure out how to use it. Hint: They probably don't want to use this explicitly but rather just know that this is available by default in libprocess.
Two things we should leave for another time but I think are worth mentioning:
1. I'm not sure there's a reason to be exposing these (profiler.hpp, system.hpp) in 'include' but let's leave that for another time.
2. Since libprocess is not a header-only library, we should consider placing implementation in .cpp files to speed up compile times.
3rdparty/libprocess/src/process.cpp
<https://reviews.apache.org/r/17325/#comment62629>
End with a period. :)
- Ben Mahler
On Jan. 24, 2014, 7:31 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17325/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2014, 7:31 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-581
> https://issues.apache.org/jira/browse/MESOS-581
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Skipped #16631 and let this RR take over.
>
> This patch adds a /system HTTP handler to libprocess for node/system centric metrics and tracing.
>
> The first metric exposed is average system load (usual uptime(1) style;1, 5 and
> 15 minutes averages) along with local cpu count. The fields are named
> cpus_total, avg_load_XXmin.
>
> Further more, help text should explain fields in /system/stats.json
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am 40f01a7
> 3rdparty/libprocess/include/process/system.hpp PRE-CREATION
> 3rdparty/libprocess/src/process.cpp bc7a1c5
>
> Diff: https://reviews.apache.org/r/17325/diff/
>
>
> Testing
> -------
>
> make check and functional testing of end-point.
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 17325: Added new /system HTTP handler and
/system/stats.json end-point.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17325/
-----------------------------------------------------------
(Updated Jan. 24, 2014, 11:31 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
Bugs: MESOS-581
https://issues.apache.org/jira/browse/MESOS-581
Repository: mesos-git
Description
-------
Skipped #16631 and let this RR take over.
This patch adds a /system HTTP handler to libprocess for node/system centric metrics and tracing.
The first metric exposed is average system load (usual uptime(1) style;1, 5 and
15 minutes averages) along with local cpu count. The fields are named
cpus_total, avg_load_XXmin.
Further more, help text should explain fields in /system/stats.json
Diffs (updated)
-----
3rdparty/libprocess/Makefile.am 40f01a7
3rdparty/libprocess/include/process/system.hpp PRE-CREATION
3rdparty/libprocess/src/process.cpp bc7a1c5
Diff: https://reviews.apache.org/r/17325/diff/
Testing
-------
make check and functional testing of end-point.
Thanks,
Niklas Nielsen