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