You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Allen Wang <al...@gmail.com> on 2018/03/19 22:19:20 UTC

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

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

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

Posted by Allen Wang <al...@gmail.com>.
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é <xa...@confluent.io> wrote:

> >
> > 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 metrics are already setup to be aggregated
> across the various tags dimensions. We do the same on with our metrics, and
> any respectable metrics system would allow you to do the same.
>
> It sounds like backwards compatibility in this case is more subjective,
> since it depends largely on how you consume / aggregate those metrics, but
> in my opinion, aggregation should be the way to go.
>
> There is precedent with topic based metrics such as BytesInPerSec, which
> are currently duplicated, once at the broker level (no topic tag), and once
> at the topic level (tagged with the topic name) and it's easy to miss that
> fact. Without looking the source code it's very hard to notice those
> subtleties.
>
> As a result it can be very error-prone to filter out the right metrics for
> aggregation, so I would argue we should not repeat the same mistake unless
> we had some good reasons to do so in the first place.
>

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

Posted by Xavier Léauté <xa...@confluent.io>.
>
> 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 metrics are already setup to be aggregated
across the various tags dimensions. We do the same on with our metrics, and
any respectable metrics system would allow you to do the same.

It sounds like backwards compatibility in this case is more subjective,
since it depends largely on how you consume / aggregate those metrics, but
in my opinion, aggregation should be the way to go.

There is precedent with topic based metrics such as BytesInPerSec, which
are currently duplicated, once at the broker level (no topic tag), and once
at the topic level (tagged with the topic name) and it's easy to miss that
fact. Without looking the source code it's very hard to notice those
subtleties.

As a result it can be very error-prone to filter out the right metrics for
aggregation, so I would argue we should not repeat the same mistake unless
we had some good reasons to do so in the first place.

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

Posted by Allen Wang <al...@gmail.com>.
Hi James,

I updated the KIP to reflect the exact change to the metric as you
suggested.

> kafka.network:type=RequestMetrics,name=RequestsPerSec,request=Produce
>
> And the KIP is proposing to change it to:
>
> kafka.network:type=RequestMetrics,name=RequestsPerSec,request=
Produce,version=1.0.0
>
> Is is possible for the broker to have BOTH metrics? That way, we don’t
have to change the name.
>
> Would that make querying/aggregating too annoying (since a query for
name=RequestsPerSec and request=Produce would return BOTH metrics)?
>

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.

Thanks,
Allen

On Wed, Mar 21, 2018 at 11:55 PM, James Cheng <wu...@gmail.com> wrote:

