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? or use
`Optional` instead like "Optional<BrokerMetadata> broker(int nodeId);"
chia_02:
Do we have similar structure already to implement the "Map<TopicPartition,
PartitionMetadata> partitionsForNode(int nodeId);"? If not, it seems we
need to iterate all partitions to generate the collection?
Best,
Chia-Ping
Kuan Po Tseng <brandb...@apache.org> 於 2025年4月29日 週二 下午6:44寫道：
> 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 compatible too: once users switch over to
> the new interface, they won’t need to recompile after we remove the old
> interface.
>
> By the way, I’ve also updated the KIP based on this idea. Would be great
> if you could take a look when you have some time. Thanks a lot!
>
> Best,
> Kuan-Po Tseng
>
> On 2025/04/29 08:39:55 Kuan Po Tseng wrote:
> > 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 relation to the old one. Of course,
> this would require more changes to the codebase than what we have now, but
> I’d like to hear your thoughts on this.
> >
> > Chia-Ping:
> > Thanks for pointing out the typo. I totally missed that, and I’ll make
> sure to update it. I’ll also add the items we discussed to the "rejected
> alternatives" section as you suggested.
> >
> > Thanks again!
> >
> > Best,
> > Kuan-Po Tseng
> >
> > On 2025/04/29 08:18:34 Chia-Ping Tsai wrote:
> > > 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 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
> > > > users who to want migrate to new `updateClusterMetadata` should make
> old
> > > > `updateClusterMetadata` throw exception and implement new
> > > > `updateClusterMetadata`.
> > > > For example:
> > > >
> > > > ```
> > > > MyCallback implements ClientQuotaCallback {
> > > >
> > > > boolean updateClusterMetadata(Cluster cluster) {
> > > > throw new UnsupportException();
> > > > }
> > > >
> > > > override boolean updateClusterMetadata(ClusterMetadata
> clusterMetadata)
> > > > {
> > > > // new implementation
> > > > }
> > > > }
> > > > ```
> > > >
> > > >
> > > > Should we instead have a new interface (which can inherit from the
> old
> > > > interface, for now)?
> > > >
> > > > Yes, we had discussed this approach. We can dynamically create the
> object
> > > > based on the new interface first, and then try the old interface for
> users
> > > > who are still using it. Additionally, a callback implementing the old
> > > > interface can be wrapped to conform to the new interface. However,
> this
> > > > still exposes the "deprecated" method/structure to users, even as
> they are
> > > > migrating to the new interface.
> > > >
> > > > A more direct approach is to have the new interface *not* extend the
> old
> > > > one. This ensures that users will definitely not interact with the
> old
> > > > structure when using the new interface. Of course, wrapping will
> still be
> > > > necessary. This approach was used when migrating the public APIs of
> tools
> > > > from Scala to Java (KIP-641).
> > > > Best,
> > > > Chia-Ping
> > > >
> > > >
> > > >
> > > >
> > > > Colin McCabe <cmcc...@apache.org> 於 2025年4月29日 週二 上午4:38寫道：
> > > >
> > > > > 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 than
> > > > > ideal, since once we remove that deprecated function, it will break
> > > > > compilation for anyone implementing it. Should we instead have a
> new
> > > > > interface (which can inherit from the old interface, for now)?
> That would
> > > > > avoid this issue...
> > > > >
> > > > > best,
> > > > > Colin
> > > > >
> > > > >
> > > > > On Tue, Apr 22, 2025, at 09:23, Kuan Po Tseng wrote:
> > > > > > Hello everyone,
> > > > > >
> > > > > > I’d like to kick off a discussion about redesigning
> > > > > > ClientQuotaCallback#updateClusterMetadata.
> > > > > > As it stands, the current implementation is quite inefficient,
> since it
> > > > > > requires creating
> > > > > > a full Cluster object each time any change is made. On top of
> that, some
> > > > > of
> > > > > > the information
> > > > > > within the Cluster object is outdated.
> > > > > >
> > > > > > With that in mind, I’d love to propose a lighter, more efficient
> > > > > solution,
> > > > > > along with a redefined
> > > > > > interface for the object passed into updateClusterMetadata. If
> you have a
> > > > > > moment, I’d really appreciate your thoughts and feedback on this.
> > > > > >
> > > > > > Thank you!
> > > > > >
> > > > > > KIP link: https://cwiki.apache.org/confluence/x/zInoF
> > > > > >
> > > > > > Best,
> > > > > > Kuan-Po Tseng
> > > > >
> > > >
> > >
> >
>
