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 >
