[ https://issues.apache.org/jira/browse/KAFKA-683?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13544506#comment-13544506 ]
Jun Rao commented on KAFKA-683: ------------------------------- Thanks for the patch. Some comments: 1. DefaultEventHandler.send(): Instead of val currentCorrelationId = correlationId.get()-1 it's probably better to val currentCorrelationId = correlationId.getAndIncrement() at the beginning and then reuse it when needed. 2. FileMessageSet: If initChannelPositionToEnd is true, we could be either creating a new segment or loading an existing segment during startup. So, we should rephrase the info message a bit. 3. KafkaMigrationTool: Currently, the tool requires exactly one of whitelist or blacklist. So, we will not be able to use the default value of whitelist. We can probably leave whitelist as a required argument, but put in the description of how to specify all topics correctly (ie, .*). 4. Log: 4.1 maybeRoll(): Rechecking the condition has the problem that time-based condition may not return the same value. We can probably check each condition once and if the condition is true, log the cause and set a boolean var shouldRoll to true. 4.2 markDeletedWhile(): For the new logging, should we somehow indicate that those logs are from this function? Also, it seems that we log whether all current index/data files exist. Should we log the name of the index/data files too so that we know which ones are missing? 5. javaapi.TopicMetadataRequest: The scala optional parameter for correlationId won't work for java. We will have to manually create two constructors, one with correlation id and the other without. 6. RequestChannel: We are already logging the whole request which includes clientid, correlationid and versionid. So, there is no need to log them explicitly. 7. config/log4j.properties: All scripts in bin/ currently uses this file. The changes are really intended for Kafka broker. Perhaps we can create a new log4j file just for the broker and change the kafka broker scripts accordingly. Also, for kafka broker, should we log to both file and console? Finally, I got the following warning when running the kafka server startup script. log4j:WARN No such property [maxBackupIndex] in org.apache.log4j.FileAppender. log4j:WARN No such property [maxFileSize] in org.apache.log4j.FileAppender. log4j:WARN No such property [maxBackupIndex] in org.apache.log4j.FileAppender. log4j:WARN No such property [maxFileSize] in org.apache.log4j.FileAppender. 8. Was the state change log added? I didn't see the change in the scala code or the log4j property file. > Fix correlation ids in all requests sent to kafka > ------------------------------------------------- > > Key: KAFKA-683 > URL: https://issues.apache.org/jira/browse/KAFKA-683 > Project: Kafka > Issue Type: Improvement > Affects Versions: 0.8 > Reporter: Neha Narkhede > Assignee: Neha Narkhede > Priority: Critical > Labels: improvement, replication > Attachments: kafka-683-v1.patch > > > We should fix the correlation ids in every request sent to Kafka and fix the > request log on the broker to specify not only the type of request and who > sent it, but also the correlation id. This will be very helpful while > troubleshooting problems in production. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira