I would prefer to not make the grace-period a mandatory argument and keep the API as-is. I understand the issue of backward compatibility, but I would still argue that we should just change the default grace period to 0 in the 3.0 release. It's a major release and thus it seems to be fine. To prepare for this change, we could start to log a WARN message, if a user does not set the grace period explicitly for now.
Just my 2 ct. Thoughts? -Matthias On 4/19/20 7:40 AM, John Roesler wrote: > Oh, man, that’s a good idea. > > I can propose to deprecate (not remove) the existing ‘of’ factory method and > add one with a mandatory grace period. Not sure why I didn’t think of that > before. Probably too caught up in looking for something “smart”. > > Thanks! > John > > On Sun, Apr 19, 2020, at 02:27, Liam Clarke wrote: >> Hi John, >> >> I can't really think of a way to make it more obvious without breaking >> backwards compatibility - e.g., obvious easy fix is that grace period is a >> mandatory arg to TimeWindows, but that would definitely break compatibility. >> >> Cheers, >> >> Liam Clarke-Hutchinson >> >> On Thu, Apr 16, 2020 at 1:59 AM John Roesler <vvcep...@apache.org> wrote: >> >>> Boom, you got it, Liam! Nice debugging work. >>> >>> This is a pretty big bummer, but I had to do it that way for >>> compatibility. I added a log message to try and help reduce the risk, but >>> it’s still kind of a trap. >>> >>> I’d like to do a KIP at some point to consider changing the default grace >>> period, but haven’t done it because it’s not clear what the default should >>> be. >>> >>> Please let me know if you have any ideas! >>> Thanks, >>> -John >>> >>> >>> On Tue, Apr 14, 2020, at 23:44, Liam Clarke wrote: >>>> And the answer is to change >>>> .windowedBy(TimeWindows.of(Duration.ofMillis(5000))) >>>> and specify the grace period: >>>> >>>> >>> windowedBy(TimeWindows.of(Duration.ofMillis(5000)).grace(Duration.ofMillis(100))) >>>> >>>> On Wed, Apr 15, 2020 at 4:34 PM Liam Clarke <liam.cla...@adscale.co.nz> >>>> wrote: >>>> >>>>> Okay, doing some debugging it looks like I'm seeing this behaviour >>> because >>>>> it's picking up a grace duration of 86,395,000 ms in >>>>> KTableImpl.buildSuppress, which would happen to be 5000 millis (my >>> window >>>>> size) off 24 hours, so I've got some clues! >>>>> >>>>> On Wed, Apr 15, 2020 at 3:43 PM Liam Clarke <liam.cla...@adscale.co.nz >>>> >>>>> wrote: >>>>> >>>>>> Hi all, >>>>>> >>>>>> I have a case where I want to consume from a topic, count the number >>> of >>>>>> certain ids in a given time period X, and emit a new record to a >>> different >>>>>> topic after that same time period X has elapsed containing the >>> aggregated >>>>>> value. >>>>>> >>>>>> I'm using suppress with Suppressed.untilWindowCloses, but nothing is >>> ever >>>>>> emitted, nor is my peek placed after the suppress ever being hit. >>>>>> My code is in the below Gist - I've hardcoded the durations for 5 >>> seconds >>>>>> after testing purposes: >>>>>> https://gist.github.com/LiamClarkeNZ/24121ccf0f09e4530749cbd92633fa46 >>>>>> >>>>>> I'm assuming I've misunderstood something drastically, and would >>> greatly >>>>>> appreciate a pointer on where I may have gone wrong. I'm wondering if >>> I >>>>>> need a larger retention on the persistent store? >>>>>> >>>>>> I understand that events have to arrive in order for windows to >>> close, so >>>>>> I've sent events after the window has expired to attempt to move the >>> window >>>>>> on, and my first peek (before the suppression) is emitting as I do: >>>>>> >>>>>> 1. 2020-04-15T03:36:48.569Z e2442bef-72bf-4424-b94e-7e4743e03c5e - 1 >>>>>> 1. 2020-04-15T03:37:11.682Z e2442bef-72bf-4424-b94e-7e4743e03c5e - 1 >>>>>> 1. 2020-04-15T03:39:18.882Z aqgzftnvyn - 1 >>>>>> >>>>>> >>>>>> Any guidance greatfully appreciated. >>>>>> >>>>>> Kind regards, >>>>>> >>>>>> Liam Clarke >>>>>> >>>>> >>>> >>> >>
signature.asc
Description: OpenPGP digital signature