lianetm commented on code in PR #21457:
URL: https://github.com/apache/kafka/pull/21457#discussion_r2830357536
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetFetcher.java:
##########
@@ -153,11 +155,17 @@ public void onSuccess(ListOffsetResult value) {
remainingToSearch.keySet().retainAll(value.partitionsToRetry);
offsetFetcherUtils.updateSubscriptionState(value.fetchedOffsets,
isolationLevel);
+
+ if (updatePartitionEndOffsetsFlag)
+
offsetFetcherUtils.clearPartitionEndOffsetRequests(remainingToSearch.keySet());
Review Comment:
I just realized we're passing a subset of the partitions here (there is a
retainAll on this collection above), so not really clearing anything
effectively if all partitions succeed? (remainingToSearch empty here)
Probably harmful because this is the successful case and for this we also
flip the flag when updating HWM/LSO, but this is still not right. Should we
pass here the original collection of `timestampsToSearch` instead? To consider
also for the failure cause even though we don't alter the collection there, but
maybe better to play safe and flip the flag for the same set of partitions that
we got intiially?
--
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]
