lianetm commented on code in PR #16241:
URL: https://github.com/apache/kafka/pull/16241#discussion_r1635211827


##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/CommitRequestManager.java:
##########
@@ -519,6 +524,7 @@ private void fetchOffsetsWithRetries(final 
OffsetFetchRequestState fetchRequest,
                 log.warn("A duplicated, inflight, request was identified, but 
unable to find it in the " +
                     "outbound buffer:" + fetchRequest);
             }
+            offsetFetchResultCache.maybeCache(fetchRequest, res);

Review Comment:
   this here means that we're caching results instead of inflight request, 
which I'm worried could be risky. Let's review this sequence, I may be missing 
something:
   
   - call to fetch offsets for tp0 (fetch1) + times out without response
   - fetch1 completes after it is expired -> we save the results in cache
   - commit offsets for tp0 -> completes successfully
   - call to fetch offsets for tp0 again (fetch2) => won't this reuse the 
cached result received for fetch1 which is not correct?



-- 
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]

Reply via email to