It seems nobody has any objections against the change. That's for the KIP improvement. I'll go ahead and merge the PR.
-Matthias On 8/21/18 2:44 PM, John Roesler wrote: > Hello again, all, > > I belatedly had a better idea for adding grace period to the Windows class > hierarchy (TimeWindows, UnlimitedWindows, JoinWindows). Instead of > providing the grace-setter in the abstract class and having to retract it > in UnlimitedWindows, I've made the getter abstract method in Windows and > only added setters to Time and Join windows. > > This should not only improve the ergonomics of grace period, but make the > whole class hierarchy more maintainable. > > See the PR for more details: https://github.com/apache/kafka/pull/5536 > > I've updated the KIP accordingly. Here's the diff: > https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=87295409&selectedPageVersions=11&selectedPageVersions=9 > > Please let me know if this changes your vote. > > Thanks, > -John > > On Mon, Aug 13, 2018 at 5:20 PM John Roesler <j...@confluent.io> wrote: > >> 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 >>>>>> >>>>> >>>> >>>> >
signature.asc
Description: OpenPGP digital signature
