No. Previously we create `/controller_epoch` on-demand if it does not exist when we try to increment controller epoch. IMO, this makes the code hard to read and reason about, especially after this patch because in that way we need to either create `\controller_epoch` if not exists and retry the atomic operation or we have two different atomic operations (one for check+create, the other one for create+create).
I think `\controller_epoch` should pre-exists before we actually use it, like other persistent zk paths (e.g. /brokers, /admin/delete_topics) . So I have included `/controller_epoch` in the "PersistentZkPaths" so that it will be created if not exists on broker startup. In this case, `getControllerEpoch` should not return None unless admin deletes `/controller_epoch` explicitly, which will ruin the cluster anyway. One drawback of pre-creating `\controller_epoch` is that admin now cannot re-initialize controller epoch by simply deleting `\controller_epoch`. Instead, admin should delete `/controller` and `/controller_epoch`, then re-create `\controller_epoch` to achieve this. But I don't know whether that is a valid use case. May I have your opinion? [ Full content available at: https://github.com/apache/kafka/pull/5101 ] This message was relayed via gitbox.apache.org for [email protected]
