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