> Regardless of what we decide, Allen, can you update the KIP and provide an
> concrete example of what an mbean will look like with the proposed change?
> Similar to what I said here:
>
> > An example of the metric we are thinking of changing is this:
> >
> > kafka.network:type=RequestMetrics,name=RequestsPerSec,request=Produce
> >
> > And the KIP is proposing to change it to:
> >
> > kafka.network:type=RequestMetrics,name=RequestsPerSec,request=
> Produce,version=1.0.0
>
>
> I think that would make the KIP easier to understand.
>
> Thanks,
> -James
>
> Sent from my iPhone
>
> > On Mar 21, 2018, at 11:49 PM, James Cheng <wu...@gmail.com> wrote:
> >
> > An example of the metric we are thinking of changing is this:
> >
> > kafka.network:type=RequestMetrics,name=RequestsPerSec,request=Produce
> >
> > And the KIP is proposing to change it to:
> >
> > kafka.network:type=RequestMetrics,name=RequestsPerSec,request=
> Produce,version=1.0.0
> >
> > Is is possible for the broker to have BOTH metrics? That way, we don’t
> have to change the name.
> >
> > Would that make querying/aggregating too annoying (since a query for
> name=RequestsPerSec and request=Produce would return BOTH metrics)?
> >
> > Also, it might be hard to query for “name=RequestsPerSec and
> request=Produce and version field NOT present”
> >
> > -James
> >
> > Sent from my iPhone
> >
> >> On Mar 21, 2018, at 10:17 PM, Jeff Widman <je...@jeffwidman.com> wrote:
> >>
> >> I agree with Allen.
> >>
> >> Go with the intuitive name, even if it means not deprecating. The
> impact of
> >> breakage here is small since it only breaks monitoring and the folks who
> >> watch their dashboards closely are the ones likely to read the release
> >> notes carefully and see this change.
> >>
> >>> On Wed, Mar 21, 2018, 3:24 PM Allen Wang <al...@gmail.com> wrote:
> >>>
> >>> I understand the impact to jmx based tools. But adding a new metric is
> >>> unnecessary for more advanced monitoring systems that can aggregate
> with or
> >>> without tags. Duplicating the metric (including the "request" tag) also
> >>> adds performance cost to the broker, especially for the metric that
> needs
> >>> to be updated for every request.
> >>>
> >>> For KIP-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 another intuitive name.
> >>>
> >>> Thanks,
> >>> Allen
> >>>
> >>>
> >>>
> >>>> On Wed, Mar 21, 2018 at 2:01 AM, Ted Yu <yu...@gmail.com> wrote:
> >>>>
> >>>> Creating new metric and deprecating existing one seems better from
> >>>> compatibility point of view.
> >>>> -------- Original message --------From: James Cheng <
> >>> wushujames@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
> 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 metric and (optionally) deprecate the existing metric?
> >>>>
> >>>> For reference, in KIP-153 [1], we changed an existing metric without
> >>> doing
> >>>> proper deprecation.
> >>>>
> >>>> However, in KIP-225, we noticed that that we maybe shouldn't have done
> >>>> that. For KIP-225 [2], we instead decided to create a new metric, and
> >>>> deprecate (but not remove) the old one.
> >>>>
> >>>> -James
> >>>>
> >>>> [1] https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >>>> 153%3A+Include+only+client+traffic+in+BytesOutPerSec+metric
> >>>>
> >>>> [2] https://cwiki.apache.org/confluence/pages/viewpage.
> >>>> action?pageId=74686649
> >>>>
> >>>>
> >>>>> On Mar 21, 2018, at 12:14 AM, Manikumar <ma...@gmail.com>
> >>>> wrote:
> >>>>>
> >>>>> 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 <gw...@confluent.io>
> >>>> wrote:
> >>>>>
> >>>>>> 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 <yu...@gmail.com>
> wrote:
> >>>>>>>
> >>>>>>> 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 <allenxwang@gmail.com
> >
> >>>>>> wrote:
> >>>>>>>
> >>>>>>>> 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
> >>>>>>>> entries in the map for "produce" requests mapping from version to
> >>>>>> meter.
> >>>>>>> Of
> >>>>>>>> course, hash map always have additional memory overhead.
> >>>>>>>>
> >>>>>>>> Thanks,
> >>>>>>>> Allen
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On Mon, Mar 19, 2018 at 3:49 PM, Ted Yu <yu...@gmail.com>
> >>> wrote:
> >>>>>>>>
> >>>>>>>>> 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 <
> allenxwang@gmail.com>
> >>>>>>>> wrote:
> >>>>>>>>>
> >>>>>>>>>> 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
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> --
> >>>>>> *Gwen Shapira*
> >>>>>> Product Manager | Confluent
> >>>>>> 650.450.2760 | @gwenshap
> >>>>>> Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
> >>>>>> <http://www.confluent.io/blog>
> >>>>>>
> >>>>
> >>>>
> >>>
>

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

Posted by James Cheng <wu...@gmail.com>.
Regardless of what we decide, Allen, can you update the KIP and provide an concrete example of what an mbean will look like with the proposed change? Similar to what I said here:

