Re: Review Request 29467: Patch for KAFKA-1660

2015-03-08 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29467/#review75656 --- I just had a patch submitted for KAFKA-1660. But I do not have permi

Re: Review Request 29467: Patch for KAFKA-1660

2015-03-03 Thread Parth Brahmbhatt
> On March 3, 2015, 5:37 a.m., Jay Kreps wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java, > > line 533 > > > > > > Now there is a bit of duplicate code between the two close metho

Re: Review Request 29467: Patch for KAFKA-1660

2015-03-03 Thread Parth Brahmbhatt
> On March 3, 2015, 4:10 a.m., Jay Kreps wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java, > > line 560 > > > > > > This seems to call initiateClose() twice, once in initiateClose

Re: Review Request 29467: Patch for KAFKA-1660

2015-03-02 Thread Jiangjie Qin
> On March 2, 2015, 11:04 p.m., Jiangjie Qin wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java, > > line 219 > > > > > > We probably need to release the caller threads that are w

Re: Review Request 29467: Patch for KAFKA-1660

2015-03-02 Thread Jay Kreps
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29467/#review74897 --- clients/src/main/java/org/apache/kafka/clients/producer/KafkaProduc

Re: Review Request 29467: Patch for KAFKA-1660

2015-03-02 Thread Jay Kreps
> On March 2, 2015, 11:04 p.m., Jiangjie Qin wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java, > > line 219 > > > > > > We probably need to release the caller threads that are w

Re: Review Request 29467: Patch for KAFKA-1660

2015-03-02 Thread Jay Kreps
On March 3, 2015, 4:10 a.m., Parth Brahmbhatt wrote: > > Two minor changes I noted, but otherwise looks good to me. Needs some unit > > tests, as you mentioned. Actually one probably I didn't think of is that forceClose() leaves the in-flight requests forever incomplete. A better approach woul

Re: Review Request 29467: Patch for KAFKA-1660

2015-03-02 Thread Jay Kreps
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29467/#review74884 --- clients/src/main/java/org/apache/kafka/clients/producer/KafkaProduc

Re: Review Request 29467: Patch for KAFKA-1660

2015-03-02 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29467/#review74843 --- Actually I spoke too fast... As the flush() has been checked in, we

Re: Review Request 29467: Patch for KAFKA-1660

2015-03-02 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29467/#review74835 --- Could we add some unit tests for this new API as I mentioned in my p

Re: Review Request 29467: Patch for KAFKA-1660

2015-03-02 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29467/#review74833 --- Ship it! LGTM. - Jiangjie Qin On March 2, 2015, 6:41 p.m., Parth

Re: Review Request 29467: Patch for KAFKA-1660

2015-03-02 Thread Parth Brahmbhatt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29467/#review74777 --- clients/src/main/java/org/apache/kafka/clients/producer/KafkaProduc

Re: Review Request 29467: Patch for KAFKA-1660

2015-03-02 Thread Parth Brahmbhatt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29467/ --- (Updated March 2, 2015, 6:41 p.m.) Review request for kafka. Bugs: KAFKA-1660

Re: Review Request 29467: Patch for KAFKA-1660

2015-02-25 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29467/#review74149 --- Could we add some unit tests for this new API, both called by anothe

Re: Review Request 29467: Patch for KAFKA-1660

2015-02-17 Thread Parth Brahmbhatt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29467/ --- (Updated Feb. 18, 2015, 12:41 a.m.) Review request for kafka. Bugs: KAFKA-166

Re: Review Request 29467: Patch for KAFKA-1660

2015-02-17 Thread Parth Brahmbhatt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29467/ --- (Updated Feb. 18, 2015, 12:36 a.m.) Review request for kafka. Bugs: KAFKA-166

Re: Review Request 29467: Patch for KAFKA-1660

2015-02-08 Thread Jay Kreps
> On Feb. 9, 2015, 1:27 a.m., Jay Kreps wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java, > > line 371 > > > > > > This approach will actually leak the sender thread if there are s

Re: Review Request 29467: Patch for KAFKA-1660

2015-02-08 Thread Jay Kreps
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29467/#review71595 --- I would vote for the name close(long timeout, TimeUnit unit) I th

Re: Review Request 29467: Patch for KAFKA-1660

2014-12-29 Thread Parth Brahmbhatt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29467/ --- (Updated Dec. 29, 2014, 10:52 p.m.) Review request for kafka. Changes ---

Review Request 29467: Patch for KAFKA-1660

2014-12-29 Thread Parth Brahmbhatt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29467/ --- Review request for kafka. Bugs: KAFKA-1660 https://issues.apache.org/jira/b