----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31850/#review75731 -----------------------------------------------------------
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java <https://reviews.apache.org/r/31850/#comment122960> This is not related to this ticket, but I think we can just throw e here as well. clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java <https://reviews.apache.org/r/31850/#comment122959> This function no longer throws InterruptException, but only KafkaException. Better change line 539 to just throw e. clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java <https://reviews.apache.org/r/31850/#comment122966> Could metrics.close() be called simultaneously? clients/src/main/java/org/apache/kafka/clients/producer/Producer.java <https://reviews.apache.org/r/31850/#comment122961> Space after "," clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java <https://reviews.apache.org/r/31850/#comment122964> How about rename to abortIncompleteBatches and return batch.done(-1, new InterruptException("")) clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java <https://reviews.apache.org/r/31850/#comment122965> I think you only need to call this.wakeup() here? clients/src/test/java/org/apache/kafka/clients/producer/internals/RecordAccumulatorTest.java <https://reviews.apache.org/r/31850/#comment122967> Rename to testAbortIncompleteBatches? clients/src/test/java/org/apache/kafka/clients/producer/internals/RecordAccumulatorTest.java <https://reviews.apache.org/r/31850/#comment122968> Could we move this object as a variable of the class, since it is created in each one of the test cases? core/src/test/scala/integration/kafka/api/ProducerSendTest.scala <https://reviews.apache.org/r/31850/#comment122970> Could we reuse the producer in the ProducerSendTest class? core/src/test/scala/integration/kafka/api/ProducerSendTest.scala <https://reviews.apache.org/r/31850/#comment122973> Could we extract this to a separate test case? core/src/test/scala/integration/kafka/api/ProducerSendTest.scala <https://reviews.apache.org/r/31850/#comment122971> "i <- 0 until 50"? - Guozhang Wang On March 9, 2015, 4:14 a.m., Jiangjie Qin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31850/ > ----------------------------------------------------------- > > (Updated March 9, 2015, 4:14 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1660 > https://issues.apache.org/jira/browse/KAFKA-1660 > > > Repository: kafka > > > Description > ------- > > A minor fix. > > > 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/RecordAccumulator.java > d5c79e2481d5e9a2524ac2ef6a6879f61cb7cb5f > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java > ed9c63a6679e3aaf83d19fde19268553a4c107c2 > clients/src/main/java/org/apache/kafka/common/serialization/Serializer.java > c2fdc23239bd2196cd912c3d121b591f21393eab > > clients/src/test/java/org/apache/kafka/clients/producer/internals/RecordAccumulatorTest.java > c1bc40648479d4c2ae4ac52f40dadc070a6bcf6f > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala > 3df450784592b894008e7507b2737f9bb07f7bd2 > > Diff: https://reviews.apache.org/r/31850/diff/ > > > Testing > ------- > > Unit tests passed. > > > Thanks, > > Jiangjie Qin > >
