lianetm commented on PR #16833: URL: https://github.com/apache/kafka/pull/16833#issuecomment-2291553679
Hey, regarding this: > The failAndRemoveExpiredFetchRequests method at L1198 clears expired fetch requests from unsentOffsetFetches. However, the partitionedBySendability collection at L1194 is created using unsentOffsetFetches, and its result does not change due to the check performed at L1198. So even if an expired fetch request is removed in failAndRemoveExpiredFetchRequests, as long as the fetch request was canSendRequest when partitionedBySendability was created, it will still be added to unsentRequests and eventually included in the PollResult . > This seems not reasonable, as expired fetch requests should not be sent again. > > I’m not sure if I missed something or if this behavior isn’t what we expected. 🤔 I believe that was intentional so that we ensure that requests are sent at least once even if timeout expired (which is what the classic consumer does). With the classic consumer, the fetch is performed in a do-while that creates the request and client.poll before checking the timer ([here](https://github.com/apache/kafka/blob/b767c655279083c0a7e958e3f319fe2cead13ddc/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java#L934-L960)). So the intention here was the same, requests are stored to be sent (in that partitionBySendability), and then expired. Makes sense? -- 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
