> On Feb. 26, 2014, 10:49 a.m., Neha Narkhede wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java, > > line 386 > > <https://reviews.apache.org/r/18343/diff/2/?file=504370#file504370line386> > > > > In order to meaningfully trace a request in the producer, we need to > > include the correlation id and partition with every trace message for that > > request. It will be printed as part of the entire request but the grep will > > be more complex in this case. It is better to have it explicitly say "for > > partition {} with correlation id {}". > >
Can't you just grep for the number? > On Feb. 26, 2014, 10:49 a.m., Neha Narkhede wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java, > > line 88 > > <https://reviews.apache.org/r/18343/diff/2/?file=504367#file504367line88> > > > > Should this be log.trace("Requesting metadata update for topic {}", > > topic) Oops. > On Feb. 26, 2014, 10:49 a.m., Neha Narkhede wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java, > > line 266 > > <https://reviews.apache.org/r/18343/diff/2/?file=504365#file504365line266> > > > > Should we have an equivalent trace message on start as well? Added it. > On Feb. 26, 2014, 10:49 a.m., Neha Narkhede wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java, > > line 77 > > <https://reviews.apache.org/r/18343/diff/2/?file=504369#file504369line77> > > > > In addition to logging a successful send, I would expect trace to also > > log adding a message to the appropriate partition queue and also the > > attempt to send a request to a particular leader. We log all request sends. Logging the details of the queuing seemed redundant, no? > On Feb. 26, 2014, 10:49 a.m., Neha Narkhede wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java, > > line 239 > > <https://reviews.apache.org/r/18343/diff/2/?file=504370#file504370line239> > > > > I'm wondering why this is in debug? The rest of the places where we > > have a "sending" and a "sent" message, the "sending" message is at trace > > and the "sent" message is at debug I'm happy to go either way. I guess the idea was that it is nice to see when your metadata updates and in steady-state that probably isn't so frequent it is spammy... > On Feb. 26, 2014, 10:49 a.m., Neha Narkhede wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java, > > line 330 > > <https://reviews.apache.org/r/18343/diff/2/?file=504370#file504370line330> > > > > Same here. Shouldn't this be at trace? We already have the debug > > statement for "Completed connection to node" Could go either way, basically connection creation is rare. We could make one or both of these debug. > On Feb. 26, 2014, 10:49 a.m., Neha Narkhede wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java, > > line 456 > > <https://reviews.apache.org/r/18343/diff/2/?file=504370#file504370line456> > > > > It is useful to know the reason for a retry. Shouldn't this be at error? Well retry is certainly not yet an error, right? I think the argument is that a retry is basically okay but if we want to move it up to warn I would be okay with that. > On Feb. 26, 2014, 10:49 a.m., Neha Narkhede wrote: > > clients/src/main/java/org/apache/kafka/common/Cluster.java, line 78 > > <https://reviews.apache.org/r/18343/diff/2/?file=504371#file504371line78> > > > > what is the motivation for this change? The large negative numbers in the log during initialization looked kind of scary. :-) - Jay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18343/#review35520 ----------------------------------------------------------- On Feb. 26, 2014, 5:32 a.m., Jay Kreps wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18343/ > ----------------------------------------------------------- > > (Updated Feb. 26, 2014, 5:32 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/Cluster.java > 5caaaae1d2ad11dc9bb75008b066390a0dbe2afb > clients/src/main/java/org/apache/kafka/common/Node.java > 4197e5098c6556b22e9c56f385fcaa3604eda39e > clients/src/main/java/org/apache/kafka/common/PartitionInfo.java > 08d66f1a71fc5f1f11cfdc94f376c3f92c51e923 > 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 > clients/src/main/java/org/apache/kafka/common/utils/Utils.java > 9c34e7dc82f33df7406cad0e64eb6a896d068dc6 > core/src/main/scala/kafka/server/ReplicaManager.scala > 3dd562ca93c44181bd6f75d4cbbd2669ea0d33aa > > Diff: https://reviews.apache.org/r/18343/diff/ > > > Testing > ------- > > > Thanks, > > Jay Kreps > >
