----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18343/#review35163 -----------------------------------------------------------
1. There is a total of 16 printStackTrace() in the code and only 9 of them are converted to log. 2. Could we log all overridden config values in the producer? clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java <https://reviews.apache.org/r/18343/#comment65580> send => sending clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java <https://reviews.apache.org/r/18343/#comment65581> Could we distinguish the error message here from the one in line 152? clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java <https://reviews.apache.org/r/18343/#comment65582> The usage here is inconsistent with the one in RecordBatch 77. Should we include a matching "{}" for exception or not? clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java <https://reviews.apache.org/r/18343/#comment65583> Should we have a matching {} for exception? clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java <https://reviews.apache.org/r/18343/#comment65572> Do we need to change the license header? A few other files have the same change. clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java <https://reviews.apache.org/r/18343/#comment65573> Should this be warn? - Jun Rao On Feb. 21, 2014, 4:26 a.m., Jay Kreps wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18343/ > ----------------------------------------------------------- > > (Updated Feb. 21, 2014, 4:26 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1250 > https://issues.apache.org/jira/browse/KAFKA-1250 > > > Repository: kafka > > > Description > ------- > > KAFKA-1250 Add logging to new producer. > > > Diffs > ----- > > build.gradle 58a6396f33413cc42231f445075f36552568563a > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java > e4bc97279585818860487a39a93b6481742b91db > clients/src/main/java/org/apache/kafka/clients/producer/ProducerRecord.java > 034bf33385fe3b4222f482ccd16de6b530e1c1a7 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java > 62613a3e29a7e5ffb1cc56d267793fef72857fc6 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java > ce5cf27efa08b79e501439cf79bc8666054a5429 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java > eb16f6d236e07b16654623606294a051531b5f58 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java > e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd > clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java > c3148e5a9061d9748b617e3772c132fd89d35f05 > clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java > e08c349a6cd7617eb025709c3fb7890639ef610e > clients/src/main/java/org/apache/kafka/common/network/Selector.java > f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b > clients/src/main/java/org/apache/kafka/common/requests/RequestHeader.java > 457abb1ad44ed67ef6d015dc7859d1c4048d02fe > clients/src/main/java/org/apache/kafka/common/requests/RequestSend.java > c5e9020b2e7692c37c65bbe6bcbce5b37574baac > clients/src/main/java/org/apache/kafka/common/utils/KafkaThread.java > 9ff793f38d70d52580255d9c759c16e3c4116e10 > > Diff: https://reviews.apache.org/r/18343/diff/ > > > Testing > ------- > > > Thanks, > > Jay Kreps > >
