To Jason: 1. We can make the BrokerEpoch a tagged field. But I am not sure about your proposed metadata structure. In the BrokerState, do we need to store the BrokerId again? It would duplicate with ReplicaId. 2. Considering that the broker reboot data loss case is rare and Kraft is coming soon. Plus we need extra effort to - Simply asking the controller to compare the epoch with its best knowledge is not enough, because the ZK controller may not know the latest broker epoch, - The current design only helps with the delayed AlterPartition issue when the broker reboots. In ZK mode, we also need to cover the broker reboot + controller reboot scenario. If the reboot broker is in ISR already, the controller also crashes during the broker reboot, the new controller can be completely unaware of the bounced broker and select this broker as the leader. - Create a test framework to simulate the event sequence of broker reboot and registration, delayed AlterPartition request.
To Jose: 1. Thanks for the renaming advice. I will update the KIP later. 2. Ack, will update. On Tue, Feb 21, 2023 at 2:49 PM José Armando García Sancio <jsan...@confluent.io.invalid> wrote: > Hi Calvin, > > Thanks for the improvement. > > 1. In the KIP, you suggest changing the Fetch request to "Rename the > ReplicaId to BrokerId" and "Add a new Field BrokerEpoch". The Fetch > RPC is used by replicas that are not brokers, for example controllers > in KRaft. > Can we keep the name "ReplicaId" and use "ReplicaEpoch". Both KRaft > and ISR partitions have the concept of replica id and replica epoch > but not necessarily the concept of a broker. > > 2. Since the new field "BrokerEpoch '' is ignorable, should it also > have a default value? How about -1 since that is what you use in > AlterPartittion RPC. > > -- > -José >
