Oh. I though `TimeWindow` (singular) is part of the public API... For this case, I agree that we might not need a KIP, if there are no compatibility concerns to internally switch from `TimeWindow` to (newly added) `SlidingWindow` (that will also be internal).
Maybe best to do a POC PR to see if we can do the fix without a KIP of if we need one? @Luke: would this work for you? -Matthias On 7/23/21 1:00 PM, Sophie Blee-Goldman wrote: > @Matthias that operator doesn't need to be deprecated/updated as the > argument to it is a SlidingWindow*s*, not > a SlidingWindow (which is what this KIP is proposing to add). > The SlidingWindows class which is part of the public > API is really just a config container class, it doesn't hold the actual > data like the TimeWindow/SlidingWindow does. > > In fact, I'm not sure we even need a KIP at all for this change. The > Window/TimeWindow/SlidingWindow classes > are/would be internal, as they are only used within the sliding windowed > aggregation itself. > > On Fri, Jul 23, 2021 at 11:04 AM Matthias J. Sax <[email protected]> wrote: > >> Thanks for the KIP. Would be nice to fix this bug... >> >> Couple of comments: >> >> (1) nit: constructor should be `SlidingWindow` (not `TimeWindow` -- >> guess just a c&p error) >> >> (2) Adding a new window type it not sufficient. We also need to update >> `KGroupedStream#windowedBy()` to allow uses to use the newly added >> window. I don't think we can change `SlidingWindows` in a backward >> compatible way, thus, we need to add a new class and new overloads for >> `windowedBy()` to make the transition. (Also for `CogroupedKStream`.) >> >> (3) #2 implies we should deprecate the exiting >> `windowBy(SlidingWindows)` methods. >> >> >> >> -Matthias >> >> >> >> On 7/23/21 6:46 AM, Luke Chen wrote: >>> Hi, Kafka. >>> >>> I'd like to discuss the KIP-765: Introduce new SlidingWindow type for >>> [start,end] time. This is an existing bug in slidingWindows, that we used >>> the wrong window type so the window time interval will have 1 ms less at >>> the end time. This KIP will fix this issue. >>> >>> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-765%3A+Introduce+new+SlidingWindow+type+for+%5Bstart%2Cend%5D+time >>> >>> Thank you. >>> >>> Luke >>> >> >
