Hi, Jose, Thanks for the reply.
JS1. I agree that returning BROKER_ID_NOT_REGISTERED for an UnregisterControllerRequest is inconsistent. Renaming BROKER_ID_NOT_REGISTERED to NODE_ID_NOT_REGISTERED introduces a change in the existing protocol and we probably need to bump up the UnregisterBrokerRequest version to reflect this change. Another way is to introduce a new CONTROLLER_ID_NOT_REGISTERED error code. Also, "NodeIdNotRegisteredException extends BrokerIdNotRegisteredException" seems unintuitive. A node is more general than a broker. So, it seems that BrokerIdNotRegisteredException should extend NodeIdNotRegisteredException, not the other way. Jun On Tue, May 12, 2026 at 5:16 AM José Armando García Sancio via dev < [email protected]> wrote: > Hi Kevin, thanks for the new feature. After introducing KIP-853, we > need to allow the user to unregister the controller from the cluster. > > JS1 > > a BROKER_ID_NOT_REGISTERED error if no registration exists for the > requested controller ID > > Is it worth cleaning up this error in this KIP? The error name is not > part of the public API but the Java exception is part of the public > API. How about: > 1. Rename BROKER_ID_NOT_REGISTERED to NODE_ID_NOT_REGISTERED > 2. Add this exception "NodeIdNotRegisteredException extends > BrokerIdNotRegisteredException". The admin client will always throw > NodeIdNotRegisteredException but legacy application can still catch > and handle BrokerIdNotRegisteredException. > > JS2 > > Use UnregisterBrokerRecord to unregister controllers > > You could reuse the metadata record with API key 1 to implement this > feature. The active controller would have to check that the finalized > metadata version support this feature before writing this record with > a controller id. Since the name UnregisterBrokerRecord is not part of > the public API, the implementation can rename this record to > UnregisterNodeRecord and rename the field brokerId to nodeId. Since > controllers currently lack an epoch, this field can be ignore for > controller unregistration. If you want to clean up that field you can > make it a tagged field and introduce version 1 of the > UnregisterNodeRecord record. > > What do you think? > -- > -José >
