My 2 cents about naming ...
PartitionCount or NumPartitions sound better to me providing an "absolute" value (as today the kafka-topics script work) for an idempotent operation while the NumPartitionsIncrease name sounds to me like the "increment" value. Paolo Patierno Senior Software Engineer (IoT) @ Red Hat Microsoft MVP on Windows Embedded & IoT Microsoft Azure Advisor Twitter : @ppatierno<http://twitter.com/ppatierno> Linkedin : paolopatierno<http://it.linkedin.com/in/paolopatierno> Blog : DevExperience<http://paolopatierno.wordpress.com/> ________________________________ From: Tom Bentley <t.j.bent...@gmail.com> Sent: Friday, September 8, 2017 9:39 AM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-195: AdminClient.increasePartitions Hi Ismael, Thanks for the comments. My bad for not noticing the custom assignment requirement. The current > proposal has the following example (I updated it a little so that 2 > partitions are added): > > increasePartitionCount(4, asList(asList(1, 2), asList(2, 3)) > > Why not simply provide the assignment? For example, if you want to add 2 > partitions, you'd simply do: > > increasePartitionCount(asList(asList(1, 2), asList(2, 3)) > > Not sure why need the number. kafka-topics.sh allows to increase the number of partitions without supplying an assignment, so one reason is simply to be able to support that. When you don't supply an assignment you're leaving it to the cluster to decide for you. By requiring an assignment you're forcing the user to decide. The user might not care much and thus make a worse choice than if you'd left it to the server. > The other two questions: > > 2. Do we want to allow people to add non-consecutive partition ids? This is > possible to do with the current AdminUtils, but it's not exactly supported > (although it apparently works fine in the broker). Still, I wanted to make > sure we considered it. > I admit I had assumed this wasn't possible. How does partitioning work if there are holes? You would need the list of partition ids in order to produce a correct partition id. Suspending my scepticism for a moment, to support something like that we'd have to change the List<List<Integer>> assignment to a Map<Integer, List<Integer>>, so the request explicitly identified the new topics, rather than it being implied. That would make it slightly less easy to form a valid request for the normal case of consecutive partition ids: You'd have to actually know the current number of partitions, which might necessitate a describeTopics(). It doesn't sound like there are any known use cases for non-consecutive partition ids. It also sounds like whatever existing support there is might be only lightly tested. It sounds like a source of gotchas and subtle bugs to me (both in Kafka itself and for users). I have to wonder whether it would be worth supporting this. If we decide not to support it, we should fix the rest of the AdminClient so it's not possible to create non-consecutive partition ids. WDYT? 3. Do we want to provide the target partition count or the number we want > to increase it by? This is related to the first point as well. Thinking > about it, one benefit of specifying the target number is that the operation > is then idempotent. If we state the number we want to increase by, it's > easier to make a mistake and increase it twice under failure scenarios. Was > that the motivation for specifying the target partition count? > > Right, if you're just supplying an increment you can't be certain what you're incrementing it to (which is what you actually care about). And idempotency is so nice to have if something goes wrong. Using an increment would make the `NumPartitionIncrease` class a bit more easily understood, as then the outer list would have to be the same size as the increment. But for me idempotency is the more valuable feature.
