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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
