You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@beam.apache.org by Kenneth Knowles <ke...@apache.org> on 2021/05/03 17:46:47 UTC
Re: Timer.withOutputTimestamp().offset().setRelative seems unusable
with event time
This seems like just a bug. If you set a timer for X and have output
timestamp Y where X < Y this should be fine. Is the problem the current
input watermark? Are you trying to set a timer with output timestamp that
is already past? I think that should be allowed, too, as long as the window
is not expired, but I may be missing something.
Some greater detail would be useful - maybe the full stack trace and/or a
failing unit test in a PR?
Kenn
On Thu, Apr 29, 2021 at 12:51 AM Jan Lukavský <je...@seznam.cz> wrote:
> Hi,
>
> I have come across a bug with timer output timestamp - when using event
> time and relative timers, setting the timer can arbitrarily throw
> IllegalArgumentException if the firing timestamp (input watermark) is
> ahead of the output timestamp (like .java.lang.IllegalArgumentException:
> Attempted to set an event-time timer with an output timestamp of
> 2021-04-29T07:16:19.369Z that is after the timer firing timestamp
> -290308-12-21T19:59:05.225Z). But there is no way to access the firing
> timestamp from user code. This means that the use has to either catch
> the IllegalArgumentException, or not use this construct at all.
>
> Catching the exception should probably not be part of a contract, so we
> should do one of the following:
>
> a) either throw the exception right away and disable using relative
> timers with output timestamp completely, or
>
> b) support it correctly
>
> What is the actual reason not to support output timestamps, that are
> ahead of firing timestamp? From my understanding, that should not be an
> issue, because there is TimestampCombiner.EARLIEST on the
> watermarkholdstate that corresponds to the output timestamp. If that is
> correct can we simply remove the check?
>
> Jan
>
>
Re: Timer.withOutputTimestamp().offset().setRelative seems unusable
with event time
Posted by Kenneth Knowles <ke...@apache.org>.
On Wed, May 5, 2021 at 2:41 AM Jan Lukavský <je...@seznam.cz> wrote:
> Hm, one thing in the last paragraph seems there might be some logical gap.
>
> > For a runner author to implement a "cleanup timer" requires a different
> mechanism. A window expires when *both* the input element watermark *and*
> the timer watermark are past the expiry time. In other words, the cleanup
> timer fires according to the minimum of these watermarks, combined. It
> *cannot* fire according to the input element watermark. If you naively try
> to implement it as a user timer, it will be incorrect. Incidentally this is
> why @OnWindowExpiration is a meaningful feature.
> The description describes a timer that fires not according to input
> watermark, but according to the output watermark (once the output watermark
> reaches certain point in time).
>
This simplification is close, but not how to think about it IMO. The output
timer is held by both the input watermark and the timer watermark, which is
also true of the combined input watermark. But the output watermark can
also be held by other things, such as in-progress elements, or propagation
delays, etc. The "combined input watermark" is an independent thing from
the output watermark.
Kenn
> That logically implies, that such a timer cannot have non-droppable output
> (at least if its output timestamp belongs to the respective window) and
> cannot create a watermark hold (because that would block the progress of
> the output watermark and might cause the timer to not fire ever). This
> maybe might be a useful user-feature as well, probably again mostly related
> to how user-code might want to deal with droppable data.
>
> Jan
>
> On 5/4/21 6:41 PM, Kenneth Knowles wrote:
>
> Mean to also add +Reuven Lax <re...@google.com>
>
> On Tue, May 4, 2021 at 9:41 AM Kenneth Knowles <ke...@apache.org> wrote:
>
>> Explicitly pinging a couple folks who were involved in the original
>> change which yours essentially reverts. There's a model question here that
>> I want to clarify on-list:
>>
>> When you have a ParDo setting timers, you have an additional watermark
>> that must be considered:
>>
>> - input element watermark
>> - output watermark
>> - *(user) timer watermark*
>>
>> The timer watermark is an input to the ParDo. Sometimes you might think
>> of the "timer channel" as a self loop, where each timer is an element. Each
>> timer has a timestamp (the output timestamp) and separately some
>> instructions on when to deliver that timer. This is the same as the usual
>> difference between event time and processing time.
>>
>> The instruction on when to deliver a timer can have two forms:
>>
>> - wait a certain amount of processing time
>> - deliver the timer when the *input element watermark* reaches a time X
>>
>> Here is an important point: "cleanup timers" are *not* user timers. They
>> are an implementation detail. They are not part of the model. The runner's
>> job is to reclaim resources as windows expire. A user should never be
>> reasoning about how their timers relate to cleanup timers (except for
>> resource consumption). Because there is no relationship except that the
>> cleanup should happen "eventually" and invisibly.
>>
>> For a runner author to implement a "cleanup timer" requires a different
>> mechanism. A window expires when *both* the input element watermark *and*
>> the timer watermark are past the expiry time. In other words, the cleanup
>> timer fires according to the minimum of these watermarks, combined. It
>> *cannot* fire according to the input element watermark. If you naively try
>> to implement it as a user timer, it will be incorrect. Incidentally this is
>> why @OnWindowExpiration is a meaningful feature.
>>
>> Kenn
>>
>> On Tue, May 4, 2021 at 4:29 AM Jan Lukavský <je...@seznam.cz> wrote:
>>
>>> Hi Kenn,
>>>
>>> I created BEAM-12276 [1] with PR [2].
>>>
>>> Jan
>>>
>>> [1] https://issues.apache.org/jira/browse/BEAM-12276
>>>
>>> [2] https://github.com/apache/beam/pull/14718
>>> On 5/3/21 7:46 PM, Kenneth Knowles wrote:
>>>
>>> This seems like just a bug. If you set a timer for X and have output
>>> timestamp Y where X < Y this should be fine. Is the problem the current
>>> input watermark? Are you trying to set a timer with output timestamp that
>>> is already past? I think that should be allowed, too, as long as the window
>>> is not expired, but I may be missing something.
>>>
>>> Some greater detail would be useful - maybe the full stack trace and/or
>>> a failing unit test in a PR?
>>>
>>> Kenn
>>>
>>> On Thu, Apr 29, 2021 at 12:51 AM Jan Lukavský <je...@seznam.cz> wrote:
>>>
>>>> Hi,
>>>>
>>>> I have come across a bug with timer output timestamp - when using event
>>>> time and relative timers, setting the timer can arbitrarily throw
>>>> IllegalArgumentException if the firing timestamp (input watermark) is
>>>> ahead of the output timestamp (like
>>>> .java.lang.IllegalArgumentException:
>>>> Attempted to set an event-time timer with an output timestamp of
>>>> 2021-04-29T07:16:19.369Z that is after the timer firing timestamp
>>>> -290308-12-21T19:59:05.225Z). But there is no way to access the firing
>>>> timestamp from user code. This means that the use has to either catch
>>>> the IllegalArgumentException, or not use this construct at all.
>>>>
>>>> Catching the exception should probably not be part of a contract, so we
>>>> should do one of the following:
>>>>
>>>> a) either throw the exception right away and disable using relative
>>>> timers with output timestamp completely, or
>>>>
>>>> b) support it correctly
>>>>
>>>> What is the actual reason not to support output timestamps, that are
>>>> ahead of firing timestamp? From my understanding, that should not be an
>>>> issue, because there is TimestampCombiner.EARLIEST on the
>>>> watermarkholdstate that corresponds to the output timestamp. If that is
>>>> correct can we simply remove the check?
>>>>
>>>> Jan
>>>>
>>>>
Re: Timer.withOutputTimestamp().offset().setRelative seems unusable
with event time
Posted by Reuven Lax <re...@google.com>.
Timer ordering is I believe part of the Beam model. Cleanup timers are not
though - that's an implementation detail (and an optimization, since the
programming model works fine if nothing is ever cleaned up).
On Wed, May 5, 2021 at 11:02 AM Kenneth Knowles <ke...@apache.org> wrote:
>
>
> On Wed, May 5, 2021 at 9:55 AM Reuven Lax <re...@google.com> wrote:
>
>>
>>
>> On Wed, May 5, 2021 at 9:36 AM Jan Lukavský <je...@seznam.cz> wrote:
>>
>>> Yes, what I meant was the distinguishing of cleanup timers from plain
>>> user other timers - that seems to be due to the fact that they fire base on
>>> different watermark (input/output).
>>>
>> They don't though, because AFAIK no runner has the capability to fire
>> timers based on output watermarks. Beam relies on timer ordering to fire
>> cleanup timers, which has a somewhat similar effect - by the time the
>> cleanup timer fires, all other timers are guaranteed to have fired.
>>
>
> Again, cleanup timers are not part of the Beam model. Beam doesn't rely on
> ordering of timers. Beam doesn't even rely on the _existence_ of cleanup
> timers. Beam doesn't even rely on the existence of _cleanup_.
>
> Kenn
>
>
>>
>>
>>> And firing timers based on output watermark might be actually a good
>>> user-facing feature, because that might help tracking output watermark in
>>> transforms that want to deal with potentially droppable data downstream
>>> (the input would have to be re-windowed to global window, of course). I
>>> don't know if there are other use-cases, if not maybe it might be
>>> sufficient to create a DroppableDataSplit transform, that would create a
>>> PCollectionTuple with droppable and other data. But that was just an idea
>>> when Kenn mentioned that the cleanup timers "fire differently" - I
>>> generally think that when there is a need for a different behavior, than it
>>> might signal there is something possibly fundamental.
>>>
>>
>> We provide OnWindowExpiration, which is basically this. It fires after
>> all other windows and timers, but just before the cleanup happens (in fact
>> we currently piggy back on the cleanup timer to implement
>> OnWindowExpiration).
>>
>>> Jan
>>> On 5/5/21 5:11 PM, Reuven Lax wrote:
>>>
>>> This is one way to put it. I think in practice Beam guarantees that
>>> timers fire in order for a given key (though there is still a bit of a bug
>>> around looping timers - the fix for that got rolled back). This means that
>>> as long as the runner sets the cleanup timer to be 1ms passed the end of
>>> the window (plus allowed lateness), it's guaranteed to be the last timer
>>> that fires for that window.
>>>
>>>
>>>
>>> On Wed, May 5, 2021 at 2:41 AM Jan Lukavský <je...@seznam.cz> wrote:
>>>
>>>> Hm, one thing in the last paragraph seems there might be some logical
>>>> gap.
>>>>
>>>> > For a runner author to implement a "cleanup timer" requires a
>>>> different mechanism. A window expires when *both* the input element
>>>> watermark *and* the timer watermark are past the expiry time. In other
>>>> words, the cleanup timer fires according to the minimum of these
>>>> watermarks, combined. It *cannot* fire according to the input element
>>>> watermark. If you naively try to implement it as a user timer, it will be
>>>> incorrect. Incidentally this is why @OnWindowExpiration is a meaningful
>>>> feature.
>>>> The description describes a timer that fires not according to input
>>>> watermark, but according to the output watermark (once the output watermark
>>>> reaches certain point in time). That logically implies, that such a timer
>>>> cannot have non-droppable output (at least if its output timestamp belongs
>>>> to the respective window) and cannot create a watermark hold (because that
>>>> would block the progress of the output watermark and might cause the timer
>>>> to not fire ever). This maybe might be a useful user-feature as well,
>>>> probably again mostly related to how user-code might want to deal with
>>>> droppable data.
>>>>
>>>> Jan
>>>>
>>>> On 5/4/21 6:41 PM, Kenneth Knowles wrote:
>>>>
>>>> Mean to also add +Reuven Lax <re...@google.com>
>>>>
>>>> On Tue, May 4, 2021 at 9:41 AM Kenneth Knowles <ke...@apache.org> wrote:
>>>>
>>>>> Explicitly pinging a couple folks who were involved in the original
>>>>> change which yours essentially reverts. There's a model question here that
>>>>> I want to clarify on-list:
>>>>>
>>>>> When you have a ParDo setting timers, you have an additional watermark
>>>>> that must be considered:
>>>>>
>>>>> - input element watermark
>>>>> - output watermark
>>>>> - *(user) timer watermark*
>>>>>
>>>>> The timer watermark is an input to the ParDo. Sometimes you might
>>>>> think of the "timer channel" as a self loop, where each timer is an
>>>>> element. Each timer has a timestamp (the output timestamp) and separately
>>>>> some instructions on when to deliver that timer. This is the same as the
>>>>> usual difference between event time and processing time.
>>>>>
>>>>> The instruction on when to deliver a timer can have two forms:
>>>>>
>>>>> - wait a certain amount of processing time
>>>>> - deliver the timer when the *input element watermark* reaches a time
>>>>> X
>>>>>
>>>>> Here is an important point: "cleanup timers" are *not* user timers.
>>>>> They are an implementation detail. They are not part of the model. The
>>>>> runner's job is to reclaim resources as windows expire. A user should never
>>>>> be reasoning about how their timers relate to cleanup timers (except for
>>>>> resource consumption). Because there is no relationship except that the
>>>>> cleanup should happen "eventually" and invisibly.
>>>>>
>>>>> For a runner author to implement a "cleanup timer" requires a
>>>>> different mechanism. A window expires when *both* the input element
>>>>> watermark *and* the timer watermark are past the expiry time. In other
>>>>> words, the cleanup timer fires according to the minimum of these
>>>>> watermarks, combined. It *cannot* fire according to the input element
>>>>> watermark. If you naively try to implement it as a user timer, it will be
>>>>> incorrect. Incidentally this is why @OnWindowExpiration is a meaningful
>>>>> feature.
>>>>>
>>>>> Kenn
>>>>>
>>>>> On Tue, May 4, 2021 at 4:29 AM Jan Lukavský <je...@seznam.cz> wrote:
>>>>>
>>>>>> Hi Kenn,
>>>>>>
>>>>>> I created BEAM-12276 [1] with PR [2].
>>>>>>
>>>>>> Jan
>>>>>>
>>>>>> [1] https://issues.apache.org/jira/browse/BEAM-12276
>>>>>>
>>>>>> [2] https://github.com/apache/beam/pull/14718
>>>>>> On 5/3/21 7:46 PM, Kenneth Knowles wrote:
>>>>>>
>>>>>> This seems like just a bug. If you set a timer for X and have output
>>>>>> timestamp Y where X < Y this should be fine. Is the problem the current
>>>>>> input watermark? Are you trying to set a timer with output timestamp that
>>>>>> is already past? I think that should be allowed, too, as long as the window
>>>>>> is not expired, but I may be missing something.
>>>>>>
>>>>>> Some greater detail would be useful - maybe the full stack trace
>>>>>> and/or a failing unit test in a PR?
>>>>>>
>>>>>> Kenn
>>>>>>
>>>>>> On Thu, Apr 29, 2021 at 12:51 AM Jan Lukavský <je...@seznam.cz>
>>>>>> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> I have come across a bug with timer output timestamp - when using
>>>>>>> event
>>>>>>> time and relative timers, setting the timer can arbitrarily throw
>>>>>>> IllegalArgumentException if the firing timestamp (input watermark)
>>>>>>> is
>>>>>>> ahead of the output timestamp (like
>>>>>>> .java.lang.IllegalArgumentException:
>>>>>>> Attempted to set an event-time timer with an output timestamp of
>>>>>>> 2021-04-29T07:16:19.369Z that is after the timer firing timestamp
>>>>>>> -290308-12-21T19:59:05.225Z). But there is no way to access the
>>>>>>> firing
>>>>>>> timestamp from user code. This means that the use has to either
>>>>>>> catch
>>>>>>> the IllegalArgumentException, or not use this construct at all.
>>>>>>>
>>>>>>> Catching the exception should probably not be part of a contract, so
>>>>>>> we
>>>>>>> should do one of the following:
>>>>>>>
>>>>>>> a) either throw the exception right away and disable using
>>>>>>> relative
>>>>>>> timers with output timestamp completely, or
>>>>>>>
>>>>>>> b) support it correctly
>>>>>>>
>>>>>>> What is the actual reason not to support output timestamps, that are
>>>>>>> ahead of firing timestamp? From my understanding, that should not be
>>>>>>> an
>>>>>>> issue, because there is TimestampCombiner.EARLIEST on the
>>>>>>> watermarkholdstate that corresponds to the output timestamp. If that
>>>>>>> is
>>>>>>> correct can we simply remove the check?
>>>>>>>
>>>>>>> Jan
>>>>>>>
>>>>>>>
Re: Timer.withOutputTimestamp().offset().setRelative seems unusable
with event time
Posted by Kenneth Knowles <ke...@apache.org>.
On Wed, May 5, 2021 at 9:55 AM Reuven Lax <re...@google.com> wrote:
>
>
> On Wed, May 5, 2021 at 9:36 AM Jan Lukavský <je...@seznam.cz> wrote:
>
>> Yes, what I meant was the distinguishing of cleanup timers from plain
>> user other timers - that seems to be due to the fact that they fire base on
>> different watermark (input/output).
>>
> They don't though, because AFAIK no runner has the capability to fire
> timers based on output watermarks. Beam relies on timer ordering to fire
> cleanup timers, which has a somewhat similar effect - by the time the
> cleanup timer fires, all other timers are guaranteed to have fired.
>
Again, cleanup timers are not part of the Beam model. Beam doesn't rely on
ordering of timers. Beam doesn't even rely on the _existence_ of cleanup
timers. Beam doesn't even rely on the existence of _cleanup_.
Kenn
>
>
>> And firing timers based on output watermark might be actually a good
>> user-facing feature, because that might help tracking output watermark in
>> transforms that want to deal with potentially droppable data downstream
>> (the input would have to be re-windowed to global window, of course). I
>> don't know if there are other use-cases, if not maybe it might be
>> sufficient to create a DroppableDataSplit transform, that would create a
>> PCollectionTuple with droppable and other data. But that was just an idea
>> when Kenn mentioned that the cleanup timers "fire differently" - I
>> generally think that when there is a need for a different behavior, than it
>> might signal there is something possibly fundamental.
>>
>
> We provide OnWindowExpiration, which is basically this. It fires after all
> other windows and timers, but just before the cleanup happens (in fact we
> currently piggy back on the cleanup timer to implement OnWindowExpiration).
>
>> Jan
>> On 5/5/21 5:11 PM, Reuven Lax wrote:
>>
>> This is one way to put it. I think in practice Beam guarantees that
>> timers fire in order for a given key (though there is still a bit of a bug
>> around looping timers - the fix for that got rolled back). This means that
>> as long as the runner sets the cleanup timer to be 1ms passed the end of
>> the window (plus allowed lateness), it's guaranteed to be the last timer
>> that fires for that window.
>>
>>
>>
>> On Wed, May 5, 2021 at 2:41 AM Jan Lukavský <je...@seznam.cz> wrote:
>>
>>> Hm, one thing in the last paragraph seems there might be some logical
>>> gap.
>>>
>>> > For a runner author to implement a "cleanup timer" requires a
>>> different mechanism. A window expires when *both* the input element
>>> watermark *and* the timer watermark are past the expiry time. In other
>>> words, the cleanup timer fires according to the minimum of these
>>> watermarks, combined. It *cannot* fire according to the input element
>>> watermark. If you naively try to implement it as a user timer, it will be
>>> incorrect. Incidentally this is why @OnWindowExpiration is a meaningful
>>> feature.
>>> The description describes a timer that fires not according to input
>>> watermark, but according to the output watermark (once the output watermark
>>> reaches certain point in time). That logically implies, that such a timer
>>> cannot have non-droppable output (at least if its output timestamp belongs
>>> to the respective window) and cannot create a watermark hold (because that
>>> would block the progress of the output watermark and might cause the timer
>>> to not fire ever). This maybe might be a useful user-feature as well,
>>> probably again mostly related to how user-code might want to deal with
>>> droppable data.
>>>
>>> Jan
>>>
>>> On 5/4/21 6:41 PM, Kenneth Knowles wrote:
>>>
>>> Mean to also add +Reuven Lax <re...@google.com>
>>>
>>> On Tue, May 4, 2021 at 9:41 AM Kenneth Knowles <ke...@apache.org> wrote:
>>>
>>>> Explicitly pinging a couple folks who were involved in the original
>>>> change which yours essentially reverts. There's a model question here that
>>>> I want to clarify on-list:
>>>>
>>>> When you have a ParDo setting timers, you have an additional watermark
>>>> that must be considered:
>>>>
>>>> - input element watermark
>>>> - output watermark
>>>> - *(user) timer watermark*
>>>>
>>>> The timer watermark is an input to the ParDo. Sometimes you might think
>>>> of the "timer channel" as a self loop, where each timer is an element. Each
>>>> timer has a timestamp (the output timestamp) and separately some
>>>> instructions on when to deliver that timer. This is the same as the usual
>>>> difference between event time and processing time.
>>>>
>>>> The instruction on when to deliver a timer can have two forms:
>>>>
>>>> - wait a certain amount of processing time
>>>> - deliver the timer when the *input element watermark* reaches a time X
>>>>
>>>> Here is an important point: "cleanup timers" are *not* user timers.
>>>> They are an implementation detail. They are not part of the model. The
>>>> runner's job is to reclaim resources as windows expire. A user should never
>>>> be reasoning about how their timers relate to cleanup timers (except for
>>>> resource consumption). Because there is no relationship except that the
>>>> cleanup should happen "eventually" and invisibly.
>>>>
>>>> For a runner author to implement a "cleanup timer" requires a different
>>>> mechanism. A window expires when *both* the input element watermark *and*
>>>> the timer watermark are past the expiry time. In other words, the cleanup
>>>> timer fires according to the minimum of these watermarks, combined. It
>>>> *cannot* fire according to the input element watermark. If you naively try
>>>> to implement it as a user timer, it will be incorrect. Incidentally this is
>>>> why @OnWindowExpiration is a meaningful feature.
>>>>
>>>> Kenn
>>>>
>>>> On Tue, May 4, 2021 at 4:29 AM Jan Lukavský <je...@seznam.cz> wrote:
>>>>
>>>>> Hi Kenn,
>>>>>
>>>>> I created BEAM-12276 [1] with PR [2].
>>>>>
>>>>> Jan
>>>>>
>>>>> [1] https://issues.apache.org/jira/browse/BEAM-12276
>>>>>
>>>>> [2] https://github.com/apache/beam/pull/14718
>>>>> On 5/3/21 7:46 PM, Kenneth Knowles wrote:
>>>>>
>>>>> This seems like just a bug. If you set a timer for X and have output
>>>>> timestamp Y where X < Y this should be fine. Is the problem the current
>>>>> input watermark? Are you trying to set a timer with output timestamp that
>>>>> is already past? I think that should be allowed, too, as long as the window
>>>>> is not expired, but I may be missing something.
>>>>>
>>>>> Some greater detail would be useful - maybe the full stack trace
>>>>> and/or a failing unit test in a PR?
>>>>>
>>>>> Kenn
>>>>>
>>>>> On Thu, Apr 29, 2021 at 12:51 AM Jan Lukavský <je...@seznam.cz> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I have come across a bug with timer output timestamp - when using
>>>>>> event
>>>>>> time and relative timers, setting the timer can arbitrarily throw
>>>>>> IllegalArgumentException if the firing timestamp (input watermark) is
>>>>>> ahead of the output timestamp (like
>>>>>> .java.lang.IllegalArgumentException:
>>>>>> Attempted to set an event-time timer with an output timestamp of
>>>>>> 2021-04-29T07:16:19.369Z that is after the timer firing timestamp
>>>>>> -290308-12-21T19:59:05.225Z). But there is no way to access the
>>>>>> firing
>>>>>> timestamp from user code. This means that the use has to either catch
>>>>>> the IllegalArgumentException, or not use this construct at all.
>>>>>>
>>>>>> Catching the exception should probably not be part of a contract, so
>>>>>> we
>>>>>> should do one of the following:
>>>>>>
>>>>>> a) either throw the exception right away and disable using relative
>>>>>> timers with output timestamp completely, or
>>>>>>
>>>>>> b) support it correctly
>>>>>>
>>>>>> What is the actual reason not to support output timestamps, that are
>>>>>> ahead of firing timestamp? From my understanding, that should not be
>>>>>> an
>>>>>> issue, because there is TimestampCombiner.EARLIEST on the
>>>>>> watermarkholdstate that corresponds to the output timestamp. If that
>>>>>> is
>>>>>> correct can we simply remove the check?
>>>>>>
>>>>>> Jan
>>>>>>
>>>>>>
Re: Timer.withOutputTimestamp().offset().setRelative seems unusable
with event time
Posted by Reuven Lax <re...@google.com>.
On Wed, May 5, 2021 at 9:36 AM Jan Lukavský <je...@seznam.cz> wrote:
> Yes, what I meant was the distinguishing of cleanup timers from plain user
> other timers - that seems to be due to the fact that they fire base on
> different watermark (input/output).
>
They don't though, because AFAIK no runner has the capability to fire
timers based on output watermarks. Beam relies on timer ordering to fire
cleanup timers, which has a somewhat similar effect - by the time the
cleanup timer fires, all other timers are guaranteed to have fired.
> And firing timers based on output watermark might be actually a good
> user-facing feature, because that might help tracking output watermark in
> transforms that want to deal with potentially droppable data downstream
> (the input would have to be re-windowed to global window, of course). I
> don't know if there are other use-cases, if not maybe it might be
> sufficient to create a DroppableDataSplit transform, that would create a
> PCollectionTuple with droppable and other data. But that was just an idea
> when Kenn mentioned that the cleanup timers "fire differently" - I
> generally think that when there is a need for a different behavior, than it
> might signal there is something possibly fundamental.
>
We provide OnWindowExpiration, which is basically this. It fires after all
other windows and timers, but just before the cleanup happens (in fact we
currently piggy back on the cleanup timer to implement OnWindowExpiration).
> Jan
> On 5/5/21 5:11 PM, Reuven Lax wrote:
>
> This is one way to put it. I think in practice Beam guarantees that timers
> fire in order for a given key (though there is still a bit of a bug around
> looping timers - the fix for that got rolled back). This means that as long
> as the runner sets the cleanup timer to be 1ms passed the end of the window
> (plus allowed lateness), it's guaranteed to be the last timer that fires
> for that window.
>
>
>
> On Wed, May 5, 2021 at 2:41 AM Jan Lukavský <je...@seznam.cz> wrote:
>
>> Hm, one thing in the last paragraph seems there might be some logical gap.
>>
>> > For a runner author to implement a "cleanup timer" requires a different
>> mechanism. A window expires when *both* the input element watermark *and*
>> the timer watermark are past the expiry time. In other words, the cleanup
>> timer fires according to the minimum of these watermarks, combined. It
>> *cannot* fire according to the input element watermark. If you naively try
>> to implement it as a user timer, it will be incorrect. Incidentally this is
>> why @OnWindowExpiration is a meaningful feature.
>> The description describes a timer that fires not according to input
>> watermark, but according to the output watermark (once the output watermark
>> reaches certain point in time). That logically implies, that such a timer
>> cannot have non-droppable output (at least if its output timestamp belongs
>> to the respective window) and cannot create a watermark hold (because that
>> would block the progress of the output watermark and might cause the timer
>> to not fire ever). This maybe might be a useful user-feature as well,
>> probably again mostly related to how user-code might want to deal with
>> droppable data.
>>
>> Jan
>>
>> On 5/4/21 6:41 PM, Kenneth Knowles wrote:
>>
>> Mean to also add +Reuven Lax <re...@google.com>
>>
>> On Tue, May 4, 2021 at 9:41 AM Kenneth Knowles <ke...@apache.org> wrote:
>>
>>> Explicitly pinging a couple folks who were involved in the original
>>> change which yours essentially reverts. There's a model question here that
>>> I want to clarify on-list:
>>>
>>> When you have a ParDo setting timers, you have an additional watermark
>>> that must be considered:
>>>
>>> - input element watermark
>>> - output watermark
>>> - *(user) timer watermark*
>>>
>>> The timer watermark is an input to the ParDo. Sometimes you might think
>>> of the "timer channel" as a self loop, where each timer is an element. Each
>>> timer has a timestamp (the output timestamp) and separately some
>>> instructions on when to deliver that timer. This is the same as the usual
>>> difference between event time and processing time.
>>>
>>> The instruction on when to deliver a timer can have two forms:
>>>
>>> - wait a certain amount of processing time
>>> - deliver the timer when the *input element watermark* reaches a time X
>>>
>>> Here is an important point: "cleanup timers" are *not* user timers. They
>>> are an implementation detail. They are not part of the model. The runner's
>>> job is to reclaim resources as windows expire. A user should never be
>>> reasoning about how their timers relate to cleanup timers (except for
>>> resource consumption). Because there is no relationship except that the
>>> cleanup should happen "eventually" and invisibly.
>>>
>>> For a runner author to implement a "cleanup timer" requires a different
>>> mechanism. A window expires when *both* the input element watermark *and*
>>> the timer watermark are past the expiry time. In other words, the cleanup
>>> timer fires according to the minimum of these watermarks, combined. It
>>> *cannot* fire according to the input element watermark. If you naively try
>>> to implement it as a user timer, it will be incorrect. Incidentally this is
>>> why @OnWindowExpiration is a meaningful feature.
>>>
>>> Kenn
>>>
>>> On Tue, May 4, 2021 at 4:29 AM Jan Lukavský <je...@seznam.cz> wrote:
>>>
>>>> Hi Kenn,
>>>>
>>>> I created BEAM-12276 [1] with PR [2].
>>>>
>>>> Jan
>>>>
>>>> [1] https://issues.apache.org/jira/browse/BEAM-12276
>>>>
>>>> [2] https://github.com/apache/beam/pull/14718
>>>> On 5/3/21 7:46 PM, Kenneth Knowles wrote:
>>>>
>>>> This seems like just a bug. If you set a timer for X and have output
>>>> timestamp Y where X < Y this should be fine. Is the problem the current
>>>> input watermark? Are you trying to set a timer with output timestamp that
>>>> is already past? I think that should be allowed, too, as long as the window
>>>> is not expired, but I may be missing something.
>>>>
>>>> Some greater detail would be useful - maybe the full stack trace and/or
>>>> a failing unit test in a PR?
>>>>
>>>> Kenn
>>>>
>>>> On Thu, Apr 29, 2021 at 12:51 AM Jan Lukavský <je...@seznam.cz> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> I have come across a bug with timer output timestamp - when using
>>>>> event
>>>>> time and relative timers, setting the timer can arbitrarily throw
>>>>> IllegalArgumentException if the firing timestamp (input watermark) is
>>>>> ahead of the output timestamp (like
>>>>> .java.lang.IllegalArgumentException:
>>>>> Attempted to set an event-time timer with an output timestamp of
>>>>> 2021-04-29T07:16:19.369Z that is after the timer firing timestamp
>>>>> -290308-12-21T19:59:05.225Z). But there is no way to access the firing
>>>>> timestamp from user code. This means that the use has to either catch
>>>>> the IllegalArgumentException, or not use this construct at all.
>>>>>
>>>>> Catching the exception should probably not be part of a contract, so
>>>>> we
>>>>> should do one of the following:
>>>>>
>>>>> a) either throw the exception right away and disable using relative
>>>>> timers with output timestamp completely, or
>>>>>
>>>>> b) support it correctly
>>>>>
>>>>> What is the actual reason not to support output timestamps, that are
>>>>> ahead of firing timestamp? From my understanding, that should not be
>>>>> an
>>>>> issue, because there is TimestampCombiner.EARLIEST on the
>>>>> watermarkholdstate that corresponds to the output timestamp. If that
>>>>> is
>>>>> correct can we simply remove the check?
>>>>>
>>>>> Jan
>>>>>
>>>>>
Re: Timer.withOutputTimestamp().offset().setRelative seems unusable
with event time
Posted by Jan Lukavský <je...@seznam.cz>.
Yes, what I meant was the distinguishing of cleanup timers from plain
user other timers - that seems to be due to the fact that they fire base
on different watermark (input/output). And firing timers based on output
watermark might be actually a good user-facing feature, because that
might help tracking output watermark in transforms that want to deal
with potentially droppable data downstream (the input would have to be
re-windowed to global window, of course). I don't know if there are
other use-cases, if not maybe it might be sufficient to create a
DroppableDataSplit transform, that would create a PCollectionTuple with
droppable and other data. But that was just an idea when Kenn mentioned
that the cleanup timers "fire differently" - I generally think that when
there is a need for a different behavior, than it might signal there is
something possibly fundamental.
Jan
On 5/5/21 5:11 PM, Reuven Lax wrote:
> This is one way to put it. I think in practice Beam guarantees that
> timers fire in order for a given key (though there is still a bit of a
> bug around looping timers - the fix for that got rolled back). This
> means that as long as the runner sets the cleanup timer to be 1ms
> passed the end of the window (plus allowed lateness), it's
> guaranteed to be the last timer that fires for that window.
>
>
>
> On Wed, May 5, 2021 at 2:41 AM Jan Lukavský <je.ik@seznam.cz
> <ma...@seznam.cz>> wrote:
>
> Hm, one thing in the last paragraph seems there might be some
> logical gap.
>
> > For a runner author to implement a "cleanup timer" requires a
> different mechanism. A window expires when *both* the input
> element watermark *and* the timer watermark are past the expiry
> time. In other words, the cleanup timer fires according to the
> minimum of these watermarks, combined. It *cannot* fire according
> to the input element watermark. If you naively try to implement it
> as a user timer, it will be incorrect. Incidentally this is
> why @OnWindowExpiration is a meaningful feature.
>
> The description describes a timer that fires not according to
> input watermark, but according to the output watermark (once the
> output watermark reaches certain point in time). That logically
> implies, that such a timer cannot have non-droppable output (at
> least if its output timestamp belongs to the respective window)
> and cannot create a watermark hold (because that would block the
> progress of the output watermark and might cause the timer to not
> fire ever). This maybe might be a useful user-feature as well,
> probably again mostly related to how user-code might want to deal
> with droppable data.
>
> Jan
>
> On 5/4/21 6:41 PM, Kenneth Knowles wrote:
>> Mean to also add +Reuven Lax <ma...@google.com>
>>
>> On Tue, May 4, 2021 at 9:41 AM Kenneth Knowles <kenn@apache.org
>> <ma...@apache.org>> wrote:
>>
>> Explicitly pinging a couple folks who were involved in the
>> original change which yours essentially reverts. There's a
>> model question here that I want to clarify on-list:
>>
>> When you have a ParDo setting timers, you have an additional
>> watermark that must be considered:
>>
>> - input element watermark
>> - output watermark
>> - *(user) timer watermark*
>>
>> The timer watermark is an input to the ParDo. Sometimes you
>> might think of the "timer channel" as a self loop, where each
>> timer is an element. Each timer has a timestamp (the output
>> timestamp) and separately some instructions on when to
>> deliver that timer. This is the same as the usual difference
>> between event time and processing time.
>>
>> The instruction on when to deliver a timer can have two forms:
>>
>> - wait a certain amount of processing time
>> - deliver the timer when the *input element watermark*
>> reaches a time X
>>
>> Here is an important point: "cleanup timers" are *not* user
>> timers. They are an implementation detail. They are not part
>> of the model. The runner's job is to reclaim resources as
>> windows expire. A user should never be reasoning about how
>> their timers relate to cleanup timers (except for resource
>> consumption). Because there is no relationship except that
>> the cleanup should happen "eventually" and invisibly.
>>
>> For a runner author to implement a "cleanup timer" requires a
>> different mechanism. A window expires when *both* the input
>> element watermark *and* the timer watermark are past the
>> expiry time. In other words, the cleanup timer fires
>> according to the minimum of these watermarks, combined. It
>> *cannot* fire according to the input element watermark. If
>> you naively try to implement it as a user timer, it will be
>> incorrect. Incidentally this is why @OnWindowExpiration is a
>> meaningful feature.
>>
>> Kenn
>>
>> On Tue, May 4, 2021 at 4:29 AM Jan Lukavský <je.ik@seznam.cz
>> <ma...@seznam.cz>> wrote:
>>
>> Hi Kenn,
>>
>> I created BEAM-12276 [1] with PR [2].
>>
>> Jan
>>
>> [1] https://issues.apache.org/jira/browse/BEAM-12276
>> <https://issues.apache.org/jira/browse/BEAM-12276>
>>
>> [2] https://github.com/apache/beam/pull/14718
>> <https://github.com/apache/beam/pull/14718>
>>
>> On 5/3/21 7:46 PM, Kenneth Knowles wrote:
>>> This seems like just a bug. If you set a timer for X and
>>> have output timestamp Y where X < Y this should be fine.
>>> Is the problem the current input watermark? Are you
>>> trying to set a timer with output timestamp that is
>>> already past? I think that should be allowed, too, as
>>> long as the window is not expired, but I may be missing
>>> something.
>>>
>>> Some greater detail would be useful - maybe the full
>>> stack trace and/or a failing unit test in a PR?
>>>
>>> Kenn
>>>
>>> On Thu, Apr 29, 2021 at 12:51 AM Jan Lukavský
>>> <je.ik@seznam.cz <ma...@seznam.cz>> wrote:
>>>
>>> Hi,
>>>
>>> I have come across a bug with timer output timestamp
>>> - when using event
>>> time and relative timers, setting the timer can
>>> arbitrarily throw
>>> IllegalArgumentException if the firing timestamp
>>> (input watermark) is
>>> ahead of the output timestamp (like
>>> .java.lang.IllegalArgumentException:
>>> Attempted to set an event-time timer with an output
>>> timestamp of
>>> 2021-04-29T07:16:19.369Z that is after the timer
>>> firing timestamp
>>> -290308-12-21T19:59:05.225Z). But there is no way to
>>> access the firing
>>> timestamp from user code. This means that the use
>>> has to either catch
>>> the IllegalArgumentException, or not use this
>>> construct at all.
>>>
>>> Catching the exception should probably not be part
>>> of a contract, so we
>>> should do one of the following:
>>>
>>> a) either throw the exception right away and
>>> disable using relative
>>> timers with output timestamp completely, or
>>>
>>> b) support it correctly
>>>
>>> What is the actual reason not to support output
>>> timestamps, that are
>>> ahead of firing timestamp? From my understanding,
>>> that should not be an
>>> issue, because there is TimestampCombiner.EARLIEST
>>> on the
>>> watermarkholdstate that corresponds to the output
>>> timestamp. If that is
>>> correct can we simply remove the check?
>>>
>>> Jan
>>>
Re: Timer.withOutputTimestamp().offset().setRelative seems unusable
with event time
Posted by Reuven Lax <re...@google.com>.
This is one way to put it. I think in practice Beam guarantees that timers
fire in order for a given key (though there is still a bit of a bug around
looping timers - the fix for that got rolled back). This means that as long
as the runner sets the cleanup timer to be 1ms passed the end of the window
(plus allowed lateness), it's guaranteed to be the last timer that fires
for that window.
On Wed, May 5, 2021 at 2:41 AM Jan Lukavský <je...@seznam.cz> wrote:
> Hm, one thing in the last paragraph seems there might be some logical gap.
>
> > For a runner author to implement a "cleanup timer" requires a different
> mechanism. A window expires when *both* the input element watermark *and*
> the timer watermark are past the expiry time. In other words, the cleanup
> timer fires according to the minimum of these watermarks, combined. It
> *cannot* fire according to the input element watermark. If you naively try
> to implement it as a user timer, it will be incorrect. Incidentally this is
> why @OnWindowExpiration is a meaningful feature.
> The description describes a timer that fires not according to input
> watermark, but according to the output watermark (once the output watermark
> reaches certain point in time). That logically implies, that such a timer
> cannot have non-droppable output (at least if its output timestamp belongs
> to the respective window) and cannot create a watermark hold (because that
> would block the progress of the output watermark and might cause the timer
> to not fire ever). This maybe might be a useful user-feature as well,
> probably again mostly related to how user-code might want to deal with
> droppable data.
>
> Jan
>
> On 5/4/21 6:41 PM, Kenneth Knowles wrote:
>
> Mean to also add +Reuven Lax <re...@google.com>
>
> On Tue, May 4, 2021 at 9:41 AM Kenneth Knowles <ke...@apache.org> wrote:
>
>> Explicitly pinging a couple folks who were involved in the original
>> change which yours essentially reverts. There's a model question here that
>> I want to clarify on-list:
>>
>> When you have a ParDo setting timers, you have an additional watermark
>> that must be considered:
>>
>> - input element watermark
>> - output watermark
>> - *(user) timer watermark*
>>
>> The timer watermark is an input to the ParDo. Sometimes you might think
>> of the "timer channel" as a self loop, where each timer is an element. Each
>> timer has a timestamp (the output timestamp) and separately some
>> instructions on when to deliver that timer. This is the same as the usual
>> difference between event time and processing time.
>>
>> The instruction on when to deliver a timer can have two forms:
>>
>> - wait a certain amount of processing time
>> - deliver the timer when the *input element watermark* reaches a time X
>>
>> Here is an important point: "cleanup timers" are *not* user timers. They
>> are an implementation detail. They are not part of the model. The runner's
>> job is to reclaim resources as windows expire. A user should never be
>> reasoning about how their timers relate to cleanup timers (except for
>> resource consumption). Because there is no relationship except that the
>> cleanup should happen "eventually" and invisibly.
>>
>> For a runner author to implement a "cleanup timer" requires a different
>> mechanism. A window expires when *both* the input element watermark *and*
>> the timer watermark are past the expiry time. In other words, the cleanup
>> timer fires according to the minimum of these watermarks, combined. It
>> *cannot* fire according to the input element watermark. If you naively try
>> to implement it as a user timer, it will be incorrect. Incidentally this is
>> why @OnWindowExpiration is a meaningful feature.
>>
>> Kenn
>>
>> On Tue, May 4, 2021 at 4:29 AM Jan Lukavský <je...@seznam.cz> wrote:
>>
>>> Hi Kenn,
>>>
>>> I created BEAM-12276 [1] with PR [2].
>>>
>>> Jan
>>>
>>> [1] https://issues.apache.org/jira/browse/BEAM-12276
>>>
>>> [2] https://github.com/apache/beam/pull/14718
>>> On 5/3/21 7:46 PM, Kenneth Knowles wrote:
>>>
>>> This seems like just a bug. If you set a timer for X and have output
>>> timestamp Y where X < Y this should be fine. Is the problem the current
>>> input watermark? Are you trying to set a timer with output timestamp that
>>> is already past? I think that should be allowed, too, as long as the window
>>> is not expired, but I may be missing something.
>>>
>>> Some greater detail would be useful - maybe the full stack trace and/or
>>> a failing unit test in a PR?
>>>
>>> Kenn
>>>
>>> On Thu, Apr 29, 2021 at 12:51 AM Jan Lukavský <je...@seznam.cz> wrote:
>>>
>>>> Hi,
>>>>
>>>> I have come across a bug with timer output timestamp - when using event
>>>> time and relative timers, setting the timer can arbitrarily throw
>>>> IllegalArgumentException if the firing timestamp (input watermark) is
>>>> ahead of the output timestamp (like
>>>> .java.lang.IllegalArgumentException:
>>>> Attempted to set an event-time timer with an output timestamp of
>>>> 2021-04-29T07:16:19.369Z that is after the timer firing timestamp
>>>> -290308-12-21T19:59:05.225Z). But there is no way to access the firing
>>>> timestamp from user code. This means that the use has to either catch
>>>> the IllegalArgumentException, or not use this construct at all.
>>>>
>>>> Catching the exception should probably not be part of a contract, so we
>>>> should do one of the following:
>>>>
>>>> a) either throw the exception right away and disable using relative
>>>> timers with output timestamp completely, or
>>>>
>>>> b) support it correctly
>>>>
>>>> What is the actual reason not to support output timestamps, that are
>>>> ahead of firing timestamp? From my understanding, that should not be an
>>>> issue, because there is TimestampCombiner.EARLIEST on the
>>>> watermarkholdstate that corresponds to the output timestamp. If that is
>>>> correct can we simply remove the check?
>>>>
>>>> Jan
>>>>
>>>>
Re: Timer.withOutputTimestamp().offset().setRelative seems unusable
with event time
Posted by Jan Lukavský <je...@seznam.cz>.
Hm, one thing in the last paragraph seems there might be some logical gap.
> For a runner author to implement a "cleanup timer" requires a
different mechanism. A window expires when *both* the input element
watermark *and* the timer watermark are past the expiry time. In other
words, the cleanup timer fires according to the minimum of these
watermarks, combined. It *cannot* fire according to the input element
watermark. If you naively try to implement it as a user timer, it will
be incorrect. Incidentally this is why @OnWindowExpiration is a
meaningful feature.
The description describes a timer that fires not according to input
watermark, but according to the output watermark (once the output
watermark reaches certain point in time). That logically implies, that
such a timer cannot have non-droppable output (at least if its output
timestamp belongs to the respective window) and cannot create a
watermark hold (because that would block the progress of the output
watermark and might cause the timer to not fire ever). This maybe might
be a useful user-feature as well, probably again mostly related to how
user-code might want to deal with droppable data.
Jan
On 5/4/21 6:41 PM, Kenneth Knowles wrote:
> Mean to also add +Reuven Lax <ma...@google.com>
>
> On Tue, May 4, 2021 at 9:41 AM Kenneth Knowles <kenn@apache.org
> <ma...@apache.org>> wrote:
>
> Explicitly pinging a couple folks who were involved in the
> original change which yours essentially reverts. There's a model
> question here that I want to clarify on-list:
>
> When you have a ParDo setting timers, you have an additional
> watermark that must be considered:
>
> - input element watermark
> - output watermark
> - *(user) timer watermark*
>
> The timer watermark is an input to the ParDo. Sometimes you might
> think of the "timer channel" as a self loop, where each timer is
> an element. Each timer has a timestamp (the output timestamp) and
> separately some instructions on when to deliver that timer. This
> is the same as the usual difference between event time and
> processing time.
>
> The instruction on when to deliver a timer can have two forms:
>
> - wait a certain amount of processing time
> - deliver the timer when the *input element watermark* reaches a
> time X
>
> Here is an important point: "cleanup timers" are *not* user
> timers. They are an implementation detail. They are not part of
> the model. The runner's job is to reclaim resources as windows
> expire. A user should never be reasoning about how their timers
> relate to cleanup timers (except for resource consumption).
> Because there is no relationship except that the cleanup should
> happen "eventually" and invisibly.
>
> For a runner author to implement a "cleanup timer" requires a
> different mechanism. A window expires when *both* the input
> element watermark *and* the timer watermark are past the expiry
> time. In other words, the cleanup timer fires according to the
> minimum of these watermarks, combined. It *cannot* fire according
> to the input element watermark. If you naively try to implement it
> as a user timer, it will be incorrect. Incidentally this is
> why @OnWindowExpiration is a meaningful feature.
>
> Kenn
>
> On Tue, May 4, 2021 at 4:29 AM Jan Lukavský <je.ik@seznam.cz
> <ma...@seznam.cz>> wrote:
>
> Hi Kenn,
>
> I created BEAM-12276 [1] with PR [2].
>
> Jan
>
> [1] https://issues.apache.org/jira/browse/BEAM-12276
> <https://issues.apache.org/jira/browse/BEAM-12276>
>
> [2] https://github.com/apache/beam/pull/14718
> <https://github.com/apache/beam/pull/14718>
>
> On 5/3/21 7:46 PM, Kenneth Knowles wrote:
>> This seems like just a bug. If you set a timer for X and have
>> output timestamp Y where X < Y this should be fine. Is the
>> problem the current input watermark? Are you trying to set a
>> timer with output timestamp that is already past? I think
>> that should be allowed, too, as long as the window is not
>> expired, but I may be missing something.
>>
>> Some greater detail would be useful - maybe the full stack
>> trace and/or a failing unit test in a PR?
>>
>> Kenn
>>
>> On Thu, Apr 29, 2021 at 12:51 AM Jan Lukavský
>> <je.ik@seznam.cz <ma...@seznam.cz>> wrote:
>>
>> Hi,
>>
>> I have come across a bug with timer output timestamp -
>> when using event
>> time and relative timers, setting the timer can
>> arbitrarily throw
>> IllegalArgumentException if the firing timestamp (input
>> watermark) is
>> ahead of the output timestamp (like
>> .java.lang.IllegalArgumentException:
>> Attempted to set an event-time timer with an output
>> timestamp of
>> 2021-04-29T07:16:19.369Z that is after the timer firing
>> timestamp
>> -290308-12-21T19:59:05.225Z). But there is no way to
>> access the firing
>> timestamp from user code. This means that the use has to
>> either catch
>> the IllegalArgumentException, or not use this construct
>> at all.
>>
>> Catching the exception should probably not be part of a
>> contract, so we
>> should do one of the following:
>>
>> a) either throw the exception right away and disable
>> using relative
>> timers with output timestamp completely, or
>>
>> b) support it correctly
>>
>> What is the actual reason not to support output
>> timestamps, that are
>> ahead of firing timestamp? From my understanding, that
>> should not be an
>> issue, because there is TimestampCombiner.EARLIEST on the
>> watermarkholdstate that corresponds to the output
>> timestamp. If that is
>> correct can we simply remove the check?
>>
>> Jan
>>
Re: Timer.withOutputTimestamp().offset().setRelative seems unusable
with event time
Posted by Kenneth Knowles <ke...@apache.org>.
Mean to also add +Reuven Lax <re...@google.com>
On Tue, May 4, 2021 at 9:41 AM Kenneth Knowles <ke...@apache.org> wrote:
> Explicitly pinging a couple folks who were involved in the original change
> which yours essentially reverts. There's a model question here that I want
> to clarify on-list:
>
> When you have a ParDo setting timers, you have an additional watermark
> that must be considered:
>
> - input element watermark
> - output watermark
> - *(user) timer watermark*
>
> The timer watermark is an input to the ParDo. Sometimes you might think of
> the "timer channel" as a self loop, where each timer is an element. Each
> timer has a timestamp (the output timestamp) and separately some
> instructions on when to deliver that timer. This is the same as the usual
> difference between event time and processing time.
>
> The instruction on when to deliver a timer can have two forms:
>
> - wait a certain amount of processing time
> - deliver the timer when the *input element watermark* reaches a time X
>
> Here is an important point: "cleanup timers" are *not* user timers. They
> are an implementation detail. They are not part of the model. The runner's
> job is to reclaim resources as windows expire. A user should never be
> reasoning about how their timers relate to cleanup timers (except for
> resource consumption). Because there is no relationship except that the
> cleanup should happen "eventually" and invisibly.
>
> For a runner author to implement a "cleanup timer" requires a different
> mechanism. A window expires when *both* the input element watermark *and*
> the timer watermark are past the expiry time. In other words, the cleanup
> timer fires according to the minimum of these watermarks, combined. It
> *cannot* fire according to the input element watermark. If you naively try
> to implement it as a user timer, it will be incorrect. Incidentally this is
> why @OnWindowExpiration is a meaningful feature.
>
> Kenn
>
> On Tue, May 4, 2021 at 4:29 AM Jan Lukavský <je...@seznam.cz> wrote:
>
>> Hi Kenn,
>>
>> I created BEAM-12276 [1] with PR [2].
>>
>> Jan
>>
>> [1] https://issues.apache.org/jira/browse/BEAM-12276
>>
>> [2] https://github.com/apache/beam/pull/14718
>> On 5/3/21 7:46 PM, Kenneth Knowles wrote:
>>
>> This seems like just a bug. If you set a timer for X and have output
>> timestamp Y where X < Y this should be fine. Is the problem the current
>> input watermark? Are you trying to set a timer with output timestamp that
>> is already past? I think that should be allowed, too, as long as the window
>> is not expired, but I may be missing something.
>>
>> Some greater detail would be useful - maybe the full stack trace and/or a
>> failing unit test in a PR?
>>
>> Kenn
>>
>> On Thu, Apr 29, 2021 at 12:51 AM Jan Lukavský <je...@seznam.cz> wrote:
>>
>>> Hi,
>>>
>>> I have come across a bug with timer output timestamp - when using event
>>> time and relative timers, setting the timer can arbitrarily throw
>>> IllegalArgumentException if the firing timestamp (input watermark) is
>>> ahead of the output timestamp (like .java.lang.IllegalArgumentException:
>>> Attempted to set an event-time timer with an output timestamp of
>>> 2021-04-29T07:16:19.369Z that is after the timer firing timestamp
>>> -290308-12-21T19:59:05.225Z). But there is no way to access the firing
>>> timestamp from user code. This means that the use has to either catch
>>> the IllegalArgumentException, or not use this construct at all.
>>>
>>> Catching the exception should probably not be part of a contract, so we
>>> should do one of the following:
>>>
>>> a) either throw the exception right away and disable using relative
>>> timers with output timestamp completely, or
>>>
>>> b) support it correctly
>>>
>>> What is the actual reason not to support output timestamps, that are
>>> ahead of firing timestamp? From my understanding, that should not be an
>>> issue, because there is TimestampCombiner.EARLIEST on the
>>> watermarkholdstate that corresponds to the output timestamp. If that is
>>> correct can we simply remove the check?
>>>
>>> Jan
>>>
>>>
Re: Timer.withOutputTimestamp().offset().setRelative seems unusable
with event time
Posted by Kenneth Knowles <ke...@apache.org>.
Explicitly pinging a couple folks who were involved in the original change
which yours essentially reverts. There's a model question here that I want
to clarify on-list:
When you have a ParDo setting timers, you have an additional watermark that
must be considered:
- input element watermark
- output watermark
- *(user) timer watermark*
The timer watermark is an input to the ParDo. Sometimes you might think of
the "timer channel" as a self loop, where each timer is an element. Each
timer has a timestamp (the output timestamp) and separately some
instructions on when to deliver that timer. This is the same as the usual
difference between event time and processing time.
The instruction on when to deliver a timer can have two forms:
- wait a certain amount of processing time
- deliver the timer when the *input element watermark* reaches a time X
Here is an important point: "cleanup timers" are *not* user timers. They
are an implementation detail. They are not part of the model. The runner's
job is to reclaim resources as windows expire. A user should never be
reasoning about how their timers relate to cleanup timers (except for
resource consumption). Because there is no relationship except that the
cleanup should happen "eventually" and invisibly.
For a runner author to implement a "cleanup timer" requires a different
mechanism. A window expires when *both* the input element watermark *and*
the timer watermark are past the expiry time. In other words, the cleanup
timer fires according to the minimum of these watermarks, combined. It
*cannot* fire according to the input element watermark. If you naively try
to implement it as a user timer, it will be incorrect. Incidentally this is
why @OnWindowExpiration is a meaningful feature.
Kenn
On Tue, May 4, 2021 at 4:29 AM Jan Lukavský <je...@seznam.cz> wrote:
> Hi Kenn,
>
> I created BEAM-12276 [1] with PR [2].
>
> Jan
>
> [1] https://issues.apache.org/jira/browse/BEAM-12276
>
> [2] https://github.com/apache/beam/pull/14718
> On 5/3/21 7:46 PM, Kenneth Knowles wrote:
>
> This seems like just a bug. If you set a timer for X and have output
> timestamp Y where X < Y this should be fine. Is the problem the current
> input watermark? Are you trying to set a timer with output timestamp that
> is already past? I think that should be allowed, too, as long as the window
> is not expired, but I may be missing something.
>
> Some greater detail would be useful - maybe the full stack trace and/or a
> failing unit test in a PR?
>
> Kenn
>
> On Thu, Apr 29, 2021 at 12:51 AM Jan Lukavský <je...@seznam.cz> wrote:
>
>> Hi,
>>
>> I have come across a bug with timer output timestamp - when using event
>> time and relative timers, setting the timer can arbitrarily throw
>> IllegalArgumentException if the firing timestamp (input watermark) is
>> ahead of the output timestamp (like .java.lang.IllegalArgumentException:
>> Attempted to set an event-time timer with an output timestamp of
>> 2021-04-29T07:16:19.369Z that is after the timer firing timestamp
>> -290308-12-21T19:59:05.225Z). But there is no way to access the firing
>> timestamp from user code. This means that the use has to either catch
>> the IllegalArgumentException, or not use this construct at all.
>>
>> Catching the exception should probably not be part of a contract, so we
>> should do one of the following:
>>
>> a) either throw the exception right away and disable using relative
>> timers with output timestamp completely, or
>>
>> b) support it correctly
>>
>> What is the actual reason not to support output timestamps, that are
>> ahead of firing timestamp? From my understanding, that should not be an
>> issue, because there is TimestampCombiner.EARLIEST on the
>> watermarkholdstate that corresponds to the output timestamp. If that is
>> correct can we simply remove the check?
>>
>> Jan
>>
>>
Re: Timer.withOutputTimestamp().offset().setRelative seems unusable
with event time
Posted by Jan Lukavský <je...@seznam.cz>.
Hi Kenn,
I created BEAM-12276 [1] with PR [2].
Jan
[1] https://issues.apache.org/jira/browse/BEAM-12276
[2] https://github.com/apache/beam/pull/14718
On 5/3/21 7:46 PM, Kenneth Knowles wrote:
> This seems like just a bug. If you set a timer for X and have output
> timestamp Y where X < Y this should be fine. Is the problem the
> current input watermark? Are you trying to set a timer with output
> timestamp that is already past? I think that should be allowed, too,
> as long as the window is not expired, but I may be missing something.
>
> Some greater detail would be useful - maybe the full stack trace
> and/or a failing unit test in a PR?
>
> Kenn
>
> On Thu, Apr 29, 2021 at 12:51 AM Jan Lukavský <je.ik@seznam.cz
> <ma...@seznam.cz>> wrote:
>
> Hi,
>
> I have come across a bug with timer output timestamp - when using
> event
> time and relative timers, setting the timer can arbitrarily throw
> IllegalArgumentException if the firing timestamp (input watermark) is
> ahead of the output timestamp (like
> .java.lang.IllegalArgumentException:
> Attempted to set an event-time timer with an output timestamp of
> 2021-04-29T07:16:19.369Z that is after the timer firing timestamp
> -290308-12-21T19:59:05.225Z). But there is no way to access the
> firing
> timestamp from user code. This means that the use has to either catch
> the IllegalArgumentException, or not use this construct at all.
>
> Catching the exception should probably not be part of a contract,
> so we
> should do one of the following:
>
> a) either throw the exception right away and disable using relative
> timers with output timestamp completely, or
>
> b) support it correctly
>
> What is the actual reason not to support output timestamps, that are
> ahead of firing timestamp? From my understanding, that should not
> be an
> issue, because there is TimestampCombiner.EARLIEST on the
> watermarkholdstate that corresponds to the output timestamp. If
> that is
> correct can we simply remove the check?
>
> Jan
>