C0urante commented on a change in pull request #11046:
URL: https://github.com/apache/kafka/pull/11046#discussion_r764163807
##########
File path:
clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java
##########
@@ -725,17 +725,13 @@ public void onFailure(RuntimeException e) {
completedFetch.nextFetchOffset,
completedFetch.lastEpoch,
position.currentLeader);
- log.trace("Update fetching position to {} for partition
{}", nextPosition, completedFetch.partition);
+ log.trace("Updating fetch position from {} to {} for
partition {} and returning {} records from `poll()`",
+ position, nextPosition, completedFetch.partition,
partRecords.size());
subscriptions.position(completedFetch.partition,
nextPosition);
positionAdvanced = true;
if (partRecords.isEmpty()) {
- log.debug(
- "Advanced position for partition {} without
receiving any user-visible records. "
- + "This is likely due to skipping over
control records in the current fetch, "
- + "and may result in the consumer
returning an empty record batch when "
- + "polled before its poll timeout has
elapsed.",
- completedFetch.partition
- );
+ log.trace("Returning empty records from `poll()` "
+ + "since the consumer's position has advanced
for at least one topic partition");
Review comment:
This is true. I was hoping we could have something in here that
explicitly states that this can happen because of the change in behavior
implemented in this PR (i.e., skipping control records or aborted
transactions). If you think it's worth it to call that out in a log message we
can do that here, otherwise the entire `if (partRecords.isEmpty())` branch is
unnecessary.
--
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.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