> An example of the metric we are thinking of changing is this:
> 
> kafka.network:type=RequestMetrics,name=RequestsPerSec,request=Produce
> 
> And the KIP is proposing to change it to:
> 
> kafka.network:type=RequestMetrics,name=RequestsPerSec,request=Produce,version=1.0.0


I think that would make the KIP easier to understand.

Thanks,
-James

Sent from my iPhone

> On Mar 21, 2018, at 11:49 PM, James Cheng <wu...@gmail.com> wrote:
> 
> An example of the metric we are thinking of changing is this:
> 
> kafka.network:type=RequestMetrics,name=RequestsPerSec,request=Produce
> 
> And the KIP is proposing to change it to:
> 
> kafka.network:type=RequestMetrics,name=RequestsPerSec,request=Produce,version=1.0.0
> 
> Is is possible for the broker to have BOTH metrics? That way, we don’t have to change the name.
> 
> Would that make querying/aggregating too annoying (since a query for name=RequestsPerSec and request=Produce would return BOTH metrics)? 
> 
> Also, it might be hard to query for “name=RequestsPerSec and request=Produce and version field NOT present”
> 
> -James
> 
> Sent from my iPhone
> 
>> On Mar 21, 2018, at 10:17 PM, Jeff Widman <je...@jeffwidman.com> wrote:
>> 
>> I agree with Allen.
>> 
>> Go with the intuitive name, even if it means not deprecating. The impact of
>> breakage here is small since it only breaks monitoring and the folks who
>> watch their dashboards closely are the ones likely to read the release
>> notes carefully and see this change.
>> 
>>> On Wed, Mar 21, 2018, 3:24 PM Allen Wang <al...@gmail.com> wrote:
>>> 
>>> I understand the impact to jmx based tools. But adding a new metric is
>>> unnecessary for more advanced monitoring systems that can aggregate with or
>>> without tags. Duplicating the metric (including the "request" tag) also
>>> adds performance cost to the broker, especially for the metric that needs
>>> to be updated for every request.
>>> 
>>> For KIP-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 another intuitive name.
>>> 
>>> Thanks,
>>> Allen
>>> 
>>> 
>>> 
>>>> On Wed, Mar 21, 2018 at 2:01 AM, Ted Yu <yu...@gmail.com> wrote:
>>>> 
>>>> Creating new metric and deprecating existing one seems better from
>>>> compatibility point of view.
>>>> -------- Original message --------From: James Cheng <
>>> wushujames@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 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 metric and (optionally) deprecate the existing metric?
>>>> 
>>>> For reference, in KIP-153 [1], we changed an existing metric without
>>> doing
>>>> proper deprecation.
>>>> 
>>>> However, in KIP-225, we noticed that that we maybe shouldn't have done
>>>> that. For KIP-225 [2], we instead decided to create a new metric, and
>>>> deprecate (but not remove) the old one.
>>>> 
>>>> -James
>>>> 
>>>> [1] https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>>>> 153%3A+Include+only+client+traffic+in+BytesOutPerSec+metric
>>>> 
>>>> [2] https://cwiki.apache.org/confluence/pages/viewpage.
>>>> action?pageId=74686649
>>>> 
>>>> 
>>>>> On Mar 21, 2018, at 12:14 AM, Manikumar <ma...@gmail.com>
>>>> wrote:
>>>>> 
>>>>> 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 <gw...@confluent.io>
>>>> wrote:
>>>>> 
>>>>>> 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 <yu...@gmail.com> wrote:
>>>>>>> 
>>>>>>> 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 <al...@gmail.com>
>>>>>> wrote:
>>>>>>> 
>>>>>>>> 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
>>>>>>>> entries in the map for "produce" requests mapping from version to
>>>>>> meter.
>>>>>>> Of
>>>>>>>> course, hash map always have additional memory overhead.
>>>>>>>> 
>>>>>>>> Thanks,
>>>>>>>> Allen
>>>>>>>> 
>>>>>>>> 
>>>>>>>> On Mon, Mar 19, 2018 at 3:49 PM, Ted Yu <yu...@gmail.com>
>>> wrote:
>>>>>>>> 
>>>>>>>>> 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 <al...@gmail.com>
>>>>>>>> wrote:
>>>>>>>>> 
>>>>>>>>>> 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
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> --
>>>>>> *Gwen Shapira*
>>>>>> Product Manager | Confluent
>>>>>> 650.450.2760 | @gwenshap
>>>>>> Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
>>>>>> <http://www.confluent.io/blog>
>>>>>> 
>>>> 
>>>> 
>>> 

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

