----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18199/#review34795 -----------------------------------------------------------
Ship it! Minor comments below clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java <https://reviews.apache.org/r/18199/#comment65112> MAX_NUM_RETRIES_CONFIG? clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java <https://reviews.apache.org/r/18199/#comment65125> This comment is not accurate, we will only return the error to user if cannot retry. clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java <https://reviews.apache.org/r/18199/#comment65130> Can we also use class hierarchy here like canRetry does instead of checking individual exception code? By doing so we move the checking definitions to the exceptions themselves instead of putting them here. clients/src/main/java/org/apache/kafka/common/errors/RetriableException.java <https://reviews.apache.org/r/18199/#comment65131> Just thought about RecordTooLargeException, for now it may be inherited from some FatalException? - Guozhang Wang On Feb. 17, 2014, 11:25 p.m., Jay Kreps wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18199/ > ----------------------------------------------------------- > > (Updated Feb. 17, 2014, 11:25 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1252 > https://issues.apache.org/jira/browse/KAFKA-1252 > > > Repository: kafka > > > Description > ------- > > KAFKA-1252 Implement retries in new producer. > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java > 3d180e885a25fa6b138f544ac320ec9e0d2a1e7f > clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java > dca9802c8d0918cf5e284ff5b5ea2ccdf788602b > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java > 52d30a86d04393ec06e8b362e91f31492a278680 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java > be8a4a399b8b43beabe2c34f8f4f728cb63a29bf > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java > 7a440a3dd29c704dac9087fdac28c35d3e33e345 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java > d93a455827a6747e02fdb388e34320f648877c34 > clients/src/main/java/org/apache/kafka/common/Cluster.java > c17a8f8162db9386a36d4204b9036676344a3451 > > clients/src/main/java/org/apache/kafka/common/errors/CorruptRecordException.java > 673f61d6271c55fe7f3422e49a3f1f8f7d3d2206 > > clients/src/main/java/org/apache/kafka/common/errors/LeaderNotAvailableException.java > 0bde6b5a351fc2475181b927effdd5ee8c7ff85c > clients/src/main/java/org/apache/kafka/common/errors/NetworkException.java > 3a041593d76cfb52471c49ca5a37ebf6129d8131 > > clients/src/main/java/org/apache/kafka/common/errors/NotLeaderForPartitionException.java > 5adc72ccf2d0cf4c0503df8d19c945a2adb2df90 > > clients/src/main/java/org/apache/kafka/common/errors/OffsetOutOfRangeException.java > d01698a3efca7d4a9ee9b20f6f56abe2f299de06 > > clients/src/main/java/org/apache/kafka/common/errors/RetriableException.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/errors/RetryableException.java > c7f2f222f712a8b6659599c25fffc4fd55772d40 > clients/src/main/java/org/apache/kafka/common/errors/TimeoutException.java > dffd64d19c35aa1f0378d220dedf619a9c115b6d > > clients/src/main/java/org/apache/kafka/common/errors/UnknownTopicOrPartitionException.java > 73d1953cbe045d7f67e423ff2f0f4d27465db41d > clients/src/main/java/org/apache/kafka/common/network/Selector.java > 8ed4c73146b2e7d8bb0bb1779dd6f942b231a627 > > clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java > 1bbe83c1bfd7599dbba12424deee302e62c7a2b5 > clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java > 41c028bffbda1c418d827a1f6a8d2a63cdf3e373 > > Diff: https://reviews.apache.org/r/18199/diff/ > > > Testing > ------- > > > Thanks, > > Jay Kreps > >
