-
Notifications
You must be signed in to change notification settings - Fork 14.8k
KAFKA-19821: Duplicated batches should be logged #20740
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
KAFKA-19821: Duplicated batches should be logged #20740
Conversation
|
|
||
| Optional<BatchMetadata> duplicateBatch = maybeLastEntry.flatMap(e -> e.findDuplicateBatch(batch)); | ||
| if (duplicateBatch.isPresent()) { | ||
| logger.info("Found duplicate batch from client, duplicateBatchMetadata={}", duplicateBatch.get()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should move the log statement to UnifiedLog#append. Since another branch already has the log logger.trace("Appended message set with ..., it would be straightforward to add a similar log for duplicate batches
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chia7712, thanks for the suggestion! Indeed, move the log.info to append method seems better.
| appendInfo.setLastOffset(duplicate.lastOffset()); | ||
| appendInfo.setLogAppendTime(duplicate.timestamp()); | ||
| appendInfo.setLogStartOffset(logStartOffset); | ||
| logger.info("Duplicate batch detected, returning AppendInfo from duplicate batch with last offset: {}, first offset: {}, next offset: {}, skipped messages: {}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit concerned about the log flooding if the producer has network problems. Wouldn't the trace level be adequate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m good with either info or trace. As long as we leave a debug message behind, that’s fine 😃
When a duplicate batch is detected, the entire MemoryRecords instance is skipped to prevent appending duplicate data to the log. This operation is silent, adding a log.info message here to provide better observability. Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
When a duplicate batch is detected, the entire MemoryRecords instance is skipped to prevent appending duplicate data to the log. This operation is silent, adding a log.info message here to provide better observability. Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
When a duplicate batch is detected, the entire MemoryRecords instance is
skipped to prevent appending duplicate data to the log. This operation
is silent, adding a log.info message here to provide better
observability.
Reviewers: Chia-Ping Tsai chia7712@gmail.com