> 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
> 
>

Reply via email to