Re: Review Request 26373: Patch for KAFKA-1647

2014-10-30 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/#review59274 --- Ship it! Ship It! - Joel Koshy On Oct. 30, 2014, 10:50 p.m., Jia

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-30 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/ --- (Updated Oct. 30, 2014, 10:50 p.m.) Review request for kafka. Bugs: KAFKA-164

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-30 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/ --- (Updated Oct. 30, 2014, 10:38 p.m.) Review request for kafka. Bugs: KAFKA-164

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-30 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/ --- (Updated Oct. 30, 2014, 10:10 p.m.) Review request for kafka. Bugs: KAFKA-164

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-30 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/ --- (Updated Oct. 30, 2014, 10:07 p.m.) Review request for kafka. Bugs: KAFKA-164

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-28 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/#review58846 --- Ship it! core/src/main/scala/kafka/server/ReplicaManager.scala

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-27 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/#review58748 --- Ship it! Looks good to me. Can you make these final edits and uploa

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-27 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/ --- (Updated Oct. 28, 2014, 12:20 a.m.) Review request for kafka. Bugs: KAFKA-164

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-27 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/ --- (Updated Oct. 28, 2014, 12:19 a.m.) Review request for kafka. Bugs: KAFKA-164

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-23 Thread Jiangjie Qin
> On Oct. 23, 2014, 12:42 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/ReplicaManager.scala, lines 531-538 > > > > > > I am not sure that I understand how Option.flatten works. Would it be > > clearly if

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-22 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/#review57947 --- core/src/main/scala/kafka/server/ReplicaManager.scala

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-22 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/#review57836 --- core/src/main/scala/kafka/server/ReplicaManager.scala

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-21 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/ --- (Updated Oct. 22, 2014, 6:08 a.m.) Review request for kafka. Bugs: KAFKA-1647

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-21 Thread Jun Rao
> On Oct. 21, 2014, 12:18 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/ReplicaManager.scala, lines 481-506 > > > > > > This doesn't quite fix the original problem though. The original > > problem is that

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-21 Thread Neha Narkhede
> On Oct. 21, 2014, 12:18 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/ReplicaManager.scala, lines 481-506 > > > > > > This doesn't quite fix the original problem though. The original > > problem is that

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-20 Thread Jiangjie Qin
> On Oct. 21, 2014, 12:18 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/ReplicaManager.scala, lines 481-506 > > > > > > This doesn't quite fix the original problem though. The original > > problem is that

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-20 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/#review57490 --- Thanks for the patch. Some comments below. core/src/main/scala/kaf

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-18 Thread Jiangjie Qin
> On Oct. 16, 2014, 8:11 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/server/ReplicaManager.scala, line 504 > > > > > > Now for partitions that have a leader, we are not adding a follower. Hi Neha, the ver

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-18 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/ --- (Updated Oct. 18, 2014, 7:26 a.m.) Review request for kafka. Bugs: KAFKA-1647

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-16 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/#review56989 --- This is a fairly tricky patch and I'm not 100% sure that we haven't

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-15 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/#review56736 --- Ship it! Ship It! - Guozhang Wang On Oct. 13, 2014, 11:38 p.m.,

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-13 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/ --- (Updated Oct. 13, 2014, 11:38 p.m.) Review request for kafka. Bugs: KAFKA-164

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-13 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/#review56434 --- core/src/main/scala/kafka/server/ReplicaManager.scala

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-09 Thread Guozhang Wang
> On Oct. 7, 2014, 12:15 a.m., Guozhang Wang wrote: > > Since now the first iteration of "if" statements is only used for logging, > > could we just merge it into the second check? > > Jiangjie Qin wrote: > Guozhang, thanks for the review. I actually thought about it before. I > agree that

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-08 Thread Guozhang Wang
> On Oct. 7, 2014, 12:15 a.m., Guozhang Wang wrote: > > Since now the first iteration of "if" statements is only used for logging, > > could we just merge it into the second check? > > Jiangjie Qin wrote: > Guozhang, thanks for the review. I actually thought about it before. I > agree that

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-08 Thread Jiangjie Qin
> On Oct. 7, 2014, 12:15 a.m., Guozhang Wang wrote: > > Since now the first iteration of "if" statements is only used for logging, > > could we just merge it into the second check? Guozhang, thanks for the review. I actually thought about it before. I agree that the code looks simpler if we ju

Re: Review Request 26373: Patch for KAFKA-1647

2014-10-06 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/#review55615 --- Since now the first iteration of "if" statements is only used for lo

Review Request 26373: Patch for KAFKA-1647

2014-10-06 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/ --- Review request for kafka. Bugs: KAFKA-1647 https://issues.apache.org/jira/b