kevin-wu24 commented on code in PR #22191:
URL: https://github.com/apache/kafka/pull/22191#discussion_r3683318009


##########
metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java:
##########
@@ -610,6 +628,17 @@ public void replay(UnregisterBrokerRecord record) {
         }
     }
 
+    public void replay(UnregisterControllerRecord record) {
+        int controllerId = record.controllerId();
+        ControllerRegistration registration = 
controllerRegistrations.remove(controllerId);
+        if (registration == null) {
+            throw new RuntimeException(String.format("Unable to replay %s: no 
controller " +

Review Comment:
   > UnregisterControllerRecord doesn't have a brokerEpoch field. Why the 
inconsistency?
   
   The main reasons are that `brokerEpoch` is used for broker lifecycle 
management between broker and active controller, such as fencing, unfencing, 
and clean shutdown detection.`brokerEpoch` also provides the active controller 
with a mechanism for detecting a request from a "zombie-broker" 
incarnation/registration and reject it. 
   
   However, controller registration is more "simple" IMO, in that they only 
really are needed by the metadata layer to know each controller's endpoints + 
supported features. These are properties which only change on a process 
restart, and controllers will refresh their registrations on restart because 
they will not see their incarnation ID in the existing registration.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to