Posted by James Cheng <wu...@gmail.com>.
An example of the metric we are thinking of changing is this:

kafka.network:type=RequestMetrics,name=RequestsPerSec,request=Produce

And the KIP is proposing to change it to:

kafka.network:type=RequestMetrics,name=RequestsPerSec,request=Produce,version=1.0.0

Is is possible for the broker to have BOTH metrics? That way, we don’t have to change the name.

Would that make querying/aggregating too annoying (since a query for name=RequestsPerSec and request=Produce would return BOTH metrics)? 

Also, it might be hard to query for “name=RequestsPerSec and request=Produce and version field NOT present”

-James

Sent from my iPhone

> On Mar 21, 2018, at 10:17 PM, Jeff Widman <je...@jeffwidman.com> wrote:
> 
> I agree with Allen.
> 
> Go with the intuitive name, even if it means not deprecating. The impact of
> breakage here is small since it only breaks monitoring and the folks who
> watch their dashboards closely are the ones likely to read the release
> notes carefully and see this change.
> 
>> On Wed, Mar 21, 2018, 3:24 PM Allen Wang <al...@gmail.com> wrote:
>> 
>> I understand the impact to jmx based tools. But adding a new metric is
>> unnecessary for more advanced monitoring systems that can aggregate with or
>> without tags. Duplicating the metric (including the "request" tag) also
>> adds performance cost to the broker, especially for the metric that needs
>> to be updated for every request.
>> 
>> For KIP-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 another intuitive name.
>> 
>> Thanks,
>> Allen
>> 
>> 
>> 
>>> On Wed, Mar 21, 2018 at 2:01 AM, Ted Yu <yu...@gmail.com> wrote:
>>> 
>>> Creating new metric and deprecating existing one seems better from
>>> compatibility point of view.
>>> -------- Original message --------From: James Cheng <
>> wushujames@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 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 metric and (optionally) deprecate the existing metric?
>>> 
>>> For reference, in KIP-153 [1], we changed an existing metric without
>> doing
>>> proper deprecation.
>>> 
>>> However, in KIP-225, we noticed that that we maybe shouldn't have done
>>> that. For KIP-225 [2], we instead decided to create a new metric, and
>>> deprecate (but not remove) the old one.
>>> 
>>> -James
>>> 
>>> [1] https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>>> 153%3A+Include+only+client+traffic+in+BytesOutPerSec+metric
>>> 
>>> [2] https://cwiki.apache.org/confluence/pages/viewpage.
>>> action?pageId=74686649
>>> 
>>> 
>>>> On Mar 21, 2018, at 12:14 AM, Manikumar <ma...@gmail.com>
>>> wrote:
>>>> 
>>>> 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 <gw...@confluent.io>
>>> wrote:
>>>> 
>>>>> 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 <yu...@gmail.com> wrote:
>>>>>> 
>>>>>> 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 <al...@gmail.com>
>>>>> wrote:
>>>>>> 
>>>>>>> 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
>>>>>>> entries in the map for "produce" requests mapping from version to
>>>>> meter.
>>>>>> Of
>>>>>>> course, hash map always have additional memory overhead.
>>>>>>> 
>>>>>>> Thanks,
>>>>>>> Allen
>>>>>>> 
>>>>>>> 
>>>>>>> On Mon, Mar 19, 2018 at 3:49 PM, Ted Yu <yu...@gmail.com>
>> wrote:
>>>>>>> 
>>>>>>>> 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 <al...@gmail.com>
>>>>>>> wrote:
>>>>>>>> 
>>>>>>>>> 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
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> --
>>>>> *Gwen Shapira*
>>>>> Product Manager | Confluent
>>>>> 650.450.2760 | @gwenshap
>>>>> Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
>>>>> <http://www.confluent.io/blog>
>>>>> 
>>> 
>>> 
>> 

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

