rondagostino commented on code in PR #14838: URL: https://github.com/apache/kafka/pull/14838#discussion_r1414154141
########## metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java: ########## @@ -284,7 +286,7 @@ ReplicaPlacer replicaPlacer() { public void activate() { heartbeatManager = new BrokerHeartbeatManager(logContext, time, sessionTimeoutNs); for (BrokerRegistration registration : brokerRegistrations.values()) { - heartbeatManager.touch(registration.id(), registration.fenced(), -1); + heartbeatManager.register(registration.id(), registration.fenced(), registration.directories()); Review Comment: This change seems a bit odd to me. I think it will work, and in fact I think it will do exactly what was being done before, but you moved the creation of the broker entry in the heartbeat manager's internal map into the `register()` method and removed it from the `touch()` method -- which I think is a good change. But now this reads weirdly because we aren't registering a broker -- the broker should already have been registered by this point, and we just want to "touch" it. I think this change can be reverted? -- 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