Re: Review Request 31568: Patch for KAFKA-1989

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31568/ --- (Updated April 8, 2015, 9:30 p.m.) Review request for kafka. Bugs: KAFKA-1989

Re: Review Request 31568: Patch for KAFKA-1989

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31568/ --- (Updated April 8, 2015, 8:28 p.m.) Review request for kafka. Bugs: KAFKA-1989

Re: Review Request 31568: Patch for KAFKA-1989

> On April 8, 2015, 6:44 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/utils/timer/TimingWheel.scala, line 96 > > > > > > Does this need to be volatile since all accesses to it are synchronized? > > Yasuhiro Matsu

Re: Review Request 31568: Patch for KAFKA-1989

> On April 8, 2015, 7:15 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/server/DelayedOperation.scala, lines 123-126 > > > > > > We do not need to address it in this patch but may think about it > > moving f

Re: Review Request 31568: Patch for KAFKA-1989

> On April 8, 2015, 6:44 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/utils/timer/TimingWheel.scala, line 59 > > > > > > Each bucket here needs to cover a window of size 3, right? Sorry, I messed up the example

Re: Review Request 31568: Patch for KAFKA-1989

> On April 6, 2015, 10:35 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/DelayedOperation.scala, lines 104-105 > > > > > > We probably should call forceComplete() first and only if it returns > > true, run

Re: Review Request 31568: Patch for KAFKA-1989

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31568/#review79408 --- Ship it! Looks great to me. Just a minor comment below. core/src/

Re: Review Request 31568: Patch for KAFKA-1989

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31568/#review79269 --- Thanks for the latest patch. Looks good. Just a few more minor comme

Re: Review Request 31568: Patch for KAFKA-1989

> On April 7, 2015, 10:17 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/server/DelayedOperation.scala, lines 178-188 > > > > > > What is the usage of watchCreated here? It seems only gets intialized > > as

Re: Review Request 31568: Patch for KAFKA-1989

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31568/#review79263 --- - Guozhang Wang On April 7, 2015, 9:59 p.m., Yasuhiro Matsuda wrot

Re: Review Request 31568: Patch for KAFKA-1989

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31568/#review79257 --- core/src/main/scala/kafka/server/DelayedOperation.scala

Re: Review Request 31568: Patch for KAFKA-1989

> On April 6, 2015, 10:35 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/DelayedOperation.scala, lines 104-105 > > > > > > We probably should call forceComplete() first and only if it returns > > true, run

Re: Review Request 31568: Patch for KAFKA-1989

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31568/ --- (Updated April 7, 2015, 9:59 p.m.) Review request for kafka. Bugs: KAFKA-1989

Re: Review Request 31568: Patch for KAFKA-1989

> On April 6, 2015, 10:35 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/DelayedOperation.scala, lines 104-105 > > > > > > We probably should call forceComplete() first and only if it returns > > true, run

Re: Review Request 31568: Patch for KAFKA-1989

> On April 1, 2015, 6:46 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/utils/timer/Timer.scala, lines 44-45 > > > > > > Could we use inLock() where applicable? > > Yasuhiro Matsuda wrote: > I don't like to hav

Re: Review Request 31568: Patch for KAFKA-1989

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31568/#review78668 --- Thanks for the new patch. Now that I understood the logic better, I

Re: Review Request 31568: Patch for KAFKA-1989

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31568/ --- (Updated April 1, 2015, 8:50 p.m.) Review request for kafka. Bugs: KAFKA-1989

Re: Review Request 31568: Patch for KAFKA-1989

> On April 1, 2015, 6:46 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/DelayedOperation.scala, line 126 > > > > > > timeoutTimer is an implementation detail in ExpiredOperationReaper. > > Could we hide it

Re: Review Request 31568: Patch for KAFKA-1989

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31568/#review78444 --- Thanks for the patch! A few comments below. core/src/main/scala/ka

Re: Review Request 31568: Patch for KAFKA-1989

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31568/ --- (Updated March 20, 2015, 3:45 p.m.) Review request for kafka. Bugs: KAFKA-198

Re: Review Request 31568: Patch for KAFKA-1989

> On March 17, 2015, 8:56 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/utils/timer/TimerTaskList.scala, line 72 > > > > > > It seems the task entry of the task will only be set once throughout > > its life

Re: Review Request 31568: Patch for KAFKA-1989

> On March 17, 2015, 8:56 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/server/DelayedOperation.scala, lines 187-192 > > > > > > TBD > > Guozhang Wang wrote: > Replace the TBD here: we can let Timer.add

Re: Review Request 31568: Patch for KAFKA-1989

> On March 18, 2015, 4:14 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/server/DelayedOperation.scala, line 373 > > > > > > Try purging on each watcher list in each doWork will likely increase > > the CPU a

Re: Review Request 31568: Patch for KAFKA-1989

> On March 17, 2015, 8:56 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/server/DelayedOperation.scala, line 116 > > > > > > We need to make tickMs and wheelSize configurable. > > Yasuhiro Matsuda wrote: >

Re: Review Request 31568: Patch for KAFKA-1989

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31568/#review76899 --- core/src/main/scala/kafka/server/DelayedOperation.scala

Re: Review Request 31568: Patch for KAFKA-1989

> On March 17, 2015, 8:56 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/server/DelayedOperation.scala, line 116 > > > > > > We need to make tickMs and wheelSize configurable. > > Yasuhiro Matsuda wrote: >

Re: Review Request 31568: Patch for KAFKA-1989

> On March 17, 2015, 8:56 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/utils/timer/Timer.scala, line 68 > > > > > > I think bucket.flush(reinsurt) will always fail on all the items since > > their expiratio

Re: Review Request 31568: Patch for KAFKA-1989

> On March 17, 2015, 8:56 p.m., Guozhang Wang wrote: > > We can probably remove DelayedItem if it is not referenced by anyone any > > more. I am using DelayedItem for KAFKA-1461. - Sriharsha --- This is an automatically generated e-mai

Re: Review Request 31568: Patch for KAFKA-1989

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31568/#review76459 --- We can probably remove DelayedItem if it is not referenced by anyone

Review Request 31568: Patch for KAFKA-1989

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31568/ --- Review request for kafka. Bugs: KAFKA-1989 https://issues.apache.org/jira/b