Hi Tom, It won't be used within Kafka, but it's a public API that can be used by other projects. And the protocol can be used by non-Java clients. So, there is still value in including it.
Regarding the dynamic configs, I personally still think we should have a specific protocol API for that. With regards to throttling, it would be worth thinking about a way where the throttling configs can be automatically removed without the user having to re-run the tool. So, yes, maybe it should be a separate KIP as well. Not sure if we need it in the template, but you're welcome to mention any dependencies when there are some. Thanks, Ismael On Thu, Sep 7, 2017 at 5:15 PM, Tom Bentley <t.j.bent...@gmail.com> wrote: > Hi Ismael, > > It would be good to get at least some of this into 1.0.0. > > We could put the increasePartitions() work into another KIP, but it would > be an unused AdminClient API in that release. The consumer of this API will > be the TopicsCommand when that get refactored to use the AdminClient. > That's something Paolo Patierno is proposing to do but afaik not in time > for 1.0.0. I don't think that's an issue, though, so I'll split out a > separate KIP. > > FWIW, we could also split out the proposal to support describeConfigs() and > alterConfigs() for dynamic configs into a separate KIP too. But that's also > a decision we can defer until we're looking at the remainder of KIP-179. > > Aside: I wonder if it would be a good idea for the KIP template to have a > "Depends on" field so people can more easily keep track of how multiple > in-flight KIPs depend on one another? > > Cheers, > > Tom > > On 7 September 2017 at 16:42, Ismael Juma <ism...@juma.me.uk> wrote: > > > Hi Tom, > > > > What do you think of moving `increasePartitionsCount` (or > > `increaseNumPartitions`) to a separate KIP? That is simple enough that we > > could potentially include it in 1.0.0. I'd be happy to review it. > > ReassignPartitions is more complex and we can probably aim to include > that > > in the January release. What do you think? > > > > Ismael > > > > On Wed, Sep 6, 2017 at 11:42 PM, Colin McCabe <cmcc...@apache.org> > wrote: > > > > > On Wed, Sep 6, 2017, at 00:20, Tom Bentley wrote: > > > > Hi Ted and Colin, > > > > > > > > Thanks for the comments. > > > > > > > > It seems you're both happier with reassign rather than assign, so I'm > > > > happy > > > > to stick with that. > > > > > > > > > > > > On 5 September 2017 at 18:46, Colin McCabe <cmcc...@apache.org> > wrote: > > > > > > > > > ... > > > > > > > > > > > > > Do we expect that reducing the number of partitions will ever be > > > > > supported by this API? It seems like decreasing would require a > > > > > different API-- one which supported data movement, had a "check > > status > > > > > of this operation" feature, etc. etc. If this API is only ever > going > > > to > > > > > be used to increase the number of partitions, I think we should > just > > > > > call it "increasePartitionCount" to avoid confusion. > > > > > > > > > > > > > I thought a little about the decrease possibility (hence the static > > > > factory > > > > methods on PartitionCount), but not really in enough detail. I > suppose > > a > > > > decrease process could look like this: > > > > > > > > 1. Producers start partitioning based on the decreased partition > count. > > > > 2. Consumers continue to consume from all partitions. > > > > 3. At some point all the records in the old partitions have expired > and > > > > they can be deleted. > > > > > > > > This wouldn't work for compacted topics. Of course a more aggressive > > > > strategy is also possible where we forcibly move data from the > > partitions > > > > to be deleted. > > > > > > > > Anyway, in either case the process would be long running, whereas the > > > > increase case is fast, so the semantics are quite different. So I > > agree, > > > > lets rename the method to make clear that it's only for increasing > the > > > > partition count. > > > > > > > > > > > > > > > Outstanding questions: > > > > > > > > > > > > 1. Is the proposed alterInterReplicaThrottle() API really better > > than > > > > > > changing the throttle via alterConfigs()? > > > > > > > > > > That's a good point. I would argue that we should just use > > > alterConfigs > > > > > to set the broker configuration, rather than having a special RPC > > just > > > > > for this. > > > > > > > > > > > > > Yes, I'm minded to agree. > > > > > > > > The reason I originally thought a special API might be better was if > > > > people > > > > felt that the DynamicConfig mechanism (which seems to exist only to > > > > support > > > > these throttles) was an implementation detail of the throttles. But I > > now > > > > realise that they're visible via kafka-topics.sh, so they're > > effectively > > > > already a public API. > > > > > > > > > > > > > > > > > > ... > > > > > > Would it be a problem that > > > > > > triggering the reassignment required ClusterAction on the > Cluster, > > > but > > > > > > throttling the assignment required Alter on the Topic? What if a > > > user had > > > > > > the former permission, but not the latter? > > > > > > > > > > We've been trying to reserve ClusterAction on Cluster for > > > > > broker-initiated operations. Alter on Cluster is the ACL for "root > > > > > stuff" and I would argue that it should be what we use here. > > > > > > > > > > For reconfiguring the broker, I think we should follow KIP-133 and > > use > > > > > AlterConfigs on the Broker resource. (Of course, if you just use > the > > > > > existing alterConfigs call, you get this without any special > effort.) > > > > > > > > > > > > > Yes, sorry, what I put in the email about authorisation wasn't what I > > put > > > > in the KIP (I revised the KIP after drafting the email and then > forgot > > to > > > > update the email). > > > > > > > > Although KIP-133 proposes a Broker resource, I don't see one in the > > code > > > > and KIP-133 was supposedly delivered in 0.11. Can anyone fill in the > > > > story > > > > here? Is it simply because the functionality to update broker configs > > > > hasn't been implemented yet? > > > > > > Look in > > > ./clients/src/main/java/org/apache/kafka/common/config/ > > > ConfigResource.java, > > > for the BROKER resource. I bet you're looking at the Resource class > > > used for ACLs, which is a different class. > > > > > > > > > > > As currently proposed, both reassignPartitions() and > > > > alterInterBrokerThrottle() > > > > require Alter on the Cluster. If we used alterConfigs() to set the > > > > throttles then we create a situation where the triggering of the > > > > reassignment required Alter(Cluster), but the throttling required > > > > Alter(Broker), and the user might have the former but not the > latter. I > > > > don't think this is likely to be a big deal in practice, but maybe > > others > > > > disagree? > > > > > > Alter:Cluster is essentially root, though. If you have Alter:Cluster > > > and you don't have AlterConfigs:Broker, you can just create a new ACL > > > giving it to yourself (creating and deleting ACLs is one of the powers > > > of Alter:Cluster) > > > > > > cheers, > > > Colin > > > > > > > > > > > > > > > > > > > > > > > 2. Is reassignPartitions() really the best name? I find the > > > distinction > > > > > > between reassigning and just assigning to be of limited value, > and > > > > > > "reassign" is misleading when the APIs now used for changing the > > > > > > replication factor too. Since the API is asynchronous/long > running, > > > it > > > > > > might be better to indicate that in the name some how. What about > > > > > > startPartitionAssignment()? > > > > > > > > > > Good idea -- I like the idea of using "start" or "initiate" to > > indicate > > > > > that this is kicking off a long-running operation. > > > > > > > > > > "reassign" seemed like a natural choice to me since this is > changing > > an > > > > > existing assignment. It was assigned (when the topic it was > > created)-- > > > > > now it's being re-assigned. If you change it to just "assign" then > > it > > > > > feels confusing to me. A new user might ask if "assigning > > partitions" > > > > > is a step that they should take after creating a topic, similar to > > how > > > > > subscribing to topics is a step you take after creating a consumer. > > > > > > > > > > > > > Yeah, I find this new user argument persuasive, so I'm happy to stick > > > > with > > > > reassign. > > > > > > > > Thanks again for the feedback, > > > > > > > > Tom > > > > > >
