You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/09/19 01:55:24 UTC

[GitHub] [kafka] kkonstantine commented on a change in pull request #9149: KAFKA-10340: improve the logging to help user know what is going on

kkonstantine commented on a change in pull request #9149:
URL: https://github.com/apache/kafka/pull/9149#discussion_r491251818



##########
File path: connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerSourceTask.java
##########
@@ -409,6 +410,9 @@ private boolean sendRecords() {
     // RegexRouter) topic creation can not be batched for multiple topics
     private void maybeCreateTopic(String topic) {
         if (!topicCreation.isTopicCreationRequired(topic)) {
+            log.trace("The topic creation setting is disabled or the topic name {} is already in the topic cache." +

Review comment:
       This is going to be very repetitive after the first time during which the topic might be created. 
   
   What's the hint we are trying to extract here?

##########
File path: connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerSourceTask.java
##########
@@ -363,6 +363,7 @@ private boolean sendRecords() {
             try {
                 maybeCreateTopic(record.topic());
                 final String topic = producerRecord.topic();
+                log.debug("{} is going to send record to {}", WorkerSourceTask.this, topic);

Review comment:
       Writing one log message per record, seems excessive, even for `DEBUG`
   
   There's a trace level message above for the record. Maybe we want to append the topic name there. 

##########
File path: connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerSourceTask.java
##########
@@ -430,7 +434,7 @@ private void maybeCreateTopic(String topic) {
             log.info("Created topic '{}' using creation group {}", newTopic, topicGroup);
         } else {
             log.warn("Request to create new topic '{}' failed", topic);
-            throw new ConnectException("Task failed to create new topic " + topic + ". Ensure "
+            throw new ConnectException("Task failed to create new topic " + newTopic + ". Ensure "

Review comment:
       You are referring to the inclusion of topic properties besides the topic name. Yeah that's fine. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org