Re: Review Request 36333: Patch for KAFKA-2123

2015-07-15 Thread Jason Gustafson
> On July 15, 2015, 8:10 p.m., Onur Karaman wrote: > > clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java, > > lines 280-282 > > > > > > I think this is a bug. schedule takes a timest

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-15 Thread Onur Karaman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36333/#review91810 --- clients/src/main/java/org/apache/kafka/clients/consumer/internals/C

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-15 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36333/#review91808 --- Ship it! Ship It! - Guozhang Wang On July 15, 2015, 1:21 a.m., J

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-14 Thread Jason Gustafson
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36333/ --- (Updated July 15, 2015, 1:21 a.m.) Review request for kafka. Bugs: KAFKA-2123

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-14 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36333/#review91651 --- LGTM overall, just some minor comments below. clients/src/main/jav

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-14 Thread Jason Gustafson
> On July 14, 2015, 6 p.m., Ewen Cheslack-Postava wrote: > > clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java, > > line 274 > > > > > > Seems like we're not handling this anymore? sen

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-14 Thread Jason Gustafson
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36333/ --- (Updated July 14, 2015, 8:21 p.m.) Review request for kafka. Bugs: KAFKA-2123

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-14 Thread Ewen Cheslack-Postava
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36333/#review91646 --- clients/src/main/java/org/apache/kafka/clients/consumer/internals/F

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-13 Thread Jason Gustafson
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36333/ --- (Updated July 14, 2015, 1:45 a.m.) Review request for kafka. Bugs: KAFKA-2123

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-13 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36333/#review91565 --- clients/src/main/java/org/apache/kafka/clients/consumer/internals/C

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-13 Thread Jason Gustafson
> On July 14, 2015, 12:32 a.m., Ewen Cheslack-Postava wrote: > > clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java, > > line 968 > > > > > > Hmm, this seems like very different behavior fr

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-13 Thread Jason Gustafson
> On July 14, 2015, 12:04 a.m., Guozhang Wang wrote: > > clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java, > > lines 117-131 > > > > > > With this change, we are now always sendin

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-13 Thread Ewen Cheslack-Postava
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36333/#review91564 --- clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsum

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-11 Thread Jason Gustafson
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36333/ --- (Updated July 12, 2015, 12:34 a.m.) Review request for kafka. Bugs: KAFKA-212

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-11 Thread Ewen Cheslack-Postava
> On July 9, 2015, 1:42 a.m., Guozhang Wang wrote: > > clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java, > > line 775 > > > > > > I think KAFKA-1894 is already fixed in this patch + KAFKA-2

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-10 Thread Jason Gustafson
> On July 9, 2015, 1:42 a.m., Guozhang Wang wrote: > > clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkClient.java, > > lines 88-93 > > > > > > This is not introduced in the patch, bu

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-10 Thread Gwen Shapira
> On July 9, 2015, 1:42 a.m., Guozhang Wang wrote: > > clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java, > > line 775 > > > > > > I think KAFKA-1894 is already fixed in this patch + KAFKA-2

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-10 Thread Ewen Cheslack-Postava
> On July 9, 2015, 1:42 a.m., Guozhang Wang wrote: > > clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java, > > line 775 > > > > > > I think KAFKA-1894 is already fixed in this patch + KAFKA-2

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-09 Thread Guozhang Wang
> On July 9, 2015, 1:42 a.m., Guozhang Wang wrote: > > clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java, > > line 775 > > > > > > I think KAFKA-1894 is already fixed in this patch + KAFKA-2

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-09 Thread Guozhang Wang
> On July 9, 2015, 3:53 a.m., Ewen Cheslack-Postava wrote: > > clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkClient.java, > > line 91 > > > > > > Is the retryBackoffMs here because

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-09 Thread Jason Gustafson
> On July 9, 2015, 3:53 a.m., Ewen Cheslack-Postava wrote: > > clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkClient.java, > > line 121 > > > > > > Is this the behavior we want? Bot

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-09 Thread Jason Gustafson
> On July 9, 2015, 1:42 a.m., Guozhang Wang wrote: > > Browsed through the patch, overall looks very promising. > > > > I am not very clear on a few detailed changes though: > > > > 1. The request future adapter / handler modifications. > > 2. Retry backoff implementation seems not correct. > >

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-08 Thread Ewen Cheslack-Postava
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36333/#review91016 --- clients/src/main/java/org/apache/kafka/clients/consumer/internals/C

Re: Review Request 36333: Patch for KAFKA-2123

2015-07-08 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36333/#review91012 --- Browsed through the patch, overall looks very promising. I am not v

Review Request 36333: Patch for KAFKA-2123

2015-07-08 Thread Jason Gustafson
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36333/ --- Review request for kafka. Bugs: KAFKA-2123 https://issues.apache.org/jira/b