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. > > >
