> On May 19, 2015, 5:41 p.m., Dong Lin wrote: > > clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.java, line > > 64 > > <https://reviews.apache.org/r/34418/diff/1/?file=963982#file963982line64> > > > > Hey Jay, > > > > My approach differs with your only in this line: > > > > long ellapsedPriorWindowsMs = config.timeWindowMs() * > > (max(stat.samples.size() -1, 1)); > > > > I guess my approach is probably a bit more fine grained (and closer to > > existing code) than yours > > in the beginning config.timeWindowMs()*(config.samples()-1) ms. After > > that the behavior is exactly the same. > > > > I think this difference doesn't matter in production environment. It > > allows minutes-long system test a bit more accurate though. I am good with > > both approaches.
Hey Jay, during my quota system test, I find that it is actually preferable to use to full window size to calculate the rate at the beginning, so that when client just starts and requests to fetch a large chunk of data, this initial large fetch request wouldn't be delayed for a long time. Therefore your approach is better. - Dong ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34418/#review84349 ----------------------------------------------------------- On May 19, 2015, 5:12 p.m., Jay Kreps wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34418/ > ----------------------------------------------------------- > > (Updated May 19, 2015, 5:12 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2191 > https://issues.apache.org/jira/browse/KAFKA-2191 > > > Repository: kafka > > > Description > ------- > > Fix 0 ellapsed time rate bug. > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.java > 98429da34418f7f1deba1b5e44e2e6025212edb3 > clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java > 544e120594de78c43581a980b1e4087b4fb98ccb > > Diff: https://reviews.apache.org/r/34418/diff/ > > > Testing > ------- > > > Thanks, > > Jay Kreps > >