Posted by Jeff Widman <je...@jeffwidman.com>.
I agree with Allen.

Go with the intuitive name, even if it means not deprecating. The impact of
breakage here is small since it only breaks monitoring and the folks who
watch their dashboards closely are the ones likely to read the release
notes carefully and see this change.

On Wed, Mar 21, 2018, 3:24 PM Allen Wang <al...@gmail.com> wrote:

> I understand the impact to jmx based tools. But adding a new metric is
> unnecessary for more advanced monitoring systems that can aggregate with or
> without tags. Duplicating the metric (including the "request" tag) also
> adds performance cost to the broker, especially for the metric that needs
> to be updated for every request.
>
> For KIP-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 another intuitive name.
>
> Thanks,
> Allen
>
>
>
> On Wed, Mar 21, 2018 at 2:01 AM, Ted Yu <yu...@gmail.com> wrote:
>
> > Creating new metric and deprecating existing one seems better from
> > compatibility point of view.
> > -------- Original message --------From: James Cheng <
> wushujames@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 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 metric and (optionally) deprecate the existing metric?
> >
> > For reference, in KIP-153 [1], we changed an existing metric without
> doing
> > proper deprecation.
> >
> > However, in KIP-225, we noticed that that we maybe shouldn't have done
> > that. For KIP-225 [2], we instead decided to create a new metric, and
> > deprecate (but not remove) the old one.
> >
> > -James
> >
> > [1] https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 153%3A+Include+only+client+traffic+in+BytesOutPerSec+metric
> >
> > [2] https://cwiki.apache.org/confluence/pages/viewpage.
> > action?pageId=74686649
> >
> >
> > > On Mar 21, 2018, at 12:14 AM, Manikumar <ma...@gmail.com>
> > wrote:
> > >
> > > 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 <gw...@confluent.io>
> > wrote:
> > >
> > >> 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 <yu...@gmail.com> wrote:
> > >>
> > >>> 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 <al...@gmail.com>
> > >> wrote:
> > >>>
> > >>>> 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
> > >>>> entries in the map for "produce" requests mapping from version to
> > >> meter.
> > >>> Of
> > >>>> course, hash map always have additional memory overhead.
> > >>>>
> > >>>> Thanks,
> > >>>> Allen
> > >>>>
> > >>>>
> > >>>> On Mon, Mar 19, 2018 at 3:49 PM, Ted Yu <yu...@gmail.com>
> wrote:
> > >>>>
> > >>>>> 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 <al...@gmail.com>
> > >>>> wrote:
> > >>>>>
> > >>>>>> 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
> > >>>>>>
> > >>>>>
> > >>>>
> > >>>
> > >>
> > >>
> > >>
> > >> --
> > >> *Gwen Shapira*
> > >> Product Manager | Confluent
> > >> 650.450.2760 | @gwenshap
> > >> Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
> > >> <http://www.confluent.io/blog>
> > >>
> >
> >
>

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

Posted by Allen Wang <al...@gmail.com>.
I understand the impact to jmx based tools. But adding a new metric is
unnecessary for more advanced monitoring systems that can aggregate with or
without tags. Duplicating the metric (including the "request" tag) also
adds performance cost to the broker, especially for the metric that needs
to be updated for every request.

For KIP-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 another intuitive name.

Thanks,
Allen



On Wed, Mar 21, 2018 at 2:01 AM, Ted Yu <yu...@gmail.com> wrote:

> Creating new metric and deprecating existing one seems better from
> compatibility point of view.
> -------- Original message --------From: James Cheng <wu...@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 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 metric and (optionally) deprecate the existing metric?
>
> For reference, in KIP-153 [1], we changed an existing metric without doing
> proper deprecation.
>
> However, in KIP-225, we noticed that that we maybe shouldn't have done
> that. For KIP-225 [2], we instead decided to create a new metric, and
> deprecate (but not remove) the old one.
>
> -James
>
> [1] https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 153%3A+Include+only+client+traffic+in+BytesOutPerSec+metric
>
> [2] https://cwiki.apache.org/confluence/pages/viewpage.
> action?pageId=74686649
>
>
> > On Mar 21, 2018, at 12:14 AM, Manikumar <ma...@gmail.com>
> wrote:
> >
> > 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 <gw...@confluent.io>
> wrote:
> >
> >> 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 <yu...@gmail.com> wrote:
> >>
> >>> 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 <al...@gmail.com>
> >> wrote:
> >>>
> >>>> 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
> >>>> entries in the map for "produce" requests mapping from version to
> >> meter.
> >>> Of
> >>>> course, hash map always have additional memory overhead.
> >>>>
> >>>> Thanks,
> >>>> Allen
> >>>>
> >>>>
> >>>> On Mon, Mar 19, 2018 at 3:49 PM, Ted Yu <yu...@gmail.com> wrote:
> >>>>
> >>>>> 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 <al...@gmail.com>
> >>>> wrote:
> >>>>>
> >>>>>> 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
> >>>>>>
> >>>>>
> >>>>
> >>>
> >>
> >>
> >>
> >> --
> >> *Gwen Shapira*
> >> Product Manager | Confluent
> >> 650.450.2760 | @gwenshap
> >> Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
> >> <http://www.confluent.io/blog>
> >>
>
>

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

Posted by Ted Yu <yu...@gmail.com>.
Creating new metric and deprecating existing one seems better from compatibility point of view.
-------- Original message --------From: James Cheng <wu...@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 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 metric and (optionally) deprecate the existing metric?

For reference, in KIP-153 [1], we changed an existing metric without doing proper deprecation.

However, in KIP-225, we noticed that that we maybe shouldn't have done that. For KIP-225 [2], we instead decided to create a new metric, and deprecate (but not remove) the old one.

-James

[1] https://cwiki.apache.org/confluence/display/KAFKA/KIP-153%3A+Include+only+client+traffic+in+BytesOutPerSec+metric

[2] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=74686649


> On Mar 21, 2018, at 12:14 AM, Manikumar <ma...@gmail.com> wrote:
> 
> 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 <gw...@confluent.io> wrote:
> 
>> 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 <yu...@gmail.com> wrote:
>> 
>>> 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 <al...@gmail.com>
>> wrote:
>>> 
>>>> 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
>>>> entries in the map for "produce" requests mapping from version to
>> meter.
>>> Of
>>>> course, hash map always have additional memory overhead.
>>>> 
>>>> Thanks,
>>>> Allen
>>>> 
>>>> 
>>>> On Mon, Mar 19, 2018 at 3:49 PM, Ted Yu <yu...@gmail.com> wrote:
>>>> 
>>>>> 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 <al...@gmail.com>
>>>> wrote:
>>>>> 
>>>>>> 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
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>> 
>> 
>> --
>> *Gwen Shapira*
>> Product Manager | Confluent
>> 650.450.2760 | @gwenshap
>> Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
>> <http://www.confluent.io/blog>
>> 


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

Posted by James Cheng <wu...@gmail.com>.
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 deprecation.

However, in KIP-225, we noticed that that we maybe shouldn't have done that. For KIP-225 [2], we instead decided to create a new metric, and deprecate (but not remove) the old one.

-James

[1] https://cwiki.apache.org/confluence/display/KAFKA/KIP-153%3A+Include+only+client+traffic+in+BytesOutPerSec+metric

[2] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=74686649


> On Mar 21, 2018, at 12:14 AM, Manikumar <ma...@gmail.com> wrote:
> 
> 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 <gw...@confluent.io> wrote:
> 
>> 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 <yu...@gmail.com> wrote:
>> 
>>> 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 <al...@gmail.com>
>> wrote:
>>> 
>>>> 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
>>>> entries in the map for "produce" requests mapping from version to
>> meter.
>>> Of
>>>> course, hash map always have additional memory overhead.
>>>> 
>>>> Thanks,
>>>> Allen
>>>> 
>>>> 
>>>> On Mon, Mar 19, 2018 at 3:49 PM, Ted Yu <yu...@gmail.com> wrote:
>>>> 
>>>>> 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 <al...@gmail.com>
>>>> wrote:
>>>>> 
>>>>>> 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
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>> 
>> 
>> --
>> *Gwen Shapira*
>> Product Manager | Confluent
>> 650.450.2760 | @gwenshap
>> Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
>> <http://www.confluent.io/blog>
>> 


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

Posted by Manikumar <ma...@gmail.com>.
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 <gw...@confluent.io> wrote:

> 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 <yu...@gmail.com> wrote:
>
> > 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 <al...@gmail.com>
> wrote:
> >
> > > 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
> > > entries in the map for "produce" requests mapping from version to
> meter.
> > Of
> > > course, hash map always have additional memory overhead.
> > >
> > > Thanks,
> > > Allen
> > >
> > >
> > > On Mon, Mar 19, 2018 at 3:49 PM, Ted Yu <yu...@gmail.com> wrote:
> > >
> > > > 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 <al...@gmail.com>
> > > wrote:
> > > >
> > > > > 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
> > > > >
> > > >
> > >
> >
>
>
>
> --
> *Gwen Shapira*
> Product Manager | Confluent
> 650.450.2760 | @gwenshap
> Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
> <http://www.confluent.io/blog>
>

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

Posted by Gwen Shapira <gw...@confluent.io>.
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 <yu...@gmail.com> wrote:

> 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 <al...@gmail.com> wrote:
>
> > 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
> > entries in the map for "produce" requests mapping from version to meter.
> Of
> > course, hash map always have additional memory overhead.
> >
> > Thanks,
> > Allen
> >
> >
> > On Mon, Mar 19, 2018 at 3:49 PM, Ted Yu <yu...@gmail.com> wrote:
> >
> > > 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 <al...@gmail.com>
> > wrote:
> > >
> > > > 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
> > > >
> > >
> >
>



-- 
*Gwen Shapira*
Product Manager | Confluent
650.450.2760 | @gwenshap
Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
<http://www.confluent.io/blog>

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

Posted by Ted Yu <yu...@gmail.com>.
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 <al...@gmail.com> wrote:

> 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
> entries in the map for "produce" requests mapping from version to meter. Of
> course, hash map always have additional memory overhead.
>
> Thanks,
> Allen
>
>
> On Mon, Mar 19, 2018 at 3:49 PM, Ted Yu <yu...@gmail.com> wrote:
>
> > 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 <al...@gmail.com>
> wrote:
> >
> > > 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
> > >
> >
>

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

Posted by Allen Wang <al...@gmail.com>.
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
entries in the map for "produce" requests mapping from version to meter. Of
course, hash map always have additional memory overhead.

Thanks,
Allen


On Mon, Mar 19, 2018 at 3:49 PM, Ted Yu <yu...@gmail.com> wrote:

> 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 <al...@gmail.com> wrote:
>
> > 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
> >
>

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

Posted by Ted Yu <yu...@gmail.com>.
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 <al...@gmail.com> wrote:

> 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
>