cadonna commented on code in PR #15640:
URL: https://github.com/apache/kafka/pull/15640#discussion_r1580785461


##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -1169,8 +1129,7 @@ private Map<TopicPartition, Long> 
beginningOrEndOffset(Collection<TopicPartition
 
             Map<TopicPartition, OffsetAndTimestampInternal> 
offsetAndTimestampMap;
             offsetAndTimestampMap = applicationEventHandler.addAndGet(
-                    listOffsetsEvent,
-                    timer);
+                    listOffsetsEvent);

Review Comment:
   nit: 
   Could you please move this parameter to the previous line? 



##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -998,7 +958,7 @@ public List<PartitionInfo> partitionsFor(String topic, 
Duration timeout) {
             wakeupTrigger.setActiveTask(topicMetadataEvent.future());
             try {
                 Map<String, List<PartitionInfo>> topicMetadata =
-                        applicationEventHandler.addAndGet(topicMetadataEvent, 
timer);
+                        applicationEventHandler.addAndGet(topicMetadataEvent);

Review Comment:
   The timer is only used in the event.



##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -946,8 +907,7 @@ public Map<TopicPartition, OffsetAndMetadata> 
committed(final Set<TopicPartition
                 timer);

Review Comment:
   The timer is not used anywhere else. Maybe a deadline for this event would 
be simpler.



##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -1107,7 +1067,7 @@ public Map<TopicPartition, OffsetAndTimestamp> 
offsetsForTimes(Map<TopicPartitio
                 return listOffsetsEvent.emptyResults();
             }
 
-            return applicationEventHandler.addAndGet(listOffsetsEvent, timer)
+            return applicationEventHandler.addAndGet(listOffsetsEvent)

Review Comment:
   The timer is only used by the event. Maybe a deadline is simpler.



##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -1169,8 +1129,7 @@ private Map<TopicPartition, Long> 
beginningOrEndOffset(Collection<TopicPartition
 
             Map<TopicPartition, OffsetAndTimestampInternal> 
offsetAndTimestampMap;
             offsetAndTimestampMap = applicationEventHandler.addAndGet(
-                    listOffsetsEvent,
-                    timer);
+                    listOffsetsEvent);

Review Comment:
   I think you do actually not need the timer in this method at all. You could 
pass a deadline to the event.



##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -1026,7 +986,7 @@ public Map<String, List<PartitionInfo>> 
listTopics(Duration timeout) {
             final AllTopicsMetadataEvent topicMetadataEvent = new 
AllTopicsMetadataEvent(timer);

Review Comment:
   Also here the timer is only used in the event. Using a deadline would be 
simpler.



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