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

Reply via email to