Hi Jose,
This looks useful. One comment I had is whether we can improve the leader
election tool. Needing to provide a json file is a bit annoying. Could we
have a way to specify partitions directly through the command line? Often
when we need to enable unclean leader election, it is just one or a small
set of partitions. I'd hope to be able to do something like this.
bin/kafka-elect-leaders.sh --allow-unclean --topic foo --partition 1
--bootstrap-server localhost:9092
Also there's a comment if the json file is not provided, the help document
says "Defaults to all existing partitions." I assume we would not keep this
behavior?
The only other question I had is whether we ought to deprecate
`AdminClient.electPreferredLeaders`?
Thanks,
Jason
On Thu, Apr 25, 2019 at 11:23 AM Jose Armando Garcia Sancio <
jsan...@confluent.io> wrote:
> On Thu, Apr 25, 2019 at 8:22 AM Colin McCabe <cmcc...@apache.org> wrote:
> >
> > On Wed, Apr 24, 2019, at 21:04, Jose Armando Garcia Sancio wrote:
> > > Thanks for the reply. Comments below.
> > >
> > > On Wed, Apr 24, 2019 at 6:07 PM Colin McCabe <cmcc...@apache.org>
> wrote:
> > > > What's the rationale for using an int8 rather than just having a
> boolean
> > > > that is either true or false for "unclean"? We only have two values
> now,
> > > > and it seems like we could modify the RPC schema in the future if
> needed.
> > > > Or is the intention to add more flags later?
> > > >
> > >
> > > There are two reason:
> > >
> > > 1. The controller supports 4 (5 technically) different election
> > > algorithms. We are only exposing "preferred" and "unclean" through
> the
> > > admin client because we only have use cases for those two election
> types.
> > > It is possible that in the future we may want to support more
> algorithms.
> > > This would allow us to make that change easier.
> > > 2. I believe that an enum is more descriptive than a boolean flag
> as it
> > > is not a matter of "unclean" vs "clean" or "preferred" vs
> "non-preferred".
> > > 1. Preferred means that the controller will attempt to elect
> only the
> > > fist replica describe in the partition assignment if it is
> > > online and it is
> > > in-sync.
> > > 2. Unclean means that the controller will attempt to elect the
> first
> > > in-sync and alive replica given the order of the partition
> assignment. If
> > > this is not satisfied it will attempt to elect the first replica
> in the
> > > assignment that is alive.
> > >
> >
> > OK, that makes sense.
> >
> > On an unrelated note, you can simplify your protocol definition to this,
> I believe:
> >
> > { "name": "Partitions", "type": "[]Partitions", "versions": "1+",
> > "about": "The partitions of this topic whose leader should be
> elected.",
> > "fields": [
> > { "name": "PartitionId", "type": "int32", "versions": "0+",
> > "about": "The partition id." },
> > { "name": "ElectionType", "type": "int8", "versions": "1+",
> > "about": "Type of elections to conduct for the
> partition. A value of '0' elects the preferred leader. A value of '1'
> elects an unclean leader if there are no in-sync leaders." }
> > ]
> > }
>
> Great suggestion. I made one modification to the "versions" field for
> "Partitions". Let me know which one is correct. The KIP should have
> the final result.
>
> >
> > The reason is because the v0 array of ints is equivalent on the wire to
> an array of structures that only have an int inside.
> >
> > In other words, this:
> > { "type": "[]int32" }
> >
> > is just another way of saying this:
> > { "type": "[]MyArrayType", "fields": [
> > { "name": "MyInt", "type": "int32", } ] }
>
> Very cool. This is the definition of zero overhead abstraction.
>
> -Jose
>
