> On Feb. 9, 2015, 1:27 a.m., Jay Kreps wrote:
> > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java, 
> > line 371
> > <https://reviews.apache.org/r/29467/diff/1/?file=802888#file802888line371>
> >
> >     This approach will actually leak the sender thread if there are still 
> > unsent requests. I think this is not what people want. I think what they 
> > want is for the sender thread to attempt to send their messages for N ms 
> > and then shutdown if it still hasn't succeeded. Leaking the thread seems 
> > like a bug.

Oh I think I understand the interpretation, the idea is that this is meant to 
attempt to close but then give up if the close doesn't complete in time. The 
problem is that this does actually close the producer but doesn't necessarily 
stop the thread and doesn't return any indication of what happened.


- Jay


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29467/#review71595
-----------------------------------------------------------


On Dec. 29, 2014, 10:52 p.m., Parth Brahmbhatt wrote:
> 
> -----------------------------------------------------------
> 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.
> 
> 
> Bugs: KAFKA-1660
>     https://issues.apache.org/jira/browse/KAFKA-1660
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1660: Adding tryClose(timeoutMillis) to producer.
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 
> f61efb35db7e0de590556e6a94a7b5cb850cdae9 
>   clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java 
> 34624c3b7a1f28735ab6c63cc9e18a410e87e63c 
>   clients/src/main/java/org/apache/kafka/clients/producer/Producer.java 
> 5baa6062bd9ba8a7d38058856ed2d831fae491f0 
> 
> Diff: https://reviews.apache.org/r/29467/diff/
> 
> 
> Testing
> -------
> 
> existing unit tests passed.
> 
> 
> Thanks,
> 
> Parth Brahmbhatt
> 
>

Reply via email to