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
>
>

Reply via email to