You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Dominic Hamon <dh...@twopensource.com> on 2014/05/02 04:19:19 UTC
Re: Review Request 19499: Ported slave statistics over to new metrics
library.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19499/
-----------------------------------------------------------
(Updated May 1, 2014, 7:19 p.m.)
Review request for mesos and Ben Mahler.
Changes
-------
rebased and updated to match benm's comments on 19504
Bugs: MESOS-1133
https://issues.apache.org/jira/browse/MESOS-1133
Repository: mesos-git
Description
-------
see summary
Diffs (updated)
-----
src/slave/slave.hpp ed20dca70cfbbd11dcdb35a074dc1777721f6ca5
src/slave/slave.cpp cb80609ba421b3b9a4664e600f0e53ecab8574c4
Diff: https://reviews.apache.org/r/19499/diff/
Testing
-------
make check. ran locally and compared stats.json endpoint to /metrics/snapshot endpoint.
Thanks,
Dominic Hamon
Re: Review Request 19499: Ported slave statistics over to new metrics
library.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19499/#review42556
-----------------------------------------------------------
Patch looks great!
Reviews applied: [19499]
All tests passed.
- Mesos ReviewBot
On May 9, 2014, 12:58 a.m., Dominic Hamon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19499/
> -----------------------------------------------------------
>
> (Updated May 9, 2014, 12:58 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1133
> https://issues.apache.org/jira/browse/MESOS-1133
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary
>
>
> Diffs
> -----
>
> src/slave/slave.hpp ed20dca70cfbbd11dcdb35a074dc1777721f6ca5
> src/slave/slave.cpp ba4bd731bc21cdf853c3d390296252ad4d0901cb
> src/tests/slave_tests.cpp 6526952528aaf69d77007b37cf96541162290c33
>
> Diff: https://reviews.apache.org/r/19499/diff/
>
>
> Testing
> -------
>
> make check. ran locally and compared stats.json endpoint to /metrics/snapshot endpoint.
>
>
> Thanks,
>
> Dominic Hamon
>
>
Re: Review Request 19499: Ported slave statistics over to new metrics
library.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19499/#review42593
-----------------------------------------------------------
Ship it!
Will get this committed! I'll clean up the test a bit and re-order the metrics a bit per my comment below.
src/slave/slave.hpp
<https://reviews.apache.org/r/19499/#comment76407>
How about we re-order these a bit by how "fundamental" these are:
process::metrics::Gauge uptime_secs;
process::metrics::Gauge registered;
process::metrics::Counter recovery_errors;
process::metrics::Gauge active_frameworks;
// TODO(dhamon): Add tasks Gauges.
process::metrics::Counter valid_status_updates;
process::metrics::Counter invalid_status_updates;
process::metrics::Counter valid_framework_messages;
process::metrics::Counter invalid_framework_messages;
src/tests/slave_tests.cpp
<https://reviews.apache.org/r/19499/#comment76409>
We can remove these.
- Ben Mahler
On May 9, 2014, 5:16 p.m., Dominic Hamon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19499/
> -----------------------------------------------------------
>
> (Updated May 9, 2014, 5:16 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1133 and MESOS-1332
> https://issues.apache.org/jira/browse/MESOS-1133
> https://issues.apache.org/jira/browse/MESOS-1332
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary
>
>
> Diffs
> -----
>
> src/slave/slave.hpp ed20dca70cfbbd11dcdb35a074dc1777721f6ca5
> src/slave/slave.cpp ba4bd731bc21cdf853c3d390296252ad4d0901cb
> src/tests/slave_tests.cpp 6526952528aaf69d77007b37cf96541162290c33
>
> Diff: https://reviews.apache.org/r/19499/diff/
>
>
> Testing
> -------
>
> make check. ran locally and compared stats.json endpoint to /metrics/snapshot endpoint.
>
>
> Thanks,
>
> Dominic Hamon
>
>
Re: Review Request 19499: Ported slave statistics over to new metrics
library.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19499/
-----------------------------------------------------------
(Updated May 9, 2014, 10:16 a.m.)
Review request for mesos and Ben Mahler.
Changes
-------
updated per MESOS-1332.
Bugs: MESOS-1133 and MESOS-1332
https://issues.apache.org/jira/browse/MESOS-1133
https://issues.apache.org/jira/browse/MESOS-1332
Repository: mesos-git
Description
-------
see summary
Diffs (updated)
-----
src/slave/slave.hpp ed20dca70cfbbd11dcdb35a074dc1777721f6ca5
src/slave/slave.cpp ba4bd731bc21cdf853c3d390296252ad4d0901cb
src/tests/slave_tests.cpp 6526952528aaf69d77007b37cf96541162290c33
Diff: https://reviews.apache.org/r/19499/diff/
Testing
-------
make check. ran locally and compared stats.json endpoint to /metrics/snapshot endpoint.
Thanks,
Dominic Hamon
Re: Review Request 19499: Ported slave statistics over to new metrics
library.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19499/
-----------------------------------------------------------
(Updated May 8, 2014, 5:58 p.m.)
Review request for mesos and Ben Mahler.
Changes
-------
removed tasks counters as it's likely they should become gauges (MESOS-1332)
Bugs: MESOS-1133
https://issues.apache.org/jira/browse/MESOS-1133
Repository: mesos-git
Description
-------
see summary
Diffs (updated)
-----
src/slave/slave.hpp ed20dca70cfbbd11dcdb35a074dc1777721f6ca5
src/slave/slave.cpp ba4bd731bc21cdf853c3d390296252ad4d0901cb
src/tests/slave_tests.cpp 6526952528aaf69d77007b37cf96541162290c33
Diff: https://reviews.apache.org/r/19499/diff/
Testing
-------
make check. ran locally and compared stats.json endpoint to /metrics/snapshot endpoint.
Thanks,
Dominic Hamon
Re: Review Request 19499: Ported slave statistics over to new metrics
library.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19499/
-----------------------------------------------------------
(Updated May 8, 2014, 11:47 a.m.)
Review request for mesos and Ben Mahler.
Changes
-------
added test to ensure stats are present as expected.
Bugs: MESOS-1133
https://issues.apache.org/jira/browse/MESOS-1133
Repository: mesos-git
Description
-------
see summary
Diffs (updated)
-----
src/slave/slave.hpp ed20dca70cfbbd11dcdb35a074dc1777721f6ca5
src/slave/slave.cpp 2a48266510a600349692780ea39cc98c3e12da30
src/tests/slave_tests.cpp 09824550101577f8c9cf65c18e4980b74494a7d7
Diff: https://reviews.apache.org/r/19499/diff/
Testing
-------
make check. ran locally and compared stats.json endpoint to /metrics/snapshot endpoint.
Thanks,
Dominic Hamon
Re: Review Request 19499: Ported slave statistics over to new metrics
library.
Posted by Dominic Hamon <dh...@twopensource.com>.
> On May 8, 2014, 11:13 a.m., Ben Mahler wrote:
> > src/slave/slave.cpp, lines 3087-3107
> > <https://reviews.apache.org/r/19499/diff/9/?file=573410#file573410line3087>
> >
> > Why are these separate from the other Gauge methods, can they go in the header for now? We could move them all to slave/metrics.cpp in the future.
as with the master, these require Framework and Executor to be complete types. We can either move them to the header and change the order of class definitions in the header, or have them here. Here is less disruptive.
- Dominic
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19499/#review42520
-----------------------------------------------------------
On May 8, 2014, 11:47 a.m., Dominic Hamon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19499/
> -----------------------------------------------------------
>
> (Updated May 8, 2014, 11:47 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1133
> https://issues.apache.org/jira/browse/MESOS-1133
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary
>
>
> Diffs
> -----
>
> src/slave/slave.hpp ed20dca70cfbbd11dcdb35a074dc1777721f6ca5
> src/slave/slave.cpp 2a48266510a600349692780ea39cc98c3e12da30
> src/tests/slave_tests.cpp 09824550101577f8c9cf65c18e4980b74494a7d7
>
> Diff: https://reviews.apache.org/r/19499/diff/
>
>
> Testing
> -------
>
> make check. ran locally and compared stats.json endpoint to /metrics/snapshot endpoint.
>
>
> Thanks,
>
> Dominic Hamon
>
>
Re: Review Request 19499: Ported slave statistics over to new metrics
library.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19499/#review42520
-----------------------------------------------------------
Could we add a test like we did for the Master? (MasterTest.MetricsInStatsEndpoint).
How about we have a TODO to have slave/metrics.cpp where we can define all this stuff? :)
src/slave/slave.cpp
<https://reviews.apache.org/r/19499/#comment76309>
Why are these separate from the other Gauge methods, can they go in the header for now? We could move them all to slave/metrics.cpp in the future.
src/slave/slave.cpp
<https://reviews.apache.org/r/19499/#comment76310>
Can we remove the list logic here since we're not using it yet?
Ditto for the destructor.
- Ben Mahler
On May 2, 2014, 2:19 a.m., Dominic Hamon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19499/
> -----------------------------------------------------------
>
> (Updated May 2, 2014, 2:19 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1133
> https://issues.apache.org/jira/browse/MESOS-1133
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary
>
>
> Diffs
> -----
>
> src/slave/slave.hpp ed20dca70cfbbd11dcdb35a074dc1777721f6ca5
> src/slave/slave.cpp cb80609ba421b3b9a4664e600f0e53ecab8574c4
>
> Diff: https://reviews.apache.org/r/19499/diff/
>
>
> Testing
> -------
>
> make check. ran locally and compared stats.json endpoint to /metrics/snapshot endpoint.
>
>
> Thanks,
>
> Dominic Hamon
>
>