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


##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -1072,12 +1073,10 @@ public Map<TopicPartition, OffsetAndTimestamp> 
offsetsForTimes(Map<TopicPartitio
             }
 
             try {
-                return applicationEventHandler.addAndGet(listOffsetsEvent)
-                    .entrySet()
-                    .stream()
-                    .collect(Collectors.toMap(
-                        Map.Entry::getKey,
-                        entry -> entry.getValue().buildOffsetAndTimestamp()));
+                Map<TopicPartition, OffsetAndTimestampInternal> offsets = 
applicationEventHandler.addAndGet(listOffsetsEvent);
+                Map<TopicPartition, OffsetAndTimestamp> results = new 
HashMap<>(offsets.size());
+                offsets.forEach((k, v) -> results.put(k, v != null ? 
v.buildOffsetAndTimestamp() : null));

Review Comment:
   
[`OffsetFetchUtils.buildListOffsetsResult()`](https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetFetcherUtils.java#L243-L258)
 is used by the `ClassicKafkaConsumer` when building up the results for 
`offsetsForTimes()`. That method has two loops—the first loops over the 
`TopicPartition`s that were _requested_ and fills in `offsetsResults` with 
`null`. The second loops over the `TopicPartition`s with _results_ and replaces 
the `null` with the `OffsetAndTimestamp`. The idea was to mimic that in the 
`AsyncKafkaConsumer` implementation.
   
   Thoughts?



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

Reply via email to