divijvaidya commented on code in PR #12590:
URL: https://github.com/apache/kafka/pull/12590#discussion_r1055540081
##########
clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java:
##########
@@ -2403,17 +2404,46 @@ private ClusterResourceListeners
configureClusterResourceListeners(Deserializer<
return clusterResourceListeners;
}
- private void close(long timeoutMs, boolean swallowException) {
+ private Timer createTimerForRequest(final Duration timeout) {
+ final Time localTime = (time == null) ? new SystemTime() : time;
Review Comment:
The `close()` could be called [from the
constructor](https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java#L827)
of this class itself when an exception is thrown before the field `this.time`
could be initialized. I can avoid it by setting the field as the first thing
that happens in the constructor but then we could be creating a coupling with
the initialization order of fields in the constructor to the close method.
Instead, to be on the safer side, a better approach IMO is to handle the null
case explicitly.
I have added a comment explaining when it could be null in the upcoming
commit.
--
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
