Hi Ziming,

I'm sorry it took me a while to reply.

Thank you for having a look at this KIP and providing feedback.

> 1. We have a version field in meta.properties, currently it’s 1, and we can
> set it to 2 in this KIP, and we can give an example of server.properties and
> it’s corresponding meta.properties generated by the storage command tool.

> 2. When using storage format tool we should specify cluster-id, it will be an
> arduous work if we should manually specify directory-ids for all log dirs,
> I think you can make it more clear about this change that the directory-ids
> are generated automatically.

Thank you for these suggestions. I've updated the KIP to:

* include an example
* clarify that the version field would be changed to 2
* clarify that the log directory UUIDs are automatically generated

> 3. When controller place a replica, it will select a broker as well as a log
> directory, the latter is currently accomplished in the broker side, so this
> will be a big change?

I think there can be benefits, as Jason described previously, if we change how
log directories are assigned as follow-up work.

>From a codebase surface area perspective, it is definitely a big change
because there are many models, types and interfaces that assume replicas are
identified solely by a broker id.
That will have to change to include the directory UUID, many lines of code will
be affected.

But in terms of behavior it shouldn't be a big change at all. Brokers currently
select the log directory with the least logs in it. This isn't a very nice
policy, as logs can have wildly different sizes, and log directories can have
different capacities. But it is a policy that the controller can keep.

If we decide to extend the selection policy and keep it in the broker,
the broker will continue to be able to override the controller's selection
of log directory, using the `AssignReplicasToDirectories` RPC.

> 4. When handling log directory failures we will update Leader and ISR using
> the existing replica state machine, what is this state machine referring to,
> do you mean the AlterPartition RPC?

No, I mean that it will use the same rules and mechanism
(`PartitionChangeBuilder`) that is used when a broker is fenced, shutdown or
unregistered.

I think maybe the term "replica state machine" isn't the very appropriate.
I've updated the KIP to rephrase this section.

Thanks,

--
Igor

Reply via email to