[GitHub] [kafka] izzyacademy commented on a change in pull request #10740: Kafka 8613 kip 633 drop default grace period streams

[GitBox <gi...@apache.org>]

izzyacademy commented on a change in pull request #10740:
URL: https://github.com/apache/kafka/pull/10740#discussion_r637013927



##########
File path: streams/src/main/java/org/apache/kafka/streams/kstream/SessionWindows.java
##########
@@ -84,17 +86,14 @@ private SessionWindows(final long gapMs, final long graceMs) {
      * Create a new window specification with the specified inactivity gap.
      *
      * @param inactivityGap the gap of inactivity between sessions
-     * @return a new window specification with default maintain duration of 1 day
+     * @return a new window specification with default without any grace period

Review comment:
       > I might have forgotten to specify this in the KIP, but we should make sure not to change the behavior of these old deprecated constructors. Maybe we can phrase this as something like:
   
   Hi @ableegoldman I may have misunderstood the KIP but I thought that we were changing org.apache.kafka.streams.kstream.Windows.DEFAULT_GRACE_PERIOD_MS from 24 hours (in milliseconds) to 0 milliseconds. That was what I did. Based on your feedback, it seems I misread the KIP and have the wrong implementation. 
   
   So, should we still leave the org.apache.kafka.streams.kstream.Windows.DEFAULT_GRACE_PERIOD_MS constant as (24 * 60 * 60 * 1000L) milliseconds and only set the grace period of zero milliseconds in the new static constructors?




-- 
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:
users@infra.apache.org