chia7712 commented on code in PR #19888: URL: https://github.com/apache/kafka/pull/19888#discussion_r2129117111
########## 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: @apalan60 Yes, it puts the broker ID before creating the gauge. However, there's another path used to delete the item. Currently, that's fine, as it removes the gauge before removing the item. Nevertheless, it's error-prone if the order changes down the line, and I believe keeping the defensive code brings no side effects. -- 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