ableegoldman commented on a change in pull request #9157:
URL: https://github.com/apache/kafka/pull/9157#discussion_r478758482
##########
File path:
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamSlidingWindowAggregate.java
##########
@@ -160,11 +160,18 @@ public void processInOrder(final K key, final V value,
final long timestamp) {
if (endTime < timestamp) {
leftWinAgg = next.value;
+ // store the combined window if it is found so that a
right window can be created for
+ // the combined window's max record, as needed
if (isLeftWindow(next) || endTime ==
windows.timeDifferenceMs()) {
Review comment:
Yeah, you're saying that we just always keep track of the
`previousRecordTimestamp`, but before we go ahead and create a left window for
the current window we just actually verify that the previous record is within
range? That makes sense to me, actually if anything I feel like it will make
`rightWindowNecessaryAndPossible` even more clear to put it in terms of
`previousRecordTimestamp`. What I'm realizing from this is that it's easier to
understand these boolean checks in terms of the actual record locations, in
general. Maybe it's just my mental model, I still picture a rectangle sliding
over boxes on a timeline 😄
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
