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