Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2024-03-19 Thread Jorge Esteban Quilcate Otoya
Sorry I missed that comment on the thread. Proposal looks great, thanks, Abhijeet! On Sat, 16 Mar 2024 at 13:19, Abhijeet Kumar wrote: > Hi Jorge, > > The configs name was chosen to keep it consistent with the other existing > quota configs, such as >

Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2024-03-16 Thread Abhijeet Kumar
Hi Jorge, The configs name was chosen to keep it consistent with the other existing quota configs, such as *replica.alter.log.dirs.io.max.bytes.per.second* as pointed out by Jun in the thread. Also, we can revisit the names of the components during implementation, since those are not exposed to

Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2024-03-11 Thread Jorge Esteban Quilcate Otoya
Hi Abhijeet, Thanks for the KIP! Looks good to me. I just have a minor comments on naming: Would it be work to align the config names to existing quota names? e.g. `remote.log.manager.copy.byte.rate.quota` (or similar) instead of `remote.log.manager.copy.max.bytes.per.second`? Same for new

Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2024-03-08 Thread Abhijeet Kumar
Thank you all for your comments. As all the comments in the thread are addressed, I am starting a Vote thread for the KIP. Please have a look. Regards. On Thu, Mar 7, 2024 at 12:34 PM Luke Chen wrote: > Hi Abhijeet, > > Thanks for the update and the explanation. > I had another look, and it

Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2024-03-06 Thread Luke Chen
Hi Abhijeet, Thanks for the update and the explanation. I had another look, and it LGTM now! Thanks. Luke On Tue, Mar 5, 2024 at 2:50 AM Jun Rao wrote: > Hi, Abhijeet, > > Thanks for the reply. Sounds good to me. > > Jun > > > On Sat, Mar 2, 2024 at 7:40 PM Abhijeet Kumar > wrote: > > > Hi

Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2024-03-04 Thread Jun Rao
Hi, Abhijeet, Thanks for the reply. Sounds good to me. Jun On Sat, Mar 2, 2024 at 7:40 PM Abhijeet Kumar wrote: > Hi Jun, > > Thanks for pointing it out. It makes sense to me. We can have the following > metrics instead. What do you think? > >- remote-(fetch|copy)-throttle-time-avg (The

Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2024-03-02 Thread Abhijeet Kumar
Hi Jun, Thanks for pointing it out. It makes sense to me. We can have the following metrics instead. What do you think? - remote-(fetch|copy)-throttle-time-avg (The average time in ms remote fetches/copies was throttled by a broker) - remote-(fetch|copy)-throttle-time--max (The maximum

Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2024-02-28 Thread Jun Rao
Hi, Abhijeet, Thanks for the reply. The issue with recording the throttle time as a gauge is that it's transient. If the metric is not read immediately, the recorded value could be reset to 0. The admin won't realize that throttling has happened. For client quotas, the throttle time is tracked

Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2024-02-28 Thread Abhijeet Kumar
Hi Jun, Clarified the meaning of the two metrics. Also updated the KIP. kafka.log.remote:type=RemoteLogManager, name=RemoteFetchThrottleTime -> The duration of time required at a given moment to bring the observed fetch rate within the allowed limit, by preventing further reads.

Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2024-02-27 Thread Jun Rao
Hi, Abhijeet, Thanks for the explanation. Makes sense to me now. Just a minor comment. Could you document the exact meaning of the following two metrics? For example, is the time accumulated? If so, is it from the start of the broker or over some window? kafka.log.remote:type=RemoteLogManager,

Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2024-02-27 Thread Abhijeet Kumar
Hi Jun, The existing quota system for consumers is designed to throttle the consumer if it exceeds the allowed fetch rate. The additional quota we want to add works on the broker level. If the broker-level remote read quota is being exceeded, we prevent additional reads from the remote storage

Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2024-02-26 Thread Jun Rao
Hi, Abhijeet, Sorry for the late reply. It seems that you haven't updated the KIP based on the discussion? One more comment. 11. Currently, we already have a quota system for both the producers and consumers. I can understand why we need an additional remote.log.manager.write.quota.default

Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2024-02-12 Thread Abhijeet Kumar
Comments inline On Wed, Dec 6, 2023 at 1:12 AM Jun Rao wrote: > Hi, Abhijeet, > > Thanks for the KIP. A few comments. > > 10. remote.log.manager.write.quota.default: > 10.1 For other configs, we > use replica.alter.log.dirs.io.max.bytes.per.second. To be consistent, > perhaps this can be sth

Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2024-02-12 Thread Abhijeet Kumar
Comments inline On Tue, Nov 28, 2023 at 3:50 PM Luke Chen wrote: > > Hi Abhijeet, > > Thanks for the KIP! > This is an important feature for tiered storage. > > Some comments: > 1. Will we introduce new metrics for this tiered storage quotas? > This is important because the admin can know the

Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2024-02-02 Thread Abhijeet Kumar
Yes, it is part of V1. I have added it now. I will follow up on the KIP comments in the next couple of days and try to close the review in the next few weeks. Regards. On Thu, Feb 1, 2024 at 7:40 PM Francois Visconte wrote: > > Hi, > > I see that the ticket has been left untouched since a

Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2024-02-01 Thread Francois Visconte
Hi, I see that the ticket has been left untouched since a while now. Should it be included in the tiered storage v1? We've observed that lacking a way to throttle uploads to tiered storage has a major impact on producers and consumers when tiered storage access recovers (starving disk

Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2023-12-05 Thread Jun Rao
Hi, Abhijeet, Thanks for the KIP. A few comments. 10. remote.log.manager.write.quota.default: 10.1 For other configs, we use replica.alter.log.dirs.io.max.bytes.per.second. To be consistent, perhaps this can be sth like remote.log.manager.write.max.bytes.per.second. 10.2 Could we list the new

Re: [DISCUSS] KIP-956: Tiered Storage Quotas

2023-11-28 Thread Luke Chen
Hi Abhijeet, Thanks for the KIP! This is an important feature for tiered storage. Some comments: 1. Will we introduce new metrics for this tiered storage quotas? This is important because the admin can know the throttling status by checking the metrics while the remote write/read are slow, like

[DISCUSS] KIP-956: Tiered Storage Quotas

2023-11-22 Thread Abhijeet Kumar
Hi All, I have created KIP-956 for defining read and write quota for tiered storage. https://cwiki.apache.org/confluence/display/KAFKA/KIP-956+Tiered+Storage+Quotas Feedback and suggestions are welcome. Regards, Abhijeet.