Thanks Guozhang -- I'll definitely make sure we have this benchmarked with an eye for any regressions before it makes it into a release. That said, if it's any comfort, we use the cached system time and record only count and sum type metrics, so unlike the *-rate *metrics for example we don't have to make any calls to the system clock which I know we have found to impact performance when investigating past regressions.
On Wed, Jun 1, 2022 at 11:07 AM Guozhang Wang <wangg...@gmail.com> wrote: > Thanks Sophie, that makes sense. Also I agree that since we are adding it > at finest granularity for consumed metrics, it's better to have symmetry > and add produced metrics at processor-node level as well. > > Regarding the benchmarks, I think that would be critical to have before we > add in the next release, since we propose to make this INFO level, meaning > it would be on by default, and collecting per-record is a critical path. > > > Guozhang > > On Wed, Jun 1, 2022 at 8:04 AM Sophie Blee-Goldman > <sop...@confluent.io.invalid> wrote: > > > I just want to send out a small update -- I decided to include the "- > > *consumed*" metrics in the KIP alongside the > > *"-produced"* metrics after all, for reasons I address in the paragraph I > > added at the end of the motivation section. > > Please let me know if you have any questions or concerns > > > > Cheers, > > Sophie > > > > On Wed, Jun 1, 2022 at 1:38 AM Sophie Blee-Goldman <sop...@confluent.io> > > wrote: > > > > > Just a quick question: for filling the gap of sub-topology > visibilities, > > >> would task-level produced metrics be sufficient? > > > > > > > > > If I understand your question correctly, you're asking whether we could > > > just report at the task/subtopology > > > level since we mainly want the bytes/throughput produced by the > > > subtopology itself? > > > > > > Note that since we're only reporting this metric at the sink nodes, > it's > > > basically the same as a task-level metric > > > for any subtopology that has only one sink and only scales with the > > number > > > of output topics. If you're concerned > > > about a potential performance impact I would say that (a) making these > > > task-level doesn't buy us much, if anything, > > > and (b) we can always revisit this if our benchmarks do indeed reveal a > > > regression but for now let's not over- > > > optimize too much :) > > > > > > Also, the general philosophy behind the KAfka Streams metrics thus far > > > has been to report at the finest granularity > > > and allow users to roll them up into whatever scope they want to > > aggregate > > > over. This KIP adopts the same approach. > > > > > > On Tue, May 31, 2022 at 12:02 PM Guozhang Wang <wangg...@gmail.com> > > wrote: > > > > > >> Hi Sophie, > > >> > > >> Just a quick question: for filling the gap of sub-topology > visibilities, > > >> would task-level produced metrics be sufficient? > > >> > > >> On Mon, May 30, 2022 at 10:59 AM Bill Bejeck <bbej...@gmail.com> > wrote: > > >> > > >> > Thanks for the KIP Sophie. > > >> > > > >> > I'm in favor of this change as well. I don't have any comments in > > >> > addition to the ones already expressed. > > >> > > > >> > -Bill > > >> > > > >> > On Mon, May 30, 2022 at 4:55 AM Sagar <sagarmeansoc...@gmail.com> > > >> wrote: > > >> > > > >> > > Hi Sophie, > > >> > > > > >> > > A very minor comment but you might want to remove this KIP > template > > >> > related > > >> > > information from the top of the KIP: > > >> > > > > >> > > *This page is meant as a template for writing a KIP > > >> > > < > > >> > > > > >> > > > >> > > > https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals > > >> > > >. > > >> > > To create a KIP choose Tools->Copy on this page and modify with > your > > >> > > content and replace the heading with the next KIP number and a > > >> > description > > >> > > of your issue. Replace anything in italics with your own > > description.* > > >> > > > > >> > > > > >> > > Thanks! > > >> > > Sagar. > > >> > > > > >> > > On Mon, May 30, 2022 at 1:04 PM Sophie Blee-Goldman > > >> > > <sop...@confluent.io.invalid> wrote: > > >> > > > > >> > > > > > > >> > > > > Why does the title of the KIP talk about task-level metrics, > but > > >> the > > >> > > > > specified metrics are on processor-level? > > >> > > > > > >> > > > > > >> > > > Ah, my mistake -- it should indeed say "processor-level > metrics". > > >> > Thanks > > >> > > > for the catch Bruno, the title has been fixed. > > >> > > > > > >> > > > Since there don't seem to be any concerns I'll proceed with > > kicking > > >> off > > >> > > the > > >> > > > vote. Thanks all! > > >> > > > > > >> > > > On Mon, May 30, 2022 at 12:01 AM Bruno Cadonna < > > cado...@apache.org> > > >> > > wrote: > > >> > > > > > >> > > > > Thanks for the KIP, Sophie! > > >> > > > > > > >> > > > > I am also in favor of this KIP! > > >> > > > > > > >> > > > > I have one minor question: > > >> > > > > > > >> > > > > Why does the title of the KIP talk about task-level metrics, > but > > >> the > > >> > > > > specified metrics are on processor-level? > > >> > > > > > > >> > > > > For the rest, I am +1. > > >> > > > > > > >> > > > > Best, > > >> > > > > Bruno > > >> > > > > > > >> > > > > On 29.05.22 00:20, John Roesler wrote: > > >> > > > > > Thanks for the well motivated and documented KIP, Sophie! > I’m > > in > > >> > > favor > > >> > > > > of this change. > > >> > > > > > > > >> > > > > > -John > > >> > > > > > > > >> > > > > > On Sat, May 28, 2022, at 06:42, Sophie Blee-Goldman wrote: > > >> > > > > >> Hey all, > > >> > > > > >> > > >> > > > > >> I'd like to propose a very small KIP to add two metrics > that > > >> will > > >> > > help > > >> > > > > fill > > >> > > > > >> a gap in the derivable produced and consumed metrics. > Please > > >> take > > >> > a > > >> > > > look > > >> > > > > >> and reply here with any questions or concerns. > > >> > > > > >> > > >> > > > > >> KIP-846: Task-level Streams metrics for bytes/records > > Produced > > >> > > > > >> < > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=211886093 > > >> > > > > > > > >> > > > > >> > > >> > > > > >> Given the small nature of this I'm going to call for a vote > > >> soon, > > >> > > but > > >> > > > > >> please don't hesitate to raise anything you feel should be > > >> > discussed > > >> > > > in > > >> > > > > >> more detail first. > > >> > > > > >> > > >> > > > > >> Thanks! > > >> > > > > >> Sophie > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > > >> -- > > >> -- Guozhang > > >> > > > > > > > > -- > -- Guozhang >
