Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2019-04-16 Thread John Roesler
Hi again, all, Casey Green pointed out that we overlooked the scala api when we added Suppress. He was kind enough to send https://github.com/apache/kafka/pull/6314 to correct this, and we also updated the KIP. Since it's essentially just copying the existing Java API over to Scala, we didn't

Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2018-11-16 Thread Bill Bejeck
Hi John, Thanks for the update, I'm +1 on changes and my +1 vote stands. -Bill On Fri, Nov 16, 2018 at 4:19 PM John Roesler wrote: > Hi all, sorry to do this again, but during review of the code to add the > metrics proposed in this KIP, the reviewers and I noticed some > inconsistencies and

Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2018-11-16 Thread John Roesler
Hi all, sorry to do this again, but during review of the code to add the metrics proposed in this KIP, the reviewers and I noticed some inconsistencies and drawbacks of the metrics I proposed in the KIP. Here's the diff:

Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2018-10-04 Thread John Roesler
Update: Here's a link to the documented eviction behavior: https://cwiki.apache.org/confluence/display/KAFKA/KIP-328%3A+Ability+to+suppress+updates+for+KTables#KIP-328:AbilitytosuppressupdatesforKTables-BufferEvictionBehavior(akaSuppressEmitBehavior) On Thu, Oct 4, 2018 at 11:12 AM John Roesler

Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2018-10-04 Thread John Roesler
Hello again, all, During review, we realized that there is a relationship between this (KIP-328) and KIP-372. KIP-372 proposed to allow naming *all* internal topics, and KIP-328 adds a new internal topic (the changelog for the suppression buffer). However, we didn't consider this relationship

Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2018-09-20 Thread John Roesler
Hello all, During review of https://github.com/apache/kafka/pull/5567 for KIP-328, the reviewers raised many good suggestions for the API. The basic design of the suppress operation remains the same, but the config object is (in my opinion) far more ergonomic with their suggestions. I have

Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2018-08-23 Thread Matthias J. Sax
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,

Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2018-08-21 Thread John Roesler
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

Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2018-08-13 Thread John Roesler
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=9=8 Basically: * the metrics named "*-event-*" are inconsistent

Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2018-08-07 Thread John Roesler
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 wrote: > +1 (binding) > > Thanks for the KIP. > > > -Matthias > > On 8/3/18 12:52 AM,

Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2018-08-06 Thread Matthias J. Sax
+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 wrote: > >> Yes, the addendum lgtm as well. Thanks! >> >> On Mon, Jul 30, 2018 at 3:34 PM, John Roesler wrote: >> >>> Another thing that came up

Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2018-08-03 Thread Damian Guy
Thanks John! +1 On Mon, 30 Jul 2018 at 23:58 Guozhang Wang wrote: > Yes, the addendum lgtm as well. Thanks! > > On Mon, Jul 30, 2018 at 3:34 PM, John Roesler wrote: > > > Another thing that came up after I started working on an implementation > is > > that in addition to deprecating

Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2018-07-30 Thread Guozhang Wang
Yes, the addendum lgtm as well. Thanks! On Mon, Jul 30, 2018 at 3:34 PM, John Roesler 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

Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2018-07-30 Thread John Roesler
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,

Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2018-07-30 Thread John Roesler
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

Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2018-07-30 Thread Guozhang Wang
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

Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2018-07-30 Thread Bill Bejeck
Thanks for the KIP! +1 -Bill On Mon, Jul 30, 2018 at 3:42 PM Ted Yu wrote: > +1 > > On Mon, Jul 30, 2018 at 11:46 AM John Roesler 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:

Re: [VOTE] KIP-328: Ability to suppress updates for KTables

2018-07-30 Thread Ted Yu
+1 On Mon, Jul 30, 2018 at 11:46 AM John Roesler 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

[VOTE] KIP-328: Ability to suppress updates for KTables

2018-07-30 Thread John Roesler
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