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. 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. 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? Ismael On Fri, Sep 8, 2017 at 9:22 AM, Tom Bentley <t.j.bent...@gmail.com> wrote: > Hi Ted and Ismael, > > Thanks for the comments. > > Ted, I've fixed the KIP for your comments. > > Ismael, see responses inline: > > On 8 September 2017 at 02:00, Ismael Juma <ism...@juma.me.uk> wrote: > > > Thanks Tom. Thanks for the KIP. A few comments: > > > > 1. Does the `PartitionCount` class still make sense given that the method > > can only increase the number of partitions now? > > > > Yes, it still makes sense because its optional to supply assignments of the > new partitions to brokers. I'm not totally convinced it's the best name > though. > > > > 2,. In `NewTopic`, we have `numPartitions`. Should we keep using that > > variant and call the method `increaseNumPartitions`? > > > > I don't mind, renaming it. It would make the `PartitionCount` name even > less convincing though, so perhaps the right name for that would be > `NumPartitionIncrease`? > > > > 3. Since this has been discussed at length as part of the reassign > > partitions KIP, I suggest starting the vote tomorrow if there are no > > objections from others. > > > > I'll start the vote today unless anyone raises points that I can't address. > > Thanks, > > Tom > > > > > > Thanks, > > Ismael > > > > On Thu, Sep 7, 2017 at 5:38 PM, Tom Bentley <t.j.bent...@gmail.com> > wrote: > > > > > As suggested by Ismael, I've factored the increasePartitionCounts() API > > out > > > of KIP-179 out into a separate KIP which hopefully can progress more > > > quickly. > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-195% > > > 3A+AdminClient.increasePartitions > > > > > > If you've looked at KIP-179 in the last few days there's no much new to > > see > > > here, but if not you should find KIP-195 a lighter read. > > > > > > Cheers, > > > > > > Tom > > > > > >
