Re: Review Request 26663: Patch for KAFKA-979

2014-10-15 Thread Ewen Cheslack-Postava
On Oct. 15, 2014, 4:19 a.m., Joel Koshy wrote: core/src/main/scala/kafka/log/Log.scala, line 515 https://reviews.apache.org/r/26663/diff/2/?file=721126#file721126line515 Thinking about this a bit more: do you think it would be safer to interpret jitter as an additive value to

Re: Review Request 26663: Patch for KAFKA-979

2014-10-15 Thread Joel Koshy
On Oct. 15, 2014, 4:19 a.m., Joel Koshy wrote: core/src/main/scala/kafka/log/Log.scala, line 515 https://reviews.apache.org/r/26663/diff/2/?file=721126#file721126line515 Thinking about this a bit more: do you think it would be safer to interpret jitter as an additive value to

Re: Review Request 26663: Patch for KAFKA-979

2014-10-15 Thread Ewen Cheslack-Postava
On Oct. 15, 2014, 4:19 a.m., Joel Koshy wrote: core/src/main/scala/kafka/log/Log.scala, line 515 https://reviews.apache.org/r/26663/diff/2/?file=721126#file721126line515 Thinking about this a bit more: do you think it would be safer to interpret jitter as an additive value to

Re: Review Request 26663: Patch for KAFKA-979

2014-10-15 Thread Neha Narkhede
On Oct. 15, 2014, 4:19 a.m., Joel Koshy wrote: core/src/main/scala/kafka/log/Log.scala, line 515 https://reviews.apache.org/r/26663/diff/2/?file=721126#file721126line515 Thinking about this a bit more: do you think it would be safer to interpret jitter as an additive value to

Re: Review Request 26663: Patch for KAFKA-979

2014-10-14 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26663/#review56494 --- core/src/main/scala/kafka/log/Log.scala

Re: Review Request 26663: Patch for KAFKA-979

2014-10-14 Thread Neha Narkhede
On Oct. 14, 2014, 6:27 a.m., Joel Koshy wrote: core/src/main/scala/kafka/log/Log.scala, line 515 https://reviews.apache.org/r/26663/diff/1/?file=719793#file719793line515 Although this should work I'm wondering if it would be better to recompute a random jitter for each new rolled

Re: Review Request 26663: Patch for KAFKA-979

2014-10-14 Thread Joel Koshy
On Oct. 14, 2014, 6:27 a.m., Joel Koshy wrote: core/src/main/scala/kafka/log/Log.scala, line 515 https://reviews.apache.org/r/26663/diff/1/?file=719793#file719793line515 Although this should work I'm wondering if it would be better to recompute a random jitter for each new rolled

Re: Review Request 26663: Patch for KAFKA-979

2014-10-14 Thread Neha Narkhede
On Oct. 14, 2014, 6:27 a.m., Joel Koshy wrote: core/src/main/scala/kafka/log/Log.scala, line 515 https://reviews.apache.org/r/26663/diff/1/?file=719793#file719793line515 Although this should work I'm wondering if it would be better to recompute a random jitter for each new rolled

Re: Review Request 26663: Patch for KAFKA-979

2014-10-14 Thread Neha Narkhede
On Oct. 14, 2014, 6:27 a.m., Joel Koshy wrote: core/src/main/scala/kafka/log/Log.scala, line 515 https://reviews.apache.org/r/26663/diff/1/?file=719793#file719793line515 Although this should work I'm wondering if it would be better to recompute a random jitter for each new rolled

Re: Review Request 26663: Patch for KAFKA-979

2014-10-14 Thread Ewen Cheslack-Postava
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26663/ --- (Updated Oct. 14, 2014, 10:33 p.m.) Review request for kafka. Bugs:

Re: Review Request 26663: Patch for KAFKA-979

2014-10-14 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26663/#review56652 --- Looks good overall - I have one comment on interpreting the jitter

Review Request 26663: Patch for KAFKA-979

2014-10-13 Thread Ewen Cheslack-Postava
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26663/ --- Review request for kafka. Bugs: KAFKA-979

Re: Review Request 26663: Patch for KAFKA-979

2014-10-13 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26663/#review56490 --- Looks good. Just one minor comment. Could you get rid of this build