----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19132/#review36946 -----------------------------------------------------------
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java <https://reviews.apache.org/r/19132/#comment68133> Jun, the way this works is intentional and I do not agree with this change. The user needs to handle the case where there is an error. If they don't in the current setup they get an exception (this is good). If they don't with your proposed change they will use -1 as a valid offset and potentially store this somewhere. This is actually far worse. I think the fact that the metadata can be null is documented in the javadoc. clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java <https://reviews.apache.org/r/19132/#comment68134> This should be in a separate if statement. clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java <https://reviews.apache.org/r/19132/#comment68135> Should explain why: e.g. cancelled request because the socket closed unexpectedly. clients/src/main/java/org/apache/kafka/common/utils/Utils.java <https://reviews.apache.org/r/19132/#comment68137> This is not a general purpose utility and can't go under org.apache.kafka.common at all. That package stands alone. We should instead make an ErrorLoggingCallback and put that in producer internals. - Jay Kreps On March 12, 2014, 4:31 p.m., Jun Rao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19132/ > ----------------------------------------------------------- > > (Updated March 12, 2014, 4:31 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1302 > https://issues.apache.org/jira/browse/KAFKA-1302 > > > Repository: kafka > > > Description > ------- > > Fixed the issues mentioned in the jira. Also moved errorLoggingCallback to > client utils. > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java > 1ac69436f117800815b8d50f042e9e2a29364b43 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java > 038a05a94b795ec0a95b2d40a89222394b5a74c4 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java > 7b5d144f0c72319d73d0127b8911b48117e8516c > clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java > 73ad6cdb465ed885131be21c8ba1ca1494d787f9 > clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java > 6ac2e53124d0fb9e2776a1389f097daefbad261e > clients/src/main/java/org/apache/kafka/common/utils/Utils.java > 0c6b3656375721a718fb4de10118170aacce0ea9 > core/src/main/scala/kafka/tools/newproducer/MirrorMaker.scala > d1dc13b6a3c6366b0682ef90e0be40a73d81bc4d > core/src/main/scala/kafka/utils/Utils.scala > 33e05f00d0a23d6dfec132ad0b6e45bde3960d5d > perf/src/main/scala/kafka/perf/ProducerPerformance.scala > f061dbabb49044f8fe94b8fd7dc0153c33bedeee > > Diff: https://reviews.apache.org/r/19132/diff/ > > > Testing > ------- > > > Thanks, > > Jun Rao > >
