lianetm commented on code in PR #23014:
URL: https://github.com/apache/kafka/pull/23014#discussion_r3778667611
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractFetch.java:
##########
@@ -433,9 +432,14 @@ protected Map<Node, FetchSessionHandler.FetchRequestData>
prepareFetchRequests()
List<TopicPartition> unbuffered = fetchablePartitions(buffered);
if (unbuffered.isEmpty()) {
- // If there are no partitions that don't already have data locally
buffered, there's no need to issue
- // any fetch requests at the present time.
- return Collections.emptyMap();
+ // If every currently fetchable partition already has buffered
data, there is no need to issue
+ // additional fetch requests. This is a safe point to wake the
buffer immediately because progress
+ // can be made by consuming the buffered data. If no partitions
are fetchable at all (for example,
+ // no assignment yet, invalid positions, paused, or pending
revocation/callback), the state will
+ // not change until some external event occurs, so an immediate
wakeup would only busy-loop the
+ // caller rather than allowing the normal backoff to apply.
Review Comment:
this comment hints allowing backoff if pending revocation, but that's not
what we're doing at the moment in `maximumTimeToWait`, right? (related to my
comment about the case of consumer revoking all partitions)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
