-----------------------------------------------------------
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/KafkaProducer.java
<https://reviews.apache.org/r/29467/#comment121709>

    It's probably worth adding an
      if(timeout > 0)
    on this.



clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
<https://reviews.apache.org/r/29467/#comment121708>

    This seems to call initiateClose() twice, once in initiateClose and then 
again from forceClose. This seems like it depends on all the things getting 
closed being idempotent to repeated calls (e.g. record accumulator etc). Would 
it make more sense to have forceClose() just set the force flag?


Two minor changes I noted, but otherwise looks good to me. Needs some unit 
tests, as you mentioned.

- Jay Kreps


On March 2, 2015, 6:41 p.m., Parth Brahmbhatt wrote:
> 
> -----------------------------------------------------------
> 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
>     https://issues.apache.org/jira/browse/KAFKA-1660
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Merge remote-tracking branch 'origin/trunk' into KAFKA-1660
> 
> Conflicts:
>       
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java
> 
> Merge remote-tracking branch 'origin/trunk' into KAFKA-1660
> 
> 
> Changing log levels as suggested.
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 
> 7397e565fd865214529ffccadd4222d835ac8110 
>   clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java 
> 6913090af03a455452b0b5c3df78f266126b3854 
>   clients/src/main/java/org/apache/kafka/clients/producer/Producer.java 
> 5b3e75ed83a940bc922f9eca10d4008d67aa37c9 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 
> ed9c63a6679e3aaf83d19fde19268553a4c107c2 
> 
> Diff: https://reviews.apache.org/r/29467/diff/
> 
> 
> Testing
> -------
> 
> existing unit tests passed.
> 
> 
> Thanks,
> 
> Parth Brahmbhatt
> 
>

Reply via email to