[
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