> On March 12, 2014, 6:19 p.m., Jay Kreps wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java, > > line 236 > > <https://reviews.apache.org/r/19132/diff/1/?file=517490#file517490line236> > > > > 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.
My reasoning is that if a message is not sent for some reason, it's probably useful for the user to know which partition the message is sent to. Since partition is determined by the producer logic, there is no other way for the client to know this if not provided by the callback. For the issue of getting invalid offset, we can potentially change the offset() method to throw an InvalidOffsetException if offset is -1. - Jun ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19132/#review36946 ----------------------------------------------------------- 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 > >
