You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2017/08/03 22:10:42 UTC

Re: Review Request 60957: Added a time window for Timer metrics.


> On July 19, 2017, 8:30 p.m., Benjamin Mahler wrote:
> > src/master/registrar.cpp
> > Line 156 (original), 156 (patched)
> > <https://reviews.apache.org/r/60957/diff/1/?file=1779278#file1779278line156>
> >
> >     There is only a single fetch, so this intentionally omits the time window since getting a distribution for a single data point isn't useful.
> 
> James Peach wrote:
>     Hmm, maybe this is better as a `Gauge` then? If we add the time window, then the values would eventually expire and the stats would go to zero while the initial value is still non-zero, which would be weird.
> 
> James Peach wrote:
>     I've left this change in (for now). Although the fetch only happens once, it will generate statistics consistent with other `Timer` metrics. The sample won't get expired because the history buffer won't fill and the calculated statistics will show operators that this only happens once.

Hm.. using an explicit gauge for single measurement sounds reasonable to me. Can you make that change so that we don't dump the statistics for the state fetch?


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60957/#review180973
-----------------------------------------------------------


On July 19, 2017, 9:04 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60957/
> -----------------------------------------------------------
> 
> (Updated July 19, 2017, 9:04 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6918
>     https://issues.apache.org/jira/browse/MESOS-6918
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a time window argument for the Timer metrics that were missing it.
> 
> 
> Diffs
> -----
> 
>   src/master/registrar.cpp 242cd0a4f1fad327dcc1a59260f46987cfeba59c 
>   src/state/log.cpp b71383906cf28fe0769cbb620387a0e0134f01f9 
> 
> 
> Diff: https://reviews.apache.org/r/60957/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> James Peach
> 
>