Thanks, Sophie! I think this makes perfect sense. It will be much more intuitive to use the metric for the stated motivation this way. I’d be in favor of the proposal after this update.
Thanks again for taking this on, -John On Fri, May 15, 2020, at 17:07, Sophie Blee-Goldman wrote: > Hi all, > > I'd like to clarify/modify one aspect of this KIP, which is to record the > staleness > at the *completion* of the record's processing by the operator/task in > question, > rather than on the intake. The task-level metrics will be recorded at the > sink > node instead of at the source, and the operator-level metrics will be > recorded > at the end of the operation. > > The stated purpose and intended usefulness of this KIP is to give users a > way > to gauge roughly how long it takes for a record to be reflected in the > "results", > whether these results are being read from an output topic or through IQ. To > take the IQ example, the results of a record are obviously not visible until > *after* that node has finished processing it. The staleness, while still > potentially > useful as it can impact the *way* a record is processed in a stateful and > time- > dependent operator, is not part of the problem this KIP specifically set out > to solve. > > In light of this I think it's appropriate to revert the name change back to > include > latency, since "staleness" as described above only makes sense when > measuring > relative to the arrival of a record at a task/node. I'd propose to save the > term > "staleness" for that particular meaning, and adopt Matthias's suggestion of > "record-e2e-latency" for this. > > Thanks for hanging in there all. Please let me know if you have any > concerns about > this change! > > Sophie > > On Fri, May 15, 2020 at 2:25 PM Matthias J. Sax <mj...@apache.org> wrote: > > > I am also happy with max/min/99/90. And I buy your naming argument about > > staleness vs latency. > > > > > > -Matthias > > > > On 5/15/20 12:24 PM, Boyang Chen wrote: > > > Hey Sophie, > > > > > > 90/99/min/max make sense to me. > > > > > > On Fri, May 15, 2020 at 12:20 PM Sophie Blee-Goldman < > > sop...@confluent.io> > > > wrote: > > > > > >> @Matthias > > >> Regarding tracking the 50th percentile, I'll refer you to the 4:53 mark > > of > > >> the video > > >> *you* linked: https://youtu.be/lJ8ydIuPFeU?t=293 > > >> > > >> And that's what he says about the 95th percentile! Imagine what he would > > >> say about > > >> the 50th :P > > >> > > >> But seriously, since we can't seem to agree that the mean or 50th > > >> percentile is actually > > >> useful I'm inclined to resurrect my original proposal, neither. But I > > think > > >> that's a good > > >> argument against the 75th, which I admittedly chose somewhat > > arbitrarily as > > >> an > > >> intermediate between the 50th and the higher percentiles. How about: > > >> > > >> -max > > >> -p99 > > >> -p90 > > >> -min > > >> > > >> with p50/mean still up for debate if anyone feels strongly for either of > > >> them. > > >> > > >> Regarding the name, I've already flip-flopped on this so I'm definitely > > >> still open to > > >> further arguments. But the reason for changing it from > > end-to-end-latency > > >> (which > > >> is similar to what you propose) is that this metric technically reflects > > >> how old (ie how "stale") > > >> the record is when it's *received* by the operator, not when it's > > processed > > >> by the operator. > > >> It seemed like there was the potential for confusion that > > >> "end-to-end-latency" might > > >> represent the latency from the event creation to the time the processor > > is > > >> done > > >> processing it. > > >> > > >> @John > > >> I'd rather err on the side of "not-enough" metrics as we can always add > > >> this to the > > >> stateless metrics later on. If we decide to measure the time at every > > node > > >> and don't > > >> find any evidence of a serious performance impact, and users indicate > > they > > >> would > > >> like to see this metric at all nodes, then we can easily start reporting > > >> them as well. > > >> WDYT? > > >> > > >> That said, sink nodes seem like a reasonable exception to the rule. > > >> Obviously users > > >> should be able to detect the time when the record reaches the output > > topic > > >> but that > > >> still leaves a gap in understanding how long the production latency was. > > >> This mirrors > > >> the consumption latency that is exposed by the task-level metrics, which > > >> are measured > > >> at the source node. For good symmetry what if we actually expose both > > the > > >> source > > >> and sink latency at the task-level? ie report both sets of statistical > > >> measurements with > > >> the additional tag -source/-sink > > >> > > >> @Bill > > >> Thanks for the comment about regarding the min! I hadn't considered that > > >> and it's > > >> quite useful to think about how and what is useful from a users point of > > >> view. > > >> > > >> Regarding your second. point, I'm inclined to leave that as an > > >> implementation detail > > >> but my take would be that the user should be allowed to control the > > record > > >> timestamp > > >> used for this with the timestamp extractor. My impression is that users > > may > > >> often embed > > >> the actual event time in the payload for whatever reason, and this > > >> represents the "true" > > >> timestamp as far as the Streams topology is concerned. > > >> > > >> > > >> On Fri, May 15, 2020 at 11:05 AM Bill Bejeck <bbej...@gmail.com> wrote: > > >> > > >>> Thanks for the KIP, Sophie, this will be a useful metric to add. > > >>> > > >>> Regarding tracking min, I think it could be valuable for users to > > >> discern > > >>> which part of their topologies are more efficient since this is a > > >>> task-level metric. I realize everyone seems to be on board with > > >> including > > >>> min anyway, but I wanted to add my 2 cents on this topic should we > > decide > > >>> to revisit adding min or not. > > >>> > > >>> I do have a question regarding the calculation of staleness. > > >>> Is there going to be a consideration for timestamp extractors? Users > > >> could > > >>> prefer to use a timestamp embedded in the payload, and it could skew > > the > > >>> measurements. > > >>> I was wondering if we should specify in the KIP if setting the arrival > > >> time > > >>> is always going to come from the record timestamp, or is this an > > >>> implementation detail we can cover in the PR? > > >>> > > >>> Thanks! > > >>> Bill > > >>> > > >>> On Fri, May 15, 2020 at 1:11 AM Matthias J. Sax <mj...@apache.org> > > >> wrote: > > >>> > > >>>> Thanks for the KIP Sophie. > > >>>> > > >>>> I think it's not useful to record the avg/mean; it sensitive to > > >>>> outliers. We should rather track the median (50th percentile). > > >>>> > > >>>> Not sure if tracking min is useful, but I am also ok to track it. > > >>>> > > >>>> However, I find it odd to track 75th percentile. Standard measures > > >> would > > >>>> the 90th or 95th -- I guess we don't need both, so maybe picking 90th > > >>>> might be more useful? > > >>>> > > >>>> About the name: "staleness" wound really odd, and if fact the metric > > >>>> does capture "latency" so we should call it "latency". I understand > > the > > >>>> issue that we already have a latency metric. So maybe we could call it > > >>>> `record-e2e-latency-*` ? > > >>>> > > >>>> While I agree that we should include out-or-order data (the KIP should > > >>>> talk about `out-of-order` data, not `late` data; data is only `late` > > if > > >>>> it's out-of-order and if it's dropped), I don't really understand why > > >>>> the new metric would help to configure grace period or retention time? > > >>>> As you mention in the KIP, both are define as max difference of > > >>>> `event-time - stream-time` and thus the new metric that takes > > >>>> system-/wallclock-time into account does not seem to help at all. > > >>>> > > >>>> > > >>>> Btw: there is a great talk about "How NOT to Measure Latency" by Gil > > >>>> Tene: https://www.youtube.com/watch?v=lJ8ydIuPFeU > > >>>> > > >>>> > > >>>> -Matthias > > >>>> > > >>>> > > >>>> On 5/14/20 7:17 PM, John Roesler wrote: > > >>>>> Hi Sophie, > > >>>>> > > >>>>> It seems like there would still be plenty of use cases for recording > > >>>>> this metric at all processors and not just stateful ones, but I'm > > >> happy > > >>>>> to suspend my arguments for now. Since you're proposing to keep > > >>>>> them at the processor-node level, it will be seamless later to add > > >>>>> in the stateless processors if we want. As a wise man once said, > > >>>>> "Adding is always easier than removing." > > >>>>> > > >>>>> Regarding the time measurement, it's an implementation detail > > >>>>> we don't need to consider in the KIP. Nevertheless, I'd greatly > > >>>>> prefer to measure the system time again when recording the > > >>>>> metric. I don't think we've seen any evidence that proves this > > >>>>> would harm performance, and the amount of inaccuracy using > > >>>>> the cached system time could incur is actually substantial. But, > > >>>>> if you want to just "not mention this" in the KIP, we can defer to > > >>>>> the actual PR discussion, at which time we're in a better position > > >>>>> to use benchmarks, etc., to make the call. > > >>>>> > > >>>>> Along the lines of the measurement accuracy discussion, one > > >>>>> minor thought I had is that maybe we should consider measuring > > >>>>> the task staleness metric at the sink, rather than the source, so > > >> that > > >>>>> it includes the processing latency of the task itself, not just the > > >>>> latency > > >>>>> of everything up to, but not including, the task (which seems > > >> confusing > > >>>>> for users). I guess this could also be an implementation detail, > > >>> though. > > >>>>> > > >>>>> Thanks for the update, > > >>>>> -John > > >>>>> > > >>>>> On Thu, May 14, 2020, at 13:31, Sophie Blee-Goldman wrote: > > >>>>>> Hey all, > > >>>>>> > > >>>>>> After discussing with Bruno I'd like to propose a small amendment, > > >>>>>> which is to record the processor-node-level metrics only for > > >>> *stateful* > > >>>>>> *operators*. They would still be considered a "processor-node-level" > > >>>>>> metric and not a "state-store-level" metric as the staleness is > > >> still > > >>>>>> a property of the node rather than of the state itself. However, it > > >>>> seems > > >>>>>> that this information is primarily useful for stateful operators > > >> that > > >>>> might > > >>>>>> be exposing state via IQ or otherwise dependent on the record time > > >>>>>> unlike a stateless operator. > > >>>>>> > > >>>>>> It's worth calling out that recent performance improvements to the > > >>>> metrics > > >>>>>> framework mean that we no longer fetch the system time at the > > >> operator > > >>>>>> level, but only once per task. In other words the system time is not > > >>>> updated > > >>>>>> between each process as a record flows through the subtopology, so > > >>>>>> debugging the processor-level latency via the stateleness will not > > >> be > > >>>>>> possible.Note that this doesn't mean the operator-level metrics are > > >>> not > > >>>>>> *useful* relative to the task-level metric. Upstream caching and/or > > >>>>>> suppression > > >>>>>> can still cause a record's staleness at some downstream stateful > > >>>> operator > > >>>>>> to deviate from the task-level staleness (recorded at the source > > >>> node). > > >>>>>> > > >>>>>> Please let me know if you have any concerns about this change. The > > >>>>>> KIP has been updated with the new proposal > > >>>>>> > > >>>>>> On Thu, May 14, 2020 at 3:04 AM Bruno Cadonna <br...@confluent.io> > > >>>> wrote: > > >>>>>> > > >>>>>>> Hi Sophie, > > >>>>>>> > > >>>>>>> Thank you for the KIP. > > >>>>>>> > > >>>>>>> The KIP looks good to me. > > >>>>>>> > > >>>>>>> 50th percentile: > > >>>>>>> I think we do not need it now. If we need it, we can add it. Here > > >> the > > >>>>>>> old truism applies: Adding is always easier than removing. > > >>>>>>> > > >>>>>>> processor-node-level metrics: > > >>>>>>> I think it is good to have the staleness metrics also on > > >>>>>>> processor-node-level. If we do not want to record them on all > > >>>>>>> processor nodes, you could restrict the recording to stateful > > >>>>>>> processor-nodes, since those are the ones that would benefit most > > >>> from > > >>>>>>> the staleness metrics. > > >>>>>>> > > >>>>>>> Best, > > >>>>>>> Bruno > > >>>>>>> > > >>>>>>> On Thu, May 14, 2020 at 4:15 AM Sophie Blee-Goldman < > > >>>> sop...@confluent.io> > > >>>>>>> wrote: > > >>>>>>>> > > >>>>>>>> Yeah, the specific reason was just to align with the current > > >>> metrics. > > >>>>>>>> > > >>>>>>>> Is it better to conform than to be right? History has a lot to say > > >>> on > > >>>>>>> that > > >>>>>>>> matter > > >>>>>>>> but I'm not sure how much of it applies to the fine details of > > >>> metrics > > >>>>>>>> naming :P > > >>>>>>>> > > >>>>>>>> More seriously, I figured if people are looking at this metric > > >>> they're > > >>>>>>>> likely to > > >>>>>>>> be looking at all the others. Then naming this one "-mean" would > > >>>> probably > > >>>>>>>> lead some to conclude that the "-avg" suffix in the other metrics > > >>> has > > >>>> a > > >>>>>>>> different meaning. > > >>>>>>>> > > >>>>>>>> As for the percentiles, I actually like p99 (and p75) better. I'll > > >>>> swap > > >>>>>>>> that out > > >>>>>>>> > > >>>>>>>> On Wed, May 13, 2020 at 7:07 PM John Roesler <vvcep...@apache.org > > >>> > > >>>>>>> wrote: > > >>>>>>>> > > >>>>>>>>> Thanks Sophie, > > >>>>>>>>> > > >>>>>>>>> I hope this isn't too nit-picky, but is there a reason to choose > > >>>> "avg" > > >>>>>>>>> instead > > >>>>>>>>> of "mean"? Maybe this is too paranoid, and I might be > > >> oversensitive > > >>>>>>> because > > >>>>>>>>> of the mistake I just made earlier, but it strikes me that "avg" > > >> is > > >>>>>>>>> actually > > >>>>>>>>> ambiguous, as it refers to a family of statistics, whereas "mean" > > >>> is > > >>>>>>>>> specific. > > >>>>>>>>> I see other Kafka metrics with "avg", but none with "mean"; was > > >>> that > > >>>>>>> the > > >>>>>>>>> reason? If so, I'm +1. > > >>>>>>>>> > > >>>>>>>>> Regarding the names of the percentile, I actually couldn't find > > >>> _any_ > > >>>>>>> other > > >>>>>>>>> metrics that use percentile. Was there a reason to choose "99th" > > >> as > > >>>>>>> opposed > > >>>>>>>>> to "p99" or any other scheme? This is not a criticism, I'm just > > >>>>>>> primarily > > >>>>>>>>> asking > > >>>>>>>>> for consistency's sake. > > >>>>>>>>> > > >>>>>>>>> Thanks again, > > >>>>>>>>> -John > > >>>>>>>>> > > >>>>>>>>> On Wed, May 13, 2020, at 19:19, Sophie Blee-Goldman wrote: > > >>>>>>>>>> Alright, I can get behind adding the min metric for the sake of > > >>>>>>> pretty > > >>>>>>>>>> graphs > > >>>>>>>>>> (and trivial computation). > > >>>>>>>>>> > > >>>>>>>>>> I'm still on the fence regarding the mean (or 50th percentile) > > >>> but I > > >>>>>>> can > > >>>>>>>>> see > > >>>>>>>>>> how users might expect it and find it a bit disorienting not to > > >>>>>>> have. So > > >>>>>>>>> the > > >>>>>>>>>> updated proposed metrics are > > >>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>>> - record-staleness-max [ms] > > >>>>>>>>>> - record-staleness-99th [ms] *(99th percentile)* > > >>>>>>>>>> - record-staleness-75th [ms] *(75th percentile)* > > >>>>>>>>>> - record-staleness-avg [ms] *(mean)* > > >>>>>>>>>> - record-staleness-min [ms] > > >>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>>> On Wed, May 13, 2020 at 4:42 PM John Roesler < > > >> vvcep...@apache.org > > >>>> > > >>>>>>>>> wrote: > > >>>>>>>>>> > > >>>>>>>>>>> Oh boy, I never miss an opportunity to embarrass myself. I > > >> guess > > >>>>>>> the > > >>>>>>>>> mean > > >>>>>>>>>>> seems more interesting to me than the median, but neither are > > >> as > > >>>>>>>>>>> interesting as the higher percentiles (99th and max). > > >>>>>>>>>>> > > >>>>>>>>>>> Min isn’t really important for any SLAs, but it does round out > > >>> the > > >>>>>>>>> mental > > >>>>>>>>>>> picture of the distribution. I’ve always graphed min along with > > >>> the > > >>>>>>>>> other > > >>>>>>>>>>> metrics to help me understand how fast the system can be, which > > >>>>>>> helps > > >>>>>>>>> in > > >>>>>>>>>>> optimization decisions. It’s also a relatively inexpensive > > >> metric > > >>>>>>> to > > >>>>>>>>>>> compute, so it might be nice to just throw it in. > > >>>>>>>>>>> > > >>>>>>>>>>> On Wed, May 13, 2020, at 18:18, Sophie Blee-Goldman wrote: > > >>>>>>>>>>>> G1: > > >>>>>>>>>>>> I was considering it as the "end-to-end latency *up* to the > > >>>>>>> specific > > >>>>>>>>>>> task" > > >>>>>>>>>>>> but > > >>>>>>>>>>>> I'm happy with "record-staleness" if that drives the point > > >> home > > >>>>>>>>> better. > > >>>>>>>>>>> So > > >>>>>>>>>>>> it's the > > >>>>>>>>>>>> "staleness of the record when it is received by that task" -- > > >>>>>>> will > > >>>>>>>>> update > > >>>>>>>>>>>> the KIP > > >>>>>>>>>>>> > > >>>>>>>>>>>> B1/J: > > >>>>>>>>>>>> I'm struggling to imagine a case where the min would actually > > >> be > > >>>>>>>>> useful, > > >>>>>>>>>>>> rather than > > >>>>>>>>>>>> just intellectually interesting. I don't feel strongly that we > > >>>>>>>>> shouldn't > > >>>>>>>>>>>> add it, but that's > > >>>>>>>>>>>> why I didn't include it from the start. Can you enlighten me > > >>>>>>> with an > > >>>>>>>>>>>> example? > > >>>>>>>>>>>> > > >>>>>>>>>>>> I was also vaguely concerned about the overhead of adding > > >>>>>>> multiple > > >>>>>>>>>>>> percentile > > >>>>>>>>>>>> metrics. Do we have any data to indicate what kind of > > >>> performance > > >>>>>>>>> hit we > > >>>>>>>>>>>> take on > > >>>>>>>>>>>> metrics computation? > > >>>>>>>>>>>> > > >>>>>>>>>>>> Also, not to be too pedantic but the 50th percentile would be > > >>> the > > >>>>>>>>> median > > >>>>>>>>>>>> not the > > >>>>>>>>>>>> mean. Would you propose to add the mean *and* the 50th > > >>>>>>> percentile, or > > >>>>>>>>>>> just > > >>>>>>>>>>>> one > > >>>>>>>>>>>> of the two? > > >>>>>>>>>>>> > > >>>>>>>>>>>> Thanks all! > > >>>>>>>>>>>> Sophie > > >>>>>>>>>>>> > > >>>>>>>>>>>> On Wed, May 13, 2020 at 3:34 PM John Roesler < > > >>>>>>> vvcep...@apache.org> > > >>>>>>>>>>> wrote: > > >>>>>>>>>>>> > > >>>>>>>>>>>>> Hello all, and thanks for the KIP, Sophie, > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> Just some comments on the discussion so far: > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> B2/G1: > > >>>>>>>>>>>>> In principle, it shouldn't matter whether we report "spans" > > >> or > > >>>>>>>>>>>>> "end-to-end" latency. But in practice, some of the spans are > > >>>>>>> pretty > > >>>>>>>>>>>>> difficult to really measure (like time spent waiting in the > > >>>>>>>>> topics, or > > >>>>>>>>>>>>> time from the event happening to the ETL producer choosing to > > >>>>>>> send > > >>>>>>>>> it, > > >>>>>>>>>>>>> or time spent in send/receive buffers, etc., etc. > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> In other words, it's practically easier to compute spans by > > >>>>>>>>> subtracting > > >>>>>>>>>>>>> e2e latencies than it is to compute e2e latencies by adding > > >>>>>>> spans. > > >>>>>>>>> You > > >>>>>>>>>>>>> can even just consider that the span computation from e2e > > >>>>>>> always > > >>>>>>>>> just > > >>>>>>>>>>>>> involves subtracting two numbers, whereas computing e2e > > >> latency > > >>>>>>>>> from > > >>>>>>>>>>>>> spans involves adding _all_ the spans leading up to the end > > >> you > > >>>>>>>>> care > > >>>>>>>>>>> about. > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> It seems like people really prefer to have spans when they > > >> are > > >>>>>>>>>>> debugging > > >>>>>>>>>>>>> latency problems, whereas e2e latency is a more general > > >>>>>>> measurement > > >>>>>>>>>>>>> that basically every person/application cares about and > > >> should > > >>>>>>> be > > >>>>>>>>>>>>> monitoring. > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> Altogether, it really seem to provide more value to more > > >>>>>>> people if > > >>>>>>>>> we > > >>>>>>>>>>>>> report > > >>>>>>>>>>>>> e2e latencies. Regarding "record-staleness" as a name, I > > >> think > > >>>>>>> I > > >>>>>>>>> have > > >>>>>>>>>>> no > > >>>>>>>>>>>>> preference, I'd defer to other peoples' intuition. > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> G2: > > >>>>>>>>>>>>> I think the processor-node metric is nice, since the inside > > >> of > > >>>>>>> a > > >>>>>>>>> task > > >>>>>>>>>>> can > > >>>>>>>>>>>>> introduce a significant amount of latency in some cases. > > >> Plus, > > >>>>>>>>> it's a > > >>>>>>>>>>> more > > >>>>>>>>>>>>> direct measurement, if you really wanted to know (for the > > >>>>>>> purposes > > >>>>>>>>> of > > >>>>>>>>>>> IQ > > >>>>>>>>>>>>> or something) how long it takes source events to "show up" at > > >>>>>>> the > > >>>>>>>>>>> store. > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> I think actually recording it at every processor could be > > >>>>>>>>> expensive, > > >>>>>>>>>>> but we > > >>>>>>>>>>>>> already record a bunch of metrics at the node level. > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> B1: > > >>>>>>>>>>>>> I think 50% could be reasonable to record also. Even if it's > > >> a > > >>>>>>> poor > > >>>>>>>>>>> metric > > >>>>>>>>>>>>> for operational purposes, a lot of people might expect to see > > >>>>>>>>> "mean". > > >>>>>>>>>>>>> Actually, > > >>>>>>>>>>>>> I was surprised not to see "min". Is there a reason to leave > > >> it > > >>>>>>>>> off? > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> I might suggest: > > >>>>>>>>>>>>> min, mean (50th), 75th, 99th, max > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> B3: > > >>>>>>>>>>>>> I agree we should include late records (though not the ones > > >> we > > >>>>>>>>> drop). > > >>>>>>>>>>>>> It may be spiky, but only when there are legitimately some > > >>>>>>> records > > >>>>>>>>>>> with a > > >>>>>>>>>>>>> high end-to-end latency, which is the whole point of these > > >>>>>>> metrics. > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> That's it! I don't think I have any other feedback, other > > >> than > > >>>>>>> a > > >>>>>>>>>>> request to > > >>>>>>>>>>>>> also report "min". > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> Thanks, > > >>>>>>>>>>>>> -John > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> On Wed, May 13, 2020, at 16:58, Guozhang Wang wrote: > > >>>>>>>>>>>>>> Thanks Sophie for the KIP, a few quick thoughts: > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> 1) The end-to-end latency includes both the processing > > >>>>>>> latency > > >>>>>>>>> of the > > >>>>>>>>>>>>> task > > >>>>>>>>>>>>>> and the latency spent sitting in intermediate topics. I have > > >>>>>>> a > > >>>>>>>>>>> similar > > >>>>>>>>>>>>>> feeling as Boyang mentioned above that the latency metric of > > >>>>>>> a > > >>>>>>>>> task A > > >>>>>>>>>>>>>> actually measures the latency of the sub-topology up-to but > > >>>>>>> not > > >>>>>>>>>>> including > > >>>>>>>>>>>>>> the processing of A, which is a bit weird. > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> Maybe the my feeling comes from the name "latency" itself, > > >>>>>>> since > > >>>>>>>>>>> today we > > >>>>>>>>>>>>>> already have several "latency" metrics already which are > > >>>>>>>>> measuring > > >>>>>>>>>>>>> elapsed > > >>>>>>>>>>>>>> system-time for processing a record / etc, while here we are > > >>>>>>>>>>> comparing > > >>>>>>>>>>>>> the > > >>>>>>>>>>>>>> system wallclock time with the record timestamp. > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> Maybe we can consider renaming it as "record-staleness" > > >>>>>>> (note we > > >>>>>>>>>>> already > > >>>>>>>>>>>>>> have a "record-lateness" metric), in which case recording at > > >>>>>>> the > > >>>>>>>>>>>>>> system-time before we start processing the record sounds > > >> more > > >>>>>>>>>>> natural. > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> 2) With that in mind, I'm wondering if the > > >>>>>>> processor-node-level > > >>>>>>>>> DEBUG > > >>>>>>>>>>>>>> metric is worth to add, given that we already have a > > >>>>>>> task-level > > >>>>>>>>>>>>> processing > > >>>>>>>>>>>>>> latency metric. Basically, a specific node's e2e latency is > > >>>>>>>>> similar > > >>>>>>>>>>> to > > >>>>>>>>>>>>> the > > >>>>>>>>>>>>>> task-level e2e latency + task-level processing latency. > > >>>>>>>>> Personally I > > >>>>>>>>>>>>> think > > >>>>>>>>>>>>>> having a task-level record-staleness metric is sufficient. > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> Guozhang > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> On Wed, May 13, 2020 at 11:46 AM Sophie Blee-Goldman < > > >>>>>>>>>>>>> sop...@confluent.io> > > >>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> 1. I felt that 50% was not a particularly useful gauge for > > >>>>>>> this > > >>>>>>>>>>>>> specific > > >>>>>>>>>>>>>>> metric, as > > >>>>>>>>>>>>>>> it's presumably most useful at putting an *upper *bound on > > >>>>>>> the > > >>>>>>>>>>> latency > > >>>>>>>>>>>>> you > > >>>>>>>>>>>>>>> can > > >>>>>>>>>>>>>>> reasonably expect to see. I chose percentiles that would > > >>>>>>>>> hopefully > > >>>>>>>>>>>>> give a > > >>>>>>>>>>>>>>> good > > >>>>>>>>>>>>>>> sense of what *most* records will experience, and what > > >>>>>>> *close > > >>>>>>>>> to > > >>>>>>>>>>> all* > > >>>>>>>>>>>>>>> records > > >>>>>>>>>>>>>>> will. > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> However I'm not married to these specific numbers and > > >>>>>>> could be > > >>>>>>>>>>>>> convinced. > > >>>>>>>>>>>>>>> Would be especially interested in hearing from users on > > >>>>>>> this. > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> 2. I'm inclined to not include the "hop-to-hop latency" in > > >>>>>>>>> this KIP > > >>>>>>>>>>>>> since > > >>>>>>>>>>>>>>> users > > >>>>>>>>>>>>>>> can always compute it themselves by subtracting the > > >>>>>>> previous > > >>>>>>>>> node's > > >>>>>>>>>>>>>>> end-to-end latency. I guess we could do it either way since > > >>>>>>>>> you can > > >>>>>>>>>>>>> always > > >>>>>>>>>>>>>>> compute one from the other, but I think the end-to-end > > >>>>>>> latency > > >>>>>>>>>>> feels > > >>>>>>>>>>>>> more > > >>>>>>>>>>>>>>> valuable as it's main motivation is not to debug > > >>>>>>> bottlenecks > > >>>>>>>>> in the > > >>>>>>>>>>>>>>> topology but > > >>>>>>>>>>>>>>> to give users a sense of how long it takes arecord to be > > >>>>>>>>> reflected > > >>>>>>>>>>> in > > >>>>>>>>>>>>>>> certain parts > > >>>>>>>>>>>>>>> of the topology. For example this might be useful for users > > >>>>>>>>> who are > > >>>>>>>>>>>>>>> wondering > > >>>>>>>>>>>>>>> roughly when a record that was just produced will be > > >>>>>>> included > > >>>>>>>>> in > > >>>>>>>>>>> their > > >>>>>>>>>>>>> IQ > > >>>>>>>>>>>>>>> results. > > >>>>>>>>>>>>>>> Debugging is just a nice side effect -- but maybe I didn't > > >>>>>>> make > > >>>>>>>>>>> that > > >>>>>>>>>>>>> clear > > >>>>>>>>>>>>>>> enough > > >>>>>>>>>>>>>>> in the KIP's motivation. > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> 3. Good question, I should address this in the KIP. The > > >>>>>>> short > > >>>>>>>>>>> answer is > > >>>>>>>>>>>>>>> "yes", > > >>>>>>>>>>>>>>> we will include late records. I added a paragraph to the > > >>>>>>> end > > >>>>>>>>> of the > > >>>>>>>>>>>>>>> Proposed > > >>>>>>>>>>>>>>> Changes section explaining the reasoning here, please let > > >>>>>>> me > > >>>>>>>>> know > > >>>>>>>>>>> if > > >>>>>>>>>>>>> you > > >>>>>>>>>>>>>>> have > > >>>>>>>>>>>>>>> any concerns. > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> 4. Assuming you're referring to the existing metric > > >>>>>>>>>>> "process-latency", > > >>>>>>>>>>>>> that > > >>>>>>>>>>>>>>> metric > > >>>>>>>>>>>>>>> reflects the time for the literal Node#process method to > > >>>>>>> run > > >>>>>>>>>>> whereas > > >>>>>>>>>>>>> this > > >>>>>>>>>>>>>>> metric > > >>>>>>>>>>>>>>> would always be measured relative to the event timestamp. > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> That said, the naming collision there is pretty confusing > > >>>>>>> so > > >>>>>>>>> I've > > >>>>>>>>>>>>> renamed > > >>>>>>>>>>>>>>> the > > >>>>>>>>>>>>>>> metrics in this KIP to "end-to-end-latency" which I feel > > >>>>>>> better > > >>>>>>>>>>>>> reflects > > >>>>>>>>>>>>>>> the nature > > >>>>>>>>>>>>>>> of the metric anyway. > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> Thanks for the feedback! > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> On Wed, May 13, 2020 at 10:21 AM Boyang Chen < > > >>>>>>>>>>>>> reluctanthero...@gmail.com> > > >>>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> Thanks for the KIP Sophie. Getting the E2E latency is > > >>>>>>>>> important > > >>>>>>>>>>> for > > >>>>>>>>>>>>>>>> understanding the bottleneck of the application. > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> A couple of questions and ideas: > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> 1. Could you clarify the rational of picking 75, 99 and > > >>>>>>> max > > >>>>>>>>>>>>> percentiles? > > >>>>>>>>>>>>>>>> Normally I see cases where we use 50, 90 percentile as > > >>>>>>> well > > >>>>>>>>> in > > >>>>>>>>>>>>> production > > >>>>>>>>>>>>>>>> systems. > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> 2. The current latency being computed is cumulative, I.E > > >>>>>>> if a > > >>>>>>>>>>> record > > >>>>>>>>>>>>> goes > > >>>>>>>>>>>>>>>> through A -> B -> C, then P(C) = T(B->C) + P(B) = > > >>>>>>> T(B->C) + > > >>>>>>>>>>> T(A->B) + > > >>>>>>>>>>>>>>> T(A) > > >>>>>>>>>>>>>>>> and so on, where P() represents the captured latency, > > >>>>>>> and T() > > >>>>>>>>>>>>> represents > > >>>>>>>>>>>>>>>> the time for transiting the records between two nodes, > > >>>>>>>>> including > > >>>>>>>>>>>>>>> processing > > >>>>>>>>>>>>>>>> time. For monitoring purpose, maybe having T(B->C) and > > >>>>>>>>> T(A->B) > > >>>>>>>>>>> are > > >>>>>>>>>>>>> more > > >>>>>>>>>>>>>>>> natural to view as "hop-to-hop latency", otherwise if > > >>>>>>> there > > >>>>>>>>> is a > > >>>>>>>>>>>>> spike in > > >>>>>>>>>>>>>>>> T(A->B), both P(B) and P(C) are affected in the same > > >>>>>>> time. > > >>>>>>>>> In the > > >>>>>>>>>>>>> same > > >>>>>>>>>>>>>>>> spirit, the E2E latency is meaningful only when the > > >>>>>>> record > > >>>>>>>>> exits > > >>>>>>>>>>>>> from the > > >>>>>>>>>>>>>>>> sink as this marks the whole time this record spent > > >>>>>>> inside > > >>>>>>>>> the > > >>>>>>>>>>>>> funnel. Do > > >>>>>>>>>>>>>>>> you think we could have separate treatment for sink > > >>>>>>> nodes and > > >>>>>>>>>>> other > > >>>>>>>>>>>>>>>> nodes, so that other nodes only count the time receiving > > >>>>>>> the > > >>>>>>>>>>> record > > >>>>>>>>>>>>> from > > >>>>>>>>>>>>>>>> last hop? I'm not proposing a solution here, just want to > > >>>>>>>>> discuss > > >>>>>>>>>>>>> this > > >>>>>>>>>>>>>>>> alternative to see if it is reasonable. > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> 3. As we are going to monitor late arrival records as > > >>>>>>> well, > > >>>>>>>>> they > > >>>>>>>>>>>>> would > > >>>>>>>>>>>>>>>> create some really spiky graphs when the out-of-order > > >>>>>>>>> records are > > >>>>>>>>>>>>>>>> interleaving with on time records. Should we also supply > > >>>>>>> a > > >>>>>>>>> smooth > > >>>>>>>>>>>>> version > > >>>>>>>>>>>>>>>> of the latency metrics, or user should just take care of > > >>>>>>> it > > >>>>>>>>> by > > >>>>>>>>>>>>> themself? > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> 4. Regarding this new metrics, we haven't discussed its > > >>>>>>>>> relation > > >>>>>>>>>>>>> with our > > >>>>>>>>>>>>>>>> existing processing latency metrics, could you add some > > >>>>>>>>> context > > >>>>>>>>>>> on > > >>>>>>>>>>>>>>>> comparison and a simple `when to use which` tutorial for > > >>>>>>> the > > >>>>>>>>>>> best? > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> Boyang > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> On Tue, May 12, 2020 at 7:28 PM Sophie Blee-Goldman < > > >>>>>>>>>>>>> sop...@confluent.io > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> Hey all, > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> I'd like to kick off discussion on KIP-613 which aims > > >>>>>>> to > > >>>>>>>>> add > > >>>>>>>>>>>>> end-to-end > > >>>>>>>>>>>>>>>>> latency metrics to Streams. Please take a look: > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>> > > >>>>>>>>>>> > > >>>>>>>>> > > >>>>>>> > > >>>> > > >>> > > >> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-613%3A+Add+end-to-end+latency+metrics+to+Streams > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> Cheers, > > >>>>>>>>>>>>>>>>> Sophie > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> -- > > >>>>>>>>>>>>>> -- Guozhang > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>> > > >>>>>>>>>>>> > > >>>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>> > > >>>>>>> > > >>>>>> > > >>>> > > >>>> > > >>> > > >> > > > > > > > >
