Hi Igor, 20. The description of the changes to meta.properties says "If there any meta.properties file is missing directory.id a new UUID is generated, and assigned to that log directory by updating the file", and the upgrade/migration section says "As the upgraded brokers come up, the existing meta.properties files in each broker are updated with a generated directory.id and directory.ids ." Currently MetaProperties#parse() checks that the version is 1, so would the described behaviour prevent downgrade of a broker to an older version of the software?
21. "If the indicated log directory UUID is not a registered log directory then the call fails with an error" can you specify which error (is it a new error code)? 22. "If multiple log directories are registered the broker will remain fenced until the controller learns of all the partition to log directory placements in that broker - i.e. no remaining replicas assigned to Uuid.ZERO ." Is an error code used in the BrokerHeartbeatResponse to indicate this state? (Or is the only way to diagnose the reason for a broker remaining fenced for this reason to look at the controller logs?) 23. Will there be a system test to cover the upgrade of a ZK+JBOD cluster to KRaft+JBOD cluster? 24. In the rejected alternatives: "However the broker is in a better position to make a choice of log directory than the broker". I think that should be "...than the controller", right? 25. I wonder about the inconsistency of the RPC names: We have the existing AlterReplicaLogDirs (and log.dirs broker config), but the new RPC is AssignReplicasToDirectories. Many thanks! Tom On Tue, 3 Jan 2023 at 18:05, Igor Soarez <soa...@apple.com.invalid> wrote: > Hi Jun, > > Thank you for having another look. > > 11. That is correct. I have updated the KIP in an attempt to make this > clearer. > I think the goal should be to try to minimize the chance that a log > directory > may happen while the metadata is incorrect about the log directory > assignment, > but also have a fallback safety mechanism to indicate to the controller > that > some replica was missed in case of a bad race. > > 13. Ok, I think I have misunderstood this. Thank you for correcting me. > In this case the broker can update the existing meta.properties and create > new meta.properties in the new log directories. > This also means that the update-directories subcommand in kafka-storage.sh > is not necessary. > I have updated the KIP to reflect this. > > Please have another look. > > > Thank you, > > -- > Igor > > > > On 22 Dec 2022, at 00:25, Jun Rao <j...@confluent.io.INVALID> wrote: > > > > Hi, Igor, > > > > Thanks for the reply. > > > > 11. Yes, your proposal could work. Once the broker receives confirmation > of > > the metadata change, I guess it needs to briefly block appends to the old > > replica, make sure the future log fully catches up and then make the > switch? > > > > 13 (b). The kafka-storage.sh is only required in KIP-631 for a brand new > > KRaft cluster. If the cluster already exists and one just wants to add a > > log dir, it seems inconvenient to have to run the kafka-storage.sh tool > > again. > > > > Thanks, > > > > Jun > >
