Thanks Yun/Piotrek, Some brief comments inline below.
On Tue, Nov 22, 2022 at 1:37 AM Piotr Nowojski <pnowoj...@apache.org> wrote: > Hi, > > All in all I would agree with Dawid's proposal. +1 We can add the flexibility > of how to deal with the timers in the low level API via adding a handler - > if someone needs to customize it, he will always have a workaround. Note > after giving it more thought, I agree that registering some handlers is > better than overloading the register timer method and modifying the timer's > state. +1. > > > At the same time, we can force the most sensible semantic that we think for > the couple of built-in operators, which should be pretty straightforward > (either ignore the timers, or fire them at once). I agree there might be > some edge cases, that theoretically user might want to wait for the timer > to fire naturally, but: > 1. I'm not sure how common in practice this will be. If not at all, then > why should we be complicating the API/system? That’s fair. However, the specifics are very important here. One thing to think about - on EOF “trigger immediately” will mean that the asynchronous wait timeout timers will also fire - which is undesirable (because they are racing with the last async call). However, the issue is cleanly resolved by waiting for the timer to be canceled when the last event is processed. (“Wait for” case). Ignoring the timer has the least justification. Registering the handler as per Dawid’s proposal and having that handler unregister the timers on EOF makes best sense. This solution also unifies the trigger immediately case as that handler can reregister the timers for early termination. The proposal: 1. Operator receives EOF 2. EOF timer handler triggers 3. EOF handler adjusts the registered timers for early trigger or ignore. If wait-for behavior is desired, timers are not changed. This is controlled in client code. 4. Operator waits for all timers to drain/trigger. (“Always”). There is no special handling for ignore/early trigger. 5. Operator allows job to proceed with shutdown. The only api change needed is an EOF handler. The other agreement we need is that “Wait for” is the desired behavior in processing time and that processing time is fundamentally different from event time in this respect. (I have changed my thinking since the last mail). 2. We can always expand the API in the future, and let the user override > the default built-in behaviour of the operators via some setter on the > stream transformation (`SingleOutputStreamOperator`), or via some custom > API DSL style in each of the operators separately. This is not required. See above. > > Re forcing the same semantics for processing time timers as for event time > ones - this is tempting, but indeed I see a possibility that users need to > adhere to some external constraints when using processing time. +1. As above, we should consider the 2 cases fundamentally different in this area. Re: Yun - > b) Another issue is that what if users use timers with different > > termination actions in the same > > operator / UDF? For example, users use some kind of timeout (like throws > > exception if some thing > > not happen after some other thing), and also some kind of window > > aggregation logic. In this case, > > without additional tags, users might not be able to distinguish which > > timer should be canceled and > > which time should be triggered ? as above. The EOF handler makes the choice. > > > 4. How could these scenarios adjust their APIs ? > > From the current listed scenarios, I'm more tend to that as @Dawid > > pointed out, there might be only > > one expected behavior for each scenario, thus it does not seems to need > > to allow users to adjust the > > behavior. Thus @Divye may I have a double confirmation currently do we > > have explicit scenarios that > > is expected to change the different behaviors for the same scenario? Wait-for behavior is probably the only expected behavior and any alterations should be from the EOF handler managing the registered timers. > > > Besides @Divye from the listed scenarios, I have another concern for > > global configuration is that for > > one job, different operators seems to still might have different > expected > > behaviors. For example, A > > job using both Window operator and AsyncWaitOperator might have > different > > requirements for timers > > on termination? Thank you for raising this case. This changed my thinking. Based on your point, we should try and align on the “Wait-for” with EOF handler proposal. I’m withdrawing the “single-runtime-config” proposal. Best, Divye
