kirktrue commented on code in PR #21457:
URL: https://github.com/apache/kafka/pull/21457#discussion_r2818526568


##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetFetcher.java:
##########
@@ -153,11 +169,17 @@ public void onSuccess(ListOffsetResult value) {
                         
remainingToSearch.keySet().retainAll(value.partitionsToRetry);
 
                         
offsetFetcherUtils.updateSubscriptionState(value.fetchedOffsets, 
isolationLevel);
+
+                        if (isZeroTimestamp && shouldClearPartitionEndOffsets)

Review Comment:
   In that method, `isZeroTimestamp` is kind of a synonym for 'only execute a 
single pass of the loop?' In the case where `isZeroTimestamp` is true, it it's 
'possibly execute the pass multiple times.'
   
   We don't want to clear the `partitionEndOffsetsRequested` until we've 
finished all the passes of the loop we're going to make. So if 
`shouldClearPartitionEndOffsets` is true but `isZeroTimestamp` is false, 
clearing the `partitionEndOffsetsRequested` flag could be premature because 
there could be enough time on the timer for a second pass of the loop that 
finds the offsets for a partition that the first pass didn't.
   
   I agree that it's confusing. I've tried a couple of different approaches, 
but they we're much clearer  😦



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