----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31893/#review76975 -----------------------------------------------------------
Thanks for the new patch. I really like the desgin of this test! A few minor comments below. core/src/test/scala/other/kafka/TestPurgatoryPerformance.scala <https://reviews.apache.org/r/31893/#comment127091> The Try syntax doesn't seem to be supported in scala 2.9. ./gradlew -PscalaVersion=2.9.1 testJar kafka/core/src/test/scala/other/kafka/TestPurgatoryPerformance.scala:29: Try is not a member of scala.util import scala.util.Try core/src/test/scala/other/kafka/TestPurgatoryPerformance.scala <https://reviews.apache.org/r/31893/#comment127122> Could we note that all times are in millisecs? core/src/test/scala/other/kafka/TestPurgatoryPerformance.scala <https://reviews.apache.org/r/31893/#comment127085> why => way core/src/test/scala/other/kafka/TestPurgatoryPerformance.scala <https://reviews.apache.org/r/31893/#comment127086> Could you add a comment on what 0.674490d stands for? core/src/test/scala/other/kafka/TestPurgatoryPerformance.scala <https://reviews.apache.org/r/31893/#comment127110> Now that we are completing the request in CompleteQueue explicitly, do we still need to override isCompleted()? core/src/test/scala/other/kafka/TestPurgatoryPerformance.scala <https://reviews.apache.org/r/31893/#comment127104> We probably can make this a ShutdownableThread. core/src/test/scala/other/kafka/TestPurgatoryPerformance.scala <https://reviews.apache.org/r/31893/#comment127102> Do we need the while loop? Could we just do a if test and go back to the outer loop? core/src/test/scala/other/kafka/TestPurgatoryPerformance.scala <https://reviews.apache.org/r/31893/#comment127105> Would it be more reliable to call forceComplete() here? Not sure if delayQueue always returns an item at the precise time. core/src/test/scala/other/kafka/TestPurgatoryPerformance.scala <https://reviews.apache.org/r/31893/#comment127107> Could we just put FakeOperation to the delayQueue directly instead of wrapping it under Scheduled? - Jun Rao On March 19, 2015, 11:31 p.m., Yasuhiro Matsuda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31893/ > ----------------------------------------------------------- > > (Updated March 19, 2015, 11:31 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2013 > https://issues.apache.org/jira/browse/KAFKA-2013 > > > Repository: kafka > > > Description > ------- > > purgatory micro benchmark > > > Diffs > ----- > > core/src/test/scala/other/kafka/TestPurgatoryPerformance.scala PRE-CREATION > > Diff: https://reviews.apache.org/r/31893/diff/ > > > Testing > ------- > > > Thanks, > > Yasuhiro Matsuda > >
