You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@beam.apache.org by Aljoscha Krettek <al...@apache.org> on 2017/05/08 10:26:25 UTC
[DISCUSS] Remove TimerInternals.deleteTimer(*) and Timer.cancel()
I wanted to bring this up before the First Stable release and see what other people think. The methods I’m talking about are:
void deleteTimer(StateNamespace namespace, String timerId, TimeDomain timeDomain);
@Deprecated
void deleteTimer(StateNamespace namespace, String timerId);
@Deprecated
void deleteTimer(TimerData timerKey);
The last two are slated for deletion. Notice that the first one doesn’t take the timestamp of the timer to delete, which implies that there is only one active timer per namespace/timerId/timeDomain.
These are my motivations for removal:
- Timer removal is difficult and has varying levels of support on different Runners and varying levels of cost.
- Removing the interface now and adding it back later is easy. Having it in the FSR and later removing it is quite hard.
I can only speak about the internal Flink implementation where Timers are on a Heap (Java Priority Queue). Here removal is quite expensive, see, for example this ML thread: https://lists.apache.org/thread.html/ac4d8e36360779a9b5047cf21303222980015720aac478e8cf632216@%3Cuser.flink.apache.org%3E. Especially this part:
I thought I would drop my opinion here maybe it is relevant.
We have used the Flink internal timer implementation in many of our production applications, this supports the Timer deletion but the deletion actually turned out to be a huge performance bottleneck because of the bad deletion performance of the Priority queue.
In many of our cases deletion could have been avoided by some more clever registration/firing logic and we also ended up completely avoiding deletion and instead using "tombstone" markers by setting a flag in the state which timers not to fire when they actually trigger.
Gyula
Note that in Flink it’s not possible to delete a timer if you don’t know the timestamp. Other systems might store timers in more elaborate data structures (hierarchical timer wheels come to mind) where it’s also hard to delete a timer without knowing the timestamp.
If we decide to keep timer removal in the user API there’s the possibility to simulate timer removal by keeping the timestamp of the currently active timer for a given timerID and checking a firing timer against that.
Best,
Aljoscha
Re: [DISCUSS] Remove TimerInternals.deleteTimer(*) and Timer.cancel()
Posted by Aljoscha Krettek <al...@apache.org>.
Thanks for taking care of that, Kenn!
> On 10. May 2017, at 05:45, Kenneth Knowles <kl...@google.com.INVALID> wrote:
>
> I think there's been adequate time for at least a quick comment like "hey,
> I use that [in Beam or similar] and I need it!"
>
> Filed https://issues.apache.org/jira/browse/BEAM-2245 and will address it
> immediately.
>
> On Mon, May 8, 2017 at 7:40 PM, JingsongLee <lz...@aliyun.com> wrote:
>
>> +1 to remove this, I have not encountered such a strong case.
>> best, JingsongLee
>>
>> ------------------------------------------------------------------
>> From:Kenneth Knowles <kl...@google.com.INVALID>
>> Time:2017 May 9 (Tue) 05:45
>> To:dev <de...@beam.apache.org>
>> Subject:Re: [DISCUSS] Remove TimerInternals.deleteTimer(*) and
>> Timer.cancel()
>> Interesting!
>>
>> I believe the only thing we need to change to remove it for FSR is
>> https://github.com/apache/beam/blob/master/sdks/java/core
>> /src/main/java/org/apache/beam/sdk/state/Timer.java#L60
>>
>> Here is how I might summarize the possibilities, at the risk of having
>> something quite wrong:
>>
>> - Cancel as-is: some runners must manage a tombstone and/or timestamp in
>> state or may choose to do so for performance.
>>
>> - Cancel requires a timestamp on the backend: runners must track the
>> timestamp in state in order to cancel. Some runners may already need to
>> track this for other reasons. For special cases like the end of the window
>> or GC time it can be guessed, but those aren't user timers.
>>
>> - Cancel requires a timestamp from the user: Really weird. IMO implies a
>> timer can be set multiple times and each one is independent. This is
>> actually an increase in capability in perhaps an interesting way, but I
>> thought it was too confusing. Also might have a wacky spec around the same
>> timer set multiple times for the same timestamp (probably fine/idempotent)
>>
>> Technically, timers are marked `@Experimental`. But, given the interest in
>> state and timers, making changes here will be very hard on users.
>>
>> Unless someone objects with a strong case, I am comfortable removing this
>> from userland and potentially adding it later if there is demand.
>>
>> Kenn
>>
>>
>> On Mon, May 8, 2017 at 3:26 AM, Aljoscha Krettek <al...@apache.org>
>> wrote:
>>
>>> I wanted to bring this up before the First Stable release and see what
>>> other people think. The methods I’m talking about are:
>>>
>>> void deleteTimer(StateNamespace namespace, String timerId, TimeDomain
>>> timeDomain);
>>>
>>> @Deprecated
>>> void deleteTimer(StateNamespace namespace, String timerId);
>>>
>>> @Deprecated
>>> void deleteTimer(TimerData timerKey);
>>>
>>> The last two are slated for deletion. Notice that the first one doesn’t
>>> take the timestamp of the timer to delete, which
>> implies that there is only
>>> one active timer per namespace/timerId/timeDomain.
>>>
>>> These are my motivations for removal:
>>> - Timer removal is difficult and has varying levels of support on
>>> different Runners and varying levels of cost.
>>> - Removing the interface now and adding it back later
>> is easy. Having it
>>> in the FSR and later removing it is quite hard.
>>>
>>> I can only speak about the internal Flink implementation
>> where Timers are
>>> on a Heap (Java Priority Queue). Here removal is quite
>> expensive, see, for
>>> example this ML thread: https://lists.apache.org/thread.html/
>>> ac4d8e36360779a9b5047cf21303222980015720aac478e8cf632216@%
>>> 3Cuser.flink.apache.org%3E. Especially this part:
>>>
>>> I thought I would drop my opinion here maybe it is relevant.
>>>
>>> We have used the Flink internal timer implementation in many of our
>>> production applications, this supports the Timer
>> deletion but the deletion
>>> actually turned out to be a huge performance bottleneck
>> because of the bad
>>> deletion performance of the Priority queue.
>>>
>>> In many of our cases deletion could have been
>> avoided by some more clever
>>> registration/firing logic and we also ended up
>> completely avoiding deletion
>>> and instead using "tombstone" markers by setting
>> a flag in the state which
>>> timers not to fire when they actually trigger.
>>>
>>> Gyula
>>>
>>> Note that in Flink it’s not possible to delete a timer if you don’t know
>>> the timestamp. Other systems might store timers in more elaborate data
>>> structures (hierarchical timer wheels come to mind)
>> where it’s also hard to
>>> delete a timer without knowing the timestamp.
>>>
>>> If we decide to keep timer removal in the user API there’
>> s the possibility
>>> to simulate timer removal by keeping the timestamp of
>> the currently active
>>> timer for a given timerID and checking a firing timer against that.
>>>
>>> Best,
>>> Aljoscha
>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
Re: [DISCUSS] Remove TimerInternals.deleteTimer(*) and Timer.cancel()
Posted by Kenneth Knowles <kl...@google.com.INVALID>.
I think there's been adequate time for at least a quick comment like "hey,
I use that [in Beam or similar] and I need it!"
Filed https://issues.apache.org/jira/browse/BEAM-2245 and will address it
immediately.
On Mon, May 8, 2017 at 7:40 PM, JingsongLee <lz...@aliyun.com> wrote:
> +1 to remove this, I have not encountered such a strong case.
> best, JingsongLee
>
> ------------------------------------------------------------------
> From:Kenneth Knowles <kl...@google.com.INVALID>
> Time:2017 May 9 (Tue) 05:45
> To:dev <de...@beam.apache.org>
> Subject:Re: [DISCUSS] Remove TimerInternals.deleteTimer(*) and
> Timer.cancel()
> Interesting!
>
> I believe the only thing we need to change to remove it for FSR is
> https://github.com/apache/beam/blob/master/sdks/java/core
> /src/main/java/org/apache/beam/sdk/state/Timer.java#L60
>
> Here is how I might summarize the possibilities, at the risk of having
> something quite wrong:
>
> - Cancel as-is: some runners must manage a tombstone and/or timestamp in
> state or may choose to do so for performance.
>
> - Cancel requires a timestamp on the backend: runners must track the
> timestamp in state in order to cancel. Some runners may already need to
> track this for other reasons. For special cases like the end of the window
> or GC time it can be guessed, but those aren't user timers.
>
> - Cancel requires a timestamp from the user: Really weird. IMO implies a
> timer can be set multiple times and each one is independent. This is
> actually an increase in capability in perhaps an interesting way, but I
> thought it was too confusing. Also might have a wacky spec around the same
> timer set multiple times for the same timestamp (probably fine/idempotent)
>
> Technically, timers are marked `@Experimental`. But, given the interest in
> state and timers, making changes here will be very hard on users.
>
> Unless someone objects with a strong case, I am comfortable removing this
> from userland and potentially adding it later if there is demand.
>
> Kenn
>
>
> On Mon, May 8, 2017 at 3:26 AM, Aljoscha Krettek <al...@apache.org>
> wrote:
>
> > I wanted to bring this up before the First Stable release and see what
> > other people think. The methods I’m talking about are:
> >
> > void deleteTimer(StateNamespace namespace, String timerId, TimeDomain
> > timeDomain);
> >
> > @Deprecated
> > void deleteTimer(StateNamespace namespace, String timerId);
> >
> > @Deprecated
> > void deleteTimer(TimerData timerKey);
> >
> > The last two are slated for deletion. Notice that the first one doesn’t
> > take the timestamp of the timer to delete, which
> implies that there is only
> > one active timer per namespace/timerId/timeDomain.
> >
> > These are my motivations for removal:
> > - Timer removal is difficult and has varying levels of support on
> > different Runners and varying levels of cost.
> > - Removing the interface now and adding it back later
> is easy. Having it
> > in the FSR and later removing it is quite hard.
> >
> > I can only speak about the internal Flink implementation
> where Timers are
> > on a Heap (Java Priority Queue). Here removal is quite
> expensive, see, for
> > example this ML thread: https://lists.apache.org/thread.html/
> > ac4d8e36360779a9b5047cf21303222980015720aac478e8cf632216@%
> > 3Cuser.flink.apache.org%3E. Especially this part:
> >
> > I thought I would drop my opinion here maybe it is relevant.
> >
> > We have used the Flink internal timer implementation in many of our
> > production applications, this supports the Timer
> deletion but the deletion
> > actually turned out to be a huge performance bottleneck
> because of the bad
> > deletion performance of the Priority queue.
> >
> > In many of our cases deletion could have been
> avoided by some more clever
> > registration/firing logic and we also ended up
> completely avoiding deletion
> > and instead using "tombstone" markers by setting
> a flag in the state which
> > timers not to fire when they actually trigger.
> >
> > Gyula
> >
> > Note that in Flink it’s not possible to delete a timer if you don’t know
> > the timestamp. Other systems might store timers in more elaborate data
> > structures (hierarchical timer wheels come to mind)
> where it’s also hard to
> > delete a timer without knowing the timestamp.
> >
> > If we decide to keep timer removal in the user API there’
> s the possibility
> > to simulate timer removal by keeping the timestamp of
> the currently active
> > timer for a given timerID and checking a firing timer against that.
> >
> > Best,
> > Aljoscha
> >
> >
> >
> >
> >
> >
>
>
Re: [DISCUSS] Remove TimerInternals.deleteTimer(*) and Timer.cancel()
Posted by JingsongLee <lz...@aliyun.com>.
+1 to remove this, I have not encountered such a strong case.
best, JingsongLee
------------------------------------------------------------------
From:Kenneth Knowles <kl...@google.com.INVALID>
Time:2017 May 9 (Tue) 05:45
To:dev <de...@beam.apache.org>
Subject:Re: [DISCUSS] Remove TimerInternals.deleteTimer(*) and Timer.cancel()
Interesting!
I believe the only thing we need to change to remove it for FSR is
https://github.com/apache/beam/blob/master/sdks/java/core
/src/main/java/org/apache/beam/sdk/state/Timer.java#L60
Here is how I might summarize the possibilities, at the risk of having
something quite wrong:
- Cancel as-is: some runners must manage a tombstone and/or timestamp in
state or may choose to do so for performance.
- Cancel requires a timestamp on the backend: runners must track the
timestamp in state in order to cancel. Some runners may already need to
track this for other reasons. For special cases like the end of the window
or GC time it can be guessed, but those aren't user timers.
- Cancel requires a timestamp from the user: Really weird. IMO implies a
timer can be set multiple times and each one is independent. This is
actually an increase in capability in perhaps an interesting way, but I
thought it was too confusing. Also might have a wacky spec around the same
timer set multiple times for the same timestamp (probably fine/idempotent)
Technically, timers are marked `@Experimental`. But, given the interest in
state and timers, making changes here will be very hard on users.
Unless someone objects with a strong case, I am comfortable removing this
from userland and potentially adding it later if there is demand.
Kenn
On Mon, May 8, 2017 at 3:26 AM, Aljoscha Krettek <al...@apache.org>
wrote:
> I wanted to bring this up before the First Stable release and see what
> other people think. The methods I’m talking about are:
>
> void deleteTimer(StateNamespace namespace, String timerId, TimeDomain
> timeDomain);
>
> @Deprecated
> void deleteTimer(StateNamespace namespace, String timerId);
>
> @Deprecated
> void deleteTimer(TimerData timerKey);
>
> The last two are slated for deletion. Notice that the first one doesn’t
> take the timestamp of the timer to delete, which implies that there is only
> one active timer per namespace/timerId/timeDomain.
>
> These are my motivations for removal:
> - Timer removal is difficult and has varying levels of support on
> different Runners and varying levels of cost.
> - Removing the interface now and adding it back later is easy. Having it
> in the FSR and later removing it is quite hard.
>
> I can only speak about the internal Flink implementation where Timers are
> on a Heap (Java Priority Queue). Here removal is quite expensive, see, for
> example this ML thread: https://lists.apache.org/thread.html/
> ac4d8e36360779a9b5047cf21303222980015720aac478e8cf632216@%
> 3Cuser.flink.apache.org%3E. Especially this part:
>
> I thought I would drop my opinion here maybe it is relevant.
>
> We have used the Flink internal timer implementation in many of our
> production applications, this supports the Timer deletion but the deletion
> actually turned out to be a huge performance bottleneck because of the bad
> deletion performance of the Priority queue.
>
> In many of our cases deletion could have been avoided by some more clever
> registration/firing logic and we also ended up completely avoiding deletion
> and instead using "tombstone" markers by setting a flag in the state which
> timers not to fire when they actually trigger.
>
> Gyula
>
> Note that in Flink it’s not possible to delete a timer if you don’t know
> the timestamp. Other systems might store timers in more elaborate data
> structures (hierarchical timer wheels come to mind) where it’s also hard to
> delete a timer without knowing the timestamp.
>
> If we decide to keep timer removal in the user API there’s the possibility
> to simulate timer removal by keeping the timestamp of the currently active
> timer for a given timerID and checking a firing timer against that.
>
> Best,
> Aljoscha
>
>
>
>
>
>
Re: [DISCUSS] Remove TimerInternals.deleteTimer(*) and Timer.cancel()
Posted by Kenneth Knowles <kl...@google.com.INVALID>.
Interesting!
I believe the only thing we need to change to remove it for FSR is
https://github.com/apache/beam/blob/master/sdks/java/core
/src/main/java/org/apache/beam/sdk/state/Timer.java#L60
Here is how I might summarize the possibilities, at the risk of having
something quite wrong:
- Cancel as-is: some runners must manage a tombstone and/or timestamp in
state or may choose to do so for performance.
- Cancel requires a timestamp on the backend: runners must track the
timestamp in state in order to cancel. Some runners may already need to
track this for other reasons. For special cases like the end of the window
or GC time it can be guessed, but those aren't user timers.
- Cancel requires a timestamp from the user: Really weird. IMO implies a
timer can be set multiple times and each one is independent. This is
actually an increase in capability in perhaps an interesting way, but I
thought it was too confusing. Also might have a wacky spec around the same
timer set multiple times for the same timestamp (probably fine/idempotent)
Technically, timers are marked `@Experimental`. But, given the interest in
state and timers, making changes here will be very hard on users.
Unless someone objects with a strong case, I am comfortable removing this
from userland and potentially adding it later if there is demand.
Kenn
On Mon, May 8, 2017 at 3:26 AM, Aljoscha Krettek <al...@apache.org>
wrote:
> I wanted to bring this up before the First Stable release and see what
> other people think. The methods I’m talking about are:
>
> void deleteTimer(StateNamespace namespace, String timerId, TimeDomain
> timeDomain);
>
> @Deprecated
> void deleteTimer(StateNamespace namespace, String timerId);
>
> @Deprecated
> void deleteTimer(TimerData timerKey);
>
> The last two are slated for deletion. Notice that the first one doesn’t
> take the timestamp of the timer to delete, which implies that there is only
> one active timer per namespace/timerId/timeDomain.
>
> These are my motivations for removal:
> - Timer removal is difficult and has varying levels of support on
> different Runners and varying levels of cost.
> - Removing the interface now and adding it back later is easy. Having it
> in the FSR and later removing it is quite hard.
>
> I can only speak about the internal Flink implementation where Timers are
> on a Heap (Java Priority Queue). Here removal is quite expensive, see, for
> example this ML thread: https://lists.apache.org/thread.html/
> ac4d8e36360779a9b5047cf21303222980015720aac478e8cf632216@%
> 3Cuser.flink.apache.org%3E. Especially this part:
>
> I thought I would drop my opinion here maybe it is relevant.
>
> We have used the Flink internal timer implementation in many of our
> production applications, this supports the Timer deletion but the deletion
> actually turned out to be a huge performance bottleneck because of the bad
> deletion performance of the Priority queue.
>
> In many of our cases deletion could have been avoided by some more clever
> registration/firing logic and we also ended up completely avoiding deletion
> and instead using "tombstone" markers by setting a flag in the state which
> timers not to fire when they actually trigger.
>
> Gyula
>
> Note that in Flink it’s not possible to delete a timer if you don’t know
> the timestamp. Other systems might store timers in more elaborate data
> structures (hierarchical timer wheels come to mind) where it’s also hard to
> delete a timer without knowing the timestamp.
>
> If we decide to keep timer removal in the user API there’s the possibility
> to simulate timer removal by keeping the timestamp of the currently active
> timer for a given timerID and checking a firing timer against that.
>
> Best,
> Aljoscha
>
>
>
>
>
>