apalan60 commented on code in PR #19888: URL: https://github.com/apache/kafka/pull/19888#discussion_r2128416052
########## metadata/src/main/java/org/apache/kafka/controller/metrics/QuorumControllerMetrics.java: ########## @@ -267,19 +267,15 @@ public long newActiveControllers() { } public void updateBrokerContactTime(int brokerId) { - AtomicLong contactTime = brokerContactTimesMs.computeIfAbsent(brokerId, k -> new AtomicLong()); - contactTime.set(time.milliseconds()); + brokerContactTimesMs.put(brokerId, time.milliseconds()); } public int timeSinceLastHeartbeatMs(int brokerId) { - if (!brokerContactTimesMs.containsKey(brokerId)) { + Long lastTime = brokerContactTimesMs.get(brokerId); + if (lastTime == null) { return sessionTimeoutMs; - } else { - return Math.min( - (int) (time.milliseconds() - brokerContactTimesMs.get(brokerId).get()), - sessionTimeoutMs - ); Review Comment: @m1a2st Thanks for your review. My original intention was to avoid double lookup when the key exists, but I agree this results in an ugly null check. How about replacing it with `getOrDefault `instead? For example: ``` public int timeSinceLastHeartbeatMs(int brokerId) { long now = time.milliseconds(); long lastTime = brokerContactTimesMs.getOrDefault(brokerId, now - sessionTimeoutMs); return Math.min((int) (now - lastTime), sessionTimeoutMs); } ``` -- 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