Hey all, I just wanted to let you know that a few small issues surfaced during implementation and review. I've updated the KIP. Here's the diff: https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=87295409&selectedPageVersions=9&selectedPageVersions=8
Basically: * the metrics named "*-event-*" are inconsistent with existing nomenclature, and will be "*-record-*" instead (late records instead of late events, for example) * the apis taking and returning Duration will use long millis instead. We do want to transition to Duration in the future, but we shouldn't do it piecemeal. Thanks, -John On Tue, Aug 7, 2018 at 12:07 PM John Roesler <j...@confluent.io> wrote: > Thanks everyone, KIP-328 has passed with 3 binding votes (Guozhang, > Damian, and Matthias) and 3 non-binding (Ted, Bill, and me). > > Thanks for your time, > -John > > On Mon, Aug 6, 2018 at 6:35 PM Matthias J. Sax <matth...@confluent.io> > wrote: > >> +1 (binding) >> >> Thanks for the KIP. >> >> >> -Matthias >> >> On 8/3/18 12:52 AM, Damian Guy wrote: >> > Thanks John! +1 >> > >> > On Mon, 30 Jul 2018 at 23:58 Guozhang Wang <wangg...@gmail.com> wrote: >> > >> >> Yes, the addendum lgtm as well. Thanks! >> >> >> >> On Mon, Jul 30, 2018 at 3:34 PM, John Roesler <j...@confluent.io> >> wrote: >> >> >> >>> Another thing that came up after I started working on an >> implementation >> >> is >> >>> that in addition to deprecating "retention" from the Windows >> interface, >> >> we >> >>> also need to deprecate "segmentInterval", for the same reasons. I >> simply >> >>> overlooked it previously. I've updated the KIP accordingly. >> >>> >> >>> Hopefully, this doesn't change anyone's vote. >> >>> >> >>> Thanks, >> >>> -John >> >>> >> >>> On Mon, Jul 30, 2018 at 5:31 PM John Roesler <j...@confluent.io> >> wrote: >> >>> >> >>>> Thanks Guozhang, >> >>>> >> >>>> Thanks for that catch. to clarify, currently, events are "late" only >> >> when >> >>>> they are older than the retention period. Currently, we detect this >> in >> >>> the >> >>>> processor and record it as a "skipped-record". We then do not attempt >> >> to >> >>>> store the event in the window store. If a user provided a >> >> pre-configured >> >>>> window store with a retention period smaller than the one they >> specify >> >>> via >> >>>> Windows#until, the segmented store will drop the update with no >> metric >> >>> and >> >>>> record a debug-level log. >> >>>> >> >>>> With KIP-328, with the introduction of "grace period" and moving >> >>> retention >> >>>> fully into the state store, we need to have metrics for both "late >> >>> events" >> >>>> (new records older than the grace period) and "expired window events" >> >>> (new >> >>>> records for windows that are no longer retained in the state store). >> I >> >>>> already proposed metrics for the late events, and I've just updated >> the >> >>> KIP >> >>>> with metrics for the expired window events. I also updated the KIP to >> >>> make >> >>>> it clear that neither late nor expired events will count as >> >>>> "skipped-records" any more. >> >>>> >> >>>> -John >> >>>> >> >>>> On Mon, Jul 30, 2018 at 4:22 PM Guozhang Wang <wangg...@gmail.com> >> >>> wrote: >> >>>> >> >>>>> Hi John, >> >>>>> >> >>>>> Thanks for the updated KIP, +1 from me, and one minor suggestion: >> >>>>> >> >>>>> Following your suggestion of the differentiation of >> `skipped-records` >> >>> v.s. >> >>>>> `late-event-drop`, we should probably consider moving the scenarios >> >>> where >> >>>>> records got ignored due the window not being available any more in >> >>>>> windowed >> >>>>> aggregation operators from the `skipped-records` metrics recording >> to >> >>> the >> >>>>> `late-event-drop` metrics recording. >> >>>>> >> >>>>> >> >>>>> >> >>>>> Guozhang >> >>>>> >> >>>>> >> >>>>> On Mon, Jul 30, 2018 at 1:36 PM, Bill Bejeck <bbej...@gmail.com> >> >> wrote: >> >>>>> >> >>>>>> Thanks for the KIP! >> >>>>>> >> >>>>>> +1 >> >>>>>> >> >>>>>> -Bill >> >>>>>> >> >>>>>> On Mon, Jul 30, 2018 at 3:42 PM Ted Yu <yuzhih...@gmail.com> >> wrote: >> >>>>>> >> >>>>>>> +1 >> >>>>>>> >> >>>>>>> On Mon, Jul 30, 2018 at 11:46 AM John Roesler <j...@confluent.io> >> >>>>> wrote: >> >>>>>>> >> >>>>>>>> Hello devs, >> >>>>>>>> >> >>>>>>>> The discussion of KIP-328 has gone some time with no new >> >> comments, >> >>>>> so I >> >>>>>>> am >> >>>>>>>> calling for a vote! >> >>>>>>>> >> >>>>>>>> Here's the KIP: https://cwiki.apache.org/confluence/x/sQU0BQ >> >>>>>>>> >> >>>>>>>> The basic idea is to provide: >> >>>>>>>> * more usable control over update rate (vs the current state >> >> store >> >>>>>>> caches) >> >>>>>>>> * the final-result-for-windowed-computations feature which >> >>> several >> >>>>>> people >> >>>>>>>> have requested >> >>>>>>>> >> >>>>>>>> Thanks, >> >>>>>>>> -John >> >>>>>>>> >> >>>>>>> >> >>>>>> >> >>>>> >> >>>>> >> >>>>> >> >>>>> -- >> >>>>> -- Guozhang >> >>>>> >> >>>> >> >>> >> >> >> >> >> >> >> >> -- >> >> -- Guozhang >> >> >> > >> >>
