lct45 commented on a change in pull request #9157:
URL: https://github.com/apache/kafka/pull/9157#discussion_r478459405
##########
File path:
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamSlidingWindowAggregate.java
##########
@@ -189,8 +195,8 @@ public void processInOrder(final K key, final V value,
final long timestamp) {
//create left window for new record
if (!leftWinAlreadyCreated) {
final ValueAndTimestamp<Agg> valueAndTime;
- //there's a right window that the new record could create -->
new record's left window is not empty
- if (latestLeftTypeWindow != null) {
+ // if there's a right window that the new record could create
--> new record's left window is not empty
Review comment:
It actually still works for that example. The range doesn't pick up the
window from [0,10] as it goes from timestamp - 2*timeDifference, so for this
example the range would start at 1. I initially did this to avoid these
scenarios entirely. You are right though that the comment doesn't hold for the
combined window scenarios. For example, we find the combined window and store
it as "latestLeftTypeWindow" when we process a record at say, 14. If the max
record in "latestLeftTypeWindow" is 2, then we don't want to create the right
window for that, since it would be [3,13].
I improved the comment to be `if there's a right window that the new record
could create && max record falls within left window --> new record's left
window is not empty`
----------------------------------------------------------------
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
