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 2014/10/10 03:51:36 UTC
Review Request 26535: Added batch sizes and timings to the Registrar
logging.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26535/
-----------------------------------------------------------
Review request for mesos and Vinod Kone.
Repository: mesos-git
Description
-------
We currently lack logging visibility into the batch sizes in the registrar.
While we have timing metrics, we also lack the ability to easily read timings for particular operations.
Diffs
-----
src/master/registrar.cpp ccc2fd23b1a1d56ab327cc73f6fef35d4bf6a552
Diff: https://reviews.apache.org/r/26535/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request 26535: Added batch sizes and timings to the
Registrar logging.
Posted by Ben Mahler <be...@gmail.com>.
> On Oct. 10, 2014, 5:02 p.m., Dominic Hamon wrote:
> > src/master/registrar.cpp, line 193
> > <https://reviews.apache.org/r/26535/diff/1/?file=717211#file717211line193>
> >
> > could you use the metrics timer instead (and still log the result) so that it is exposed on the metrics endpoint?
Sounds good, let me try to land benh's patch to make this possible: r/24537
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26535/#review56165
-----------------------------------------------------------
On Oct. 10, 2014, 1:51 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26535/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2014, 1:51 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We currently lack logging visibility into the batch sizes in the registrar.
>
> While we have timing metrics, we also lack the ability to easily read timings for particular operations.
>
>
> Diffs
> -----
>
> src/master/registrar.cpp ccc2fd23b1a1d56ab327cc73f6fef35d4bf6a552
>
> Diff: https://reviews.apache.org/r/26535/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 26535: Added batch sizes and timings to the
Registrar logging.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26535/#review56165
-----------------------------------------------------------
src/master/registrar.cpp
<https://reviews.apache.org/r/26535/#comment96485>
could you use the metrics timer instead (and still log the result) so that it is exposed on the metrics endpoint?
- Dominic Hamon
On Oct. 9, 2014, 6:51 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26535/
> -----------------------------------------------------------
>
> (Updated Oct. 9, 2014, 6:51 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We currently lack logging visibility into the batch sizes in the registrar.
>
> While we have timing metrics, we also lack the ability to easily read timings for particular operations.
>
>
> Diffs
> -----
>
> src/master/registrar.cpp ccc2fd23b1a1d56ab327cc73f6fef35d4bf6a552
>
> Diff: https://reviews.apache.org/r/26535/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 26535: Added batch sizes and timings to the
Registrar logging.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26535/#review56097
-----------------------------------------------------------
Patch looks great!
Reviews applied: [26535]
All tests passed.
- Mesos ReviewBot
On Oct. 10, 2014, 1:51 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26535/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2014, 1:51 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We currently lack logging visibility into the batch sizes in the registrar.
>
> While we have timing metrics, we also lack the ability to easily read timings for particular operations.
>
>
> Diffs
> -----
>
> src/master/registrar.cpp ccc2fd23b1a1d56ab327cc73f6fef35d4bf6a552
>
> Diff: https://reviews.apache.org/r/26535/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 26535: Added batch sizes and timings to the
Registrar logging.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26535/#review56200
-----------------------------------------------------------
Patch looks great!
Reviews applied: [26535]
All tests passed.
- Mesos ReviewBot
On Oct. 10, 2014, 6:25 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26535/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2014, 6:25 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We currently lack logging visibility into the batch sizes in the registrar.
>
> While we have timing metrics, we also lack the ability to easily read timings for particular operations.
>
>
> Diffs
> -----
>
> src/master/registrar.cpp ccc2fd23b1a1d56ab327cc73f6fef35d4bf6a552
>
> Diff: https://reviews.apache.org/r/26535/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 26535: Added batch sizes and timings to the
Registrar logging.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26535/#review56191
-----------------------------------------------------------
src/master/registrar.cpp
<https://reviews.apache.org/r/26535/#comment96516>
that sounds fine. I thought that start might fail if it was already running (and i didn't check).
- Dominic Hamon
On Oct. 10, 2014, 11:25 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26535/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2014, 11:25 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We currently lack logging visibility into the batch sizes in the registrar.
>
> While we have timing metrics, we also lack the ability to easily read timings for particular operations.
>
>
> Diffs
> -----
>
> src/master/registrar.cpp ccc2fd23b1a1d56ab327cc73f6fef35d4bf6a552
>
> Diff: https://reviews.apache.org/r/26535/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 26535: Added batch sizes and timings to the
Registrar logging.
Posted by Ben Mahler <be...@gmail.com>.
> On Oct. 10, 2014, 6:27 p.m., Dominic Hamon wrote:
> > src/master/registrar.cpp, line 341
> > <https://reviews.apache.org/r/26535/diff/2/?file=717559#file717559line341>
> >
> > we should probably stop the timer whether the recovery succeeded or failed. maybe we need a 'cancel' method on the timer?
I thought about this, is there any value in including the timing of failures? At least for the registrar, the point is moot since we'll fail the master.
More generally, since 'start()' will reset the timer, it seems safe to exclude certain timings (e.g. failures) by not calling stop() for these cases. Thoughts?
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26535/#review56188
-----------------------------------------------------------
On Oct. 10, 2014, 6:25 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26535/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2014, 6:25 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We currently lack logging visibility into the batch sizes in the registrar.
>
> While we have timing metrics, we also lack the ability to easily read timings for particular operations.
>
>
> Diffs
> -----
>
> src/master/registrar.cpp ccc2fd23b1a1d56ab327cc73f6fef35d4bf6a552
>
> Diff: https://reviews.apache.org/r/26535/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 26535: Added batch sizes and timings to the
Registrar logging.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26535/#review56188
-----------------------------------------------------------
src/master/registrar.cpp
<https://reviews.apache.org/r/26535/#comment96512>
we should probably stop the timer whether the recovery succeeded or failed. maybe we need a 'cancel' method on the timer?
- Dominic Hamon
On Oct. 10, 2014, 11:25 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26535/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2014, 11:25 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We currently lack logging visibility into the batch sizes in the registrar.
>
> While we have timing metrics, we also lack the ability to easily read timings for particular operations.
>
>
> Diffs
> -----
>
> src/master/registrar.cpp ccc2fd23b1a1d56ab327cc73f6fef35d4bf6a552
>
> Diff: https://reviews.apache.org/r/26535/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 26535: Added batch sizes and timings to the
Registrar logging.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26535/
-----------------------------------------------------------
(Updated Oct. 10, 2014, 6:25 p.m.)
Review request for mesos and Vinod Kone.
Changes
-------
Updated to use Timer::stop() result from https://reviews.apache.org/r/24537/ per Dominic's comment.
Hopefully this improves the accuracy of the timings as well (timer is now started before calls on 'state' occur).
Repository: mesos-git
Description
-------
We currently lack logging visibility into the batch sizes in the registrar.
While we have timing metrics, we also lack the ability to easily read timings for particular operations.
Diffs (updated)
-----
src/master/registrar.cpp ccc2fd23b1a1d56ab327cc73f6fef35d4bf6a552
Diff: https://reviews.apache.org/r/26535/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request 26535: Added batch sizes and timings to the
Registrar logging.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26535/#review56093
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On Oct. 10, 2014, 1:51 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26535/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2014, 1:51 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We currently lack logging visibility into the batch sizes in the registrar.
>
> While we have timing metrics, we also lack the ability to easily read timings for particular operations.
>
>
> Diffs
> -----
>
> src/master/registrar.cpp ccc2fd23b1a1d56ab327cc73f6fef35d4bf6a552
>
> Diff: https://reviews.apache.org/r/26535/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>