[
https://issues.apache.org/jira/browse/KAFKA-8924?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16934171#comment-16934171
]
Michał edited comment on KAFKA-8924 at 9/20/19 7:42 AM:
--------------------------------------------------------
Hi.
Thanks a lot for such a detailed explanation. I think I understand the
internals much better now.
How the default grace period works is crystal clear now. I hope that the
external articles would do a better job describing this behaviour because I
think that is where I failed to get that information from in the first place.
I also understand how this impacts users who do not use suppress.
h2. Warning message
{quote}
In 2.3, we've added an info log that prints out the grace period if you're
using Suppress. If you can point out where we can add better documentation so
that you would have seen this up front, please let us know.
{quote}
About that, I think I have seen a message saying
{code}
Warning: window end time was truncated to Long.MAX
{code}
Is that the one? Because if so, it only appears when using
{code}
Suppressed.untilTimeLimit()
{code}
and *does not appear *when using
{code}
Suppressed.untilWindowCloses
{code}
I have checked in my tests. I am using Scala API.
So maybe there is some way for minor improvement. Also, this message was really
confusing and it did not make me think I did not specify grace period.
h2. Confusion source
{quote}
Maybe we can consider actually just requiring grace period to be explicitly set
when you use Suppress.untilWindowCloses (throwing a runtime exception if it's
not explicitly set). Then, we wouldn't need to pick a default at all.
{quote}
I think the better approach would be to take a step back and check what was the
source of confusion (for me at least).
And when I looked back I believe the issue is in the misleading
{code}
TimeWindows.of(windowSize)
{code}
because it handles the grace period for me, without me realizing the default
behaviour and the need to configure it.
So, I would say, a much easier approach would be to allow a user to create the
TimeWindows object with the constructor
{code}
return new TimeWindows(sizeMs, sizeMs, -1, DEFAULT_RETENTION_MS);
{code}
because then I am fully aware of each and every parameter, reading about it
would be 5 mins and I would save 2 days of pulling hair out of my head
wondering what is going on in the internals.
was (Author: atais):
Hi.
Thanks a lot for such a detailed explanation. I think I understand the
internals much better now.
How the default grace period works is crystal clear now. I hope that the
external articles would do a better job describing this behaviour because I
think that is where the confusion began.
I also understand how this impacts users who do not use suppress.
h2. Warning message
{quote}
In 2.3, we've added an info log that prints out the grace period if you're
using Suppress. If you can point out where we can add better documentation so
that you would have seen this up front, please let us know.
{quote}
About that, I think I have seen a message saying
{code}
Warning: window end time was truncated to Long.MAX
{code}
Is that the one? Because if so, it only appears when using
{code}
Suppressed.untilTimeLimit()
{code}
and *does not appear *when using
{code}
Suppressed.untilWindowCloses
{code}
I have checked in my tests. I am using Scala API.
So maybe there is some way for minor improvement. Also, this message was really
confusing and it did not make me think I did not specify grace period.
h2. Confusion source
{quote}
Maybe we can consider actually just requiring grace period to be explicitly set
when you use Suppress.untilWindowCloses (throwing a runtime exception if it's
not explicitly set). Then, we wouldn't need to pick a default at all.
{quote}
I think the better approach would be to take a step back and check what was the
source of confusion (for me at least).
And when I looked back I believe the issue is in the misleading
{code}
TimeWindows.of(windowSize)
{code}
because it handles the grace period for me, without me realizing the default
behaviour and the need to configure it.
So, I would say, a much easier approach would be to allow a user to create the
TimeWindows object with the constructor
{code}
return new TimeWindows(sizeMs, sizeMs, -1, DEFAULT_RETENTION_MS);
{code}
because then I am fully aware of each and every parameter, reading about it
would be 5 mins and I would save 2 days of pulling hair out of my head
wondering what is going on in the internals.
> Default grace period (-1) of TimeWindows causes suppress to never emit events
> ------------------------------------------------------------------------------
>
> Key: KAFKA-8924
> URL: https://issues.apache.org/jira/browse/KAFKA-8924
> Project: Kafka
> Issue Type: Bug
> Components: streams
> Affects Versions: 2.3.0
> Reporter: Michał
> Priority: Major
>
> h2. Problem
> The default creation of TimeWindows, like
> {code}
> TimeWindows.of(ofMillis(xxx))
> {code}
> calls an internal constructor
> {code}
> return new TimeWindows(sizeMs, sizeMs, -1, DEFAULT_RETENTION_MS);
> {code}
> And the *-1* parameter is the default grace period which I think is here for
> backward compatibility
> {code}
> @SuppressWarnings("deprecation") // continuing to support
> Windows#maintainMs/segmentInterval in fallback mode
> @Override
> public long gracePeriodMs() {
> // NOTE: in the future, when we remove maintainMs,
> // we should default the grace period to 24h to maintain the default
> behavior,
> // or we can default to (24h - size) if you want to be super accurate.
> return graceMs != -1 ? graceMs : maintainMs() - size();
> }
> {code}
> The problem is that if you use a TimeWindows with gracePeriod of *-1*
> together with suppress *untilWindowCloses*, it never emits an event.
> You can check the Suppress tests
> (SuppressScenarioTest.shouldSupportFinalResultsForTimeWindows), where
> [~vvcephei] was (maybe) aware of that and all the scenarios specify the
> gracePeriod.
> I will add a test without it on my branch and it will fail.
> The test:
> https://github.com/atais/kafka/commit/221a04dc40d636ffe93a0ad95dfc6bcad653f4db
>
> h2. Now what can be done
> One easy fix would be to change the default value to 0, which works fine for
> me in my project, however, I am not aware of the impact it would have done
> due to the changes in the *gracePeriodMs* method mentioned before.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
