----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35437/#review87876 -----------------------------------------------------------
This patch only addresses the old consumer. Can you check if the rate calculation for the new consumer is accurate? The both calculate end time in different ways. New consumer uses "endMs = System.currentTimeMs" but ignores the fixed timeout of 1sec (which shouldn't affect the rate much). You can perhaps return the lastConsumed from the consume() and use that instead of endMs. core/src/main/scala/kafka/tools/ConsumerPerformance.scala <https://reviews.apache.org/r/35437/#comment140290> i think the general convention in kafka is if (consumerTimeout.get()) core/src/main/scala/kafka/tools/ConsumerPerformance.scala <https://reviews.apache.org/r/35437/#comment140291> can probably do this inline case _: ConsumerEx => consumerTimeout.set(true) - Aditya Auradkar On June 14, 2015, 11:27 a.m., Manikumar Reddy O wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35437/ > ----------------------------------------------------------- > > (Updated June 14, 2015, 11:27 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2202 > https://issues.apache.org/jira/browse/KAFKA-2202 > > > Repository: kafka > > > Description > ------- > > while computing stats, consumerTimeoutMs is considered only during > ConsumerTimeout exception senarios; This should solve KAFKA-1828 also > > > Diffs > ----- > > core/src/main/scala/kafka/tools/ConsumerPerformance.scala > 903318d15893af08104a97499798c9ad0ba98013 > > Diff: https://reviews.apache.org/r/35437/diff/ > > > Testing > ------- > > > Thanks, > > Manikumar Reddy O > >
