> On Sept. 2, 2014, 4:51 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/server/RequestPurgatory.scala, line 251
> > <https://reviews.apache.org/r/25155/diff/5/?file=673594#file673594line251>
> >
> >     It's probably better to expose an api that returns the size of the 
> > queue, instead of exposing the queue itself.

I made this change since the queue size needs to be accessed outside 
RequestPurgatory in the unit tests, and I thought since the reaper is a private 
class of the purgatory already, it is OK to expose its queue to the purgatory 
anyways. Let me know if you have a strong preference.


> On Sept. 2, 2014, 4:51 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/server/RequestPurgatory.scala, lines 268-270
> > <https://reviews.apache.org/r/25155/diff/5/?file=673594#file673594line268>
> >
> >     I thought that we only want to call purgeSatisfied if delayed.size >= 
> > purgeIntveral and similarly, only call watcherForKey...purgeSatisfied() if 
> > RequestPurgatory.this.size() >= purgeInterval?

Previously the logic checks on sum of watch list sizes and queue size, but it 
never check them separately. I thought since watch list sizes >= queue size in 
theory, and it is >> queue size in practice, we can just check on watch list 
sizes, and always purge both when the condition satisfies for simplicity. Let 
me know if you have a strong preference.


- Guozhang


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25155/#review52036
-----------------------------------------------------------


On Sept. 1, 2014, 9:41 p.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25155/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2014, 9:41 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1616
>     https://issues.apache.org/jira/browse/KAFKA-1616
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Purgatory size to be the sum of watched list sizes; delayed request to be the 
> expiry queue length; remove atomic integers for metrics; add a unit test for 
> watched list sizes and enqueued requests
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/server/RequestPurgatory.scala 
> ce06d2c381348deef8559374869fcaed923da1d1 
>   core/src/test/scala/unit/kafka/server/RequestPurgatoryTest.scala 
> 168712de241125982d556c188c76514fceb93779 
> 
> Diff: https://reviews.apache.org/r/25155/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>

Reply via email to