Re: [DISCUSS] KIP-1162: Redesign ClientQuotaCallback#updateClusterMetadata

Hi Chia-Ping, Thanks for bringing some momentum back to this discussion. The original implementation of this API had some flaws, which I attempted to address in KAFKA-19122 . However, I later realized that properly fixing the issue would require s

Re: [DISCUSS] KIP-1162: Redesign ClientQuotaCallback#updateClusterMetadata

hi Kuan-Po The method `updateClusterMetadata` hasn't been implemented for over three years after kraft was introduced. It seems we still function well without it. If the method is rarely used, introducing so many new public APIs for it seems a bit overkill. We could file a KIP to just deprecate

Re: [DISCUSS] KIP-1162: Redesign ClientQuotaCallback#updateClusterMetadata

Hi Chia-Ping, That works too. But in that case, the original method name feels inappropriate. So, as you suggested, I’ll add the topic parameter and rename the method from: Map partitionsForNode(int nodeId) to: Map partitions(String topic, int nodeId) Since the caller already knows the topic n

Re: [DISCUSS] KIP-1162: Redesign ClientQuotaCallback#updateClusterMetadata

hi Kuan-Po > In TopicImage, there is a similar method, Map partitions(), but we can't directly reuse its return value here. As you mentioned, we'll still need to iterate through all partitions to build the collection. That said, since this KIP proposes to make PartitionRegistration implements Part

Re: [DISCUSS] KIP-1162: Redesign ClientQuotaCallback#updateClusterMetadata

Hi Kuan Po Tseng, Yes, the old interface extending the new one could work. I don't have a strong preference about this, as long as someone with a correctly configured and written implementation of the NEW ClientQuotaCallback interface can upgrade from 4.x to 5.0 without hitting compatibility pr

Re: [DISCUSS] KIP-1162: Redesign ClientQuotaCallback#updateClusterMetadata

Hi Chia-Ping, chia_00: I'll update the KIP, thanks! chia_01: Regarding "partition(TopicPartition topicPartition)", it should return an Optional. For the others, we should either return an empty map (e.g., partitionsForTopic(String topic)) or an empty list (e.g., offlineReplicas(PartitionMetada

Re: [DISCUSS] KIP-1162: Redesign ClientQuotaCallback#updateClusterMetadata

hi Kuan-Po Thanks for updating KIP. This approach is better. Some comments are listed below. chia_00: Could you please update docs of `ClientQuotaCallback` to remind users to use ClientQuotaCallbackHandler instead. chia_01: what happens if users query a non-existent object? throw exception? o

Re: [DISCUSS] KIP-1162: Redesign ClientQuotaCallback#updateClusterMetadata

Hi Colin, I was thinking about it some more, and I feel like it might be better to have the old interface extend the new one. That way, it’ll be easier for us to remove the old interface later — we wouldn’t have to change too much code, just delete the old one. It would still keep everything

Re: [DISCUSS] KIP-1162: Redesign ClientQuotaCallback#updateClusterMetadata

Hi everyone, Colin: You’re right. I’ll adjust the KIP as you suggested—creating a new interface that extends the old one to avoid any compilation issues when we eventually remove the old method. However, Chia-Ping also suggested another approach: creating a completely new interface that has no

Re: [DISCUSS] KIP-1162: Redesign ClientQuotaCallback#updateClusterMetadata

hi Kuan-Po Could you please add the approaches we discussed before to "rejected" section? It could help readers to understand the picture easily. Best, Chia-Ping On 2025/04/29 04:32:52 Chia-Ping Tsai wrote: > hi Colin > > the interface `ClientQuotaCallackHandler` is a typo - the idea of this P

Re: [DISCUSS] KIP-1162: Redesign ClientQuotaCallback#updateClusterMetadata

hi Colin the interface `ClientQuotaCallackHandler` is a typo - the idea of this PR is to add new `updateClusterMetadata` to replace/deprecate old `updateClusterMetadata`. Additionally, the production code will call new `updateClusterMetadata` instead of old `updateClusterMetadata`. That means use

Re: [DISCUSS] KIP-1162: Redesign ClientQuotaCallback#updateClusterMetadata

Hi Kuan Po Tseng, Thank you for starting this discussion. This would be a very nice improvement. It seems that the current scheme requires everyone writing an implementation of ClientQuotaCallackHandler to implement the old updateClusterMetadata function that takes a Cluster. That seems less th