Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Kafka, mail # dev - Review Request 18343: KAFKA-1250: Add logging to new Kafka producer.


Copy link to this message
-
Re: Review Request 18343: KAFKA-1250: Add logging to new Kafka producer.
Jun Rao 2014-02-21, 18:13

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: