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 <wushuja...@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 <j...@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 <allenxw...@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 <yuzhih...@gmail.com> wrote: >>>> >>>> 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 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 <manikumar.re...@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 <g...@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 <yuzhih...@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 <allenxw...@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 <yuzhih...@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 <allenxw...@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> >>>>>> >>>> >>>> >>>
