Re: [DISCUSS] KIP-272: Add API version tag to broker's RequestsPerSec metric

2018-03-26 Thread Allen Wang
Hello, I would like to bring this to vote in the next day or two. Let me know if you have further comments. Thanks, Allen On Thu, Mar 22, 2018 at 11:37 AM, Xavier Léauté wrote: > > > > This kind of change will be problematic to us as the total RequestsPerSec > > will be

Re: [DISCUSS] KIP-272: Add API version tag to broker's RequestsPerSec metric

2018-03-22 Thread Xavier Léauté
> > This kind of change will be problematic to us as the total RequestsPerSec > will be double counted in our metric system as we do automatic aggregation. > It has been a headache for us as we always have to do some special handling > when querying such metrics. > I agree with Allen, most of the

Re: [DISCUSS] KIP-272: Add API version tag to broker's RequestsPerSec metric

2018-03-22 Thread Allen Wang
225, the choice makes sense because the deprecated metric's > name is > >>> undesirable anyway and the new metric name is much better than the > prefixed > >>> metric name. Not the case for RequestsPerSec. It is hard for me to > come up > >>> with a

Re: [DISCUSS] KIP-272: Add API version tag to broker's RequestsPerSec metric

2018-03-22 Thread James Cheng
>> Thanks, >>> Allen >>> >>> >>> >>>> On Wed, Mar 21, 2018 at 2:01 AM, Ted Yu <yuzhih...@gmail.com> wrote: >>>> >>>> Creating new metric and deprecating existing one seems better from >>>> compatibility point o

Re: [DISCUSS] KIP-272: Add API version tag to broker's RequestsPerSec metric

2018-03-22 Thread James Cheng
ty point of view. >>> ---- Original message ----From: James Cheng < >> wushuja...@gmail.com> >>> Date: 3/21/18 1:39 AM (GMT-08:00) To: dev@kafka.apache.org Subject: >> Re: >>> [DISCUSS] KIP-272: Add API version tag to broker's RequestsPerSec m

Re: [DISCUSS] KIP-272: Add API version tag to broker's RequestsPerSec metric

2018-03-21 Thread Jeff Widman
r from > > compatibility point of view. > > Original message From: James Cheng < > wushuja...@gmail.com> > > Date: 3/21/18 1:39 AM (GMT-08:00) To: dev@kafka.apache.org Subject: > Re: > > [DISCUSS] KIP-272: Add API version tag to broker's Reques

Re: [DISCUSS] KIP-272: Add API version tag to broker's RequestsPerSec metric

2018-03-21 Thread Allen Wang
1/18 1:39 AM (GMT-08:00) To: dev@kafka.apache.org Subject: Re: > [DISCUSS] KIP-272: Add API version tag to broker's RequestsPerSec metric > Manikumar brings up a good point. This is a breaking change to the > existing metric. Do we want to break compatibility, or do we want to add a > new

Re: [DISCUSS] KIP-272: Add API version tag to broker's RequestsPerSec metric

2018-03-21 Thread Ted Yu
Creating new metric and deprecating existing one seems better from compatibility point of view. Original message From: James Cheng <wushuja...@gmail.com> Date: 3/21/18 1:39 AM (GMT-08:00) To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-272: Add API version tag to br

Re: [DISCUSS] KIP-272: Add API version tag to broker's RequestsPerSec metric

2018-03-21 Thread James Cheng
Manikumar brings up a good point. This is a breaking change to the existing metric. Do we want to break compatibility, or do we want to add a new metric and (optionally) deprecate the existing metric? For reference, in KIP-153 [1], we changed an existing metric without doing proper

Re: [DISCUSS] KIP-272: Add API version tag to broker's RequestsPerSec metric

2018-03-21 Thread Manikumar
Can we retain total RequestsPerSec metric and add new version tag metric? When monitoring with simple jconsole/jmx based tools, It is useful to have total metric to monitor request rate. Thanks, On Wed, Mar 21, 2018 at 11:01 AM, Gwen Shapira wrote: > I love this. Not much

Re: [DISCUSS] KIP-272: Add API version tag to broker's RequestsPerSec metric

2018-03-20 Thread Gwen Shapira
I love this. Not much to add - it is an elegant solution, clean implementation and it addresses a real need, especially during upgrades. On Tue, Mar 20, 2018 at 2:49 PM, Ted Yu wrote: > Thanks for the response. > > Assuming number of client versions is limited in a cluster,

Re: [DISCUSS] KIP-272: Add API version tag to broker's RequestsPerSec metric

2018-03-20 Thread Ted Yu
Thanks for the response. Assuming number of client versions is limited in a cluster, memory consumption is not a concern. Cheers On Tue, Mar 20, 2018 at 10:47 AM, Allen Wang wrote: > Hi Ted, > > The additional hash map is very small, possibly a few KB. Each request type

Re: [DISCUSS] KIP-272: Add API version tag to broker's RequestsPerSec metric

2018-03-20 Thread Allen Wang
Hi Ted, The additional hash map is very small, possibly a few KB. Each request type ("produce", "fetch", etc.) will have such a map which have a few entries depending on the client API versions the broker will encounter. So if broker encounters two client versions for "produce", there will be two

Re: [DISCUSS] KIP-272: Add API version tag to broker's RequestsPerSec metric

2018-03-19 Thread Ted Yu
bq. *additional hash lookup is needed when updating the metric to locate the metric * *Do you have estimate how much memory is needed for maintaining the hash map ?* *Thanks* On Mon, Mar 19, 2018 at 3:19 PM, Allen Wang wrote: > Hi all, > > I have created KIP-272: Add API

[DISCUSS] KIP-272: Add API version tag to broker's RequestsPerSec metric

2018-03-19 Thread Allen Wang
Hi all, I have created KIP-272: Add API version tag to broker's RequestsPerSec metric. Here is the link to the KIP: https://cwiki.apache.org/confluence/display/KAFKA/KIP-272%3A+Add+API+version+tag+to+broker%27s+RequestsPerSec+metric Looking forward to the discussion. Thanks, Allen