> On March 16, 2015, 5:17 p.m., Jun Rao wrote: > > core/src/test/scala/other/kafka/TestPurgatoryPerformance.scala, line 193 > > <https://reviews.apache.org/r/31893/diff/1/?file=890190#file890190line193> > > > > Is there a particular reason that we need to overwrite isCompleted()? > > Typically, only tryComplete() and onComplete() need to be overwritten in a > > subclass of DelayedOperation. > > > > Actually, I am not sure how we complete the requests before the timeout > > is reached since there is no explict call for tryComplete()?
isCompleted checks if the current time has passed the schedule completion time rather than if forceComplete has been called. It makes isCompleted always accurate. Purgatory checks watcher lists every so often and calls isCompleted. Calling forceComplete from isCompeleted ensures that a completed request is removed from the timing wheels in the new implementation. In terms of timing, this is not very accurate because completed requests may stay longer then they should be. This doesn't affect the old implementaion at all, but it may impose some overheads on the new implementaion. Still, the new one outperforms the old one. It is ideal if we can call call forceComplete on scheduled completion time. It requires another timer (DelayQueue or Timer) for that. I think it is too much overhead to measure purgatory performace. And also it is hard to guarantee such a timer works accurately in this test setting. - Yasuhiro ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31893/#review76566 ----------------------------------------------------------- On March 16, 2015, 8:23 p.m., Yasuhiro Matsuda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31893/ > ----------------------------------------------------------- > > (Updated March 16, 2015, 8:23 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 > >