I think increasePartitions is better name, implying there are already partitions.
bq. use cases for non-consecutive partition ids. +1 on not supporting non-consecutive partition ids Cheers On Fri, Sep 8, 2017 at 5:34 AM, Ismael Juma <ism...@juma.me.uk> wrote: > Hmm, maybe it should be createPartitions for symmetry with createTopics? > > Ismael > > On Fri, Sep 8, 2017 at 12:33 PM, Tom Bentley <t.j.bent...@gmail.com> > wrote: > > > Hi Paolo, > > > > Thanks for commenting. > > > > The main reason I suggested `NumPartitionsIncrease` rather than just > > `NumPartitions` was in case we ever implement a decreaseNumPartitions() > > API. The semantics of the class are not appropriate for using with a > > decrease API, but calling it NumPartitions suggests that it was related. > > > > Radical thought: What about if the method was called addPartitions() and > > the class was called NewPartitions? > > > > Then if an API for decreasing were ever implemented it could be > > removePartitions() with a RemovedPartitions class if necessary. > > > > Cheers, > > > > Tom > > > > On 8 September 2017 at 12:13, Paolo Patierno <ppatie...@live.com> wrote: > > > > > 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. > > > > > >
