What are the use cases where you actually need to delete a timer? How about we only let users delete timers which they created themselves?
I guessing most of these use cases will be obsolete with the new Trigger DSL because the trigger logic can be expressed more easily. So +1 for removing the delete methods from the context. On Tue, Sep 27, 2016 at 3:43 PM, Kostas Kloudas <k.klou...@data-artisans.com> wrote: > Hi all, > > As the title of this email suggests, I am proposing to remove the methods > deleteProcessingTimeTimer(long time) and deleteEventTimeTimer(long time) > from the WindowOperator.Context. With this change, registered timers that > have nothing to do (e.g. because their state has already been cleaned up) > will be simply ignored by the windowOperator, when their time comes. > > The reason for the change is that by allowing custom user code, e.g. a custom > Trigger, > to delete timers we may have unpredictable behavior. > > As an example, one can imagine the case where we have allowed_lateness = 0 > and the cleanup > timer for a window collides with the end_of_window one. In this case, by > deleting the end_of_window > timer from the trigger (possibly a custom one), we end up also deleting the > cleanup one, > which in turn can lead to the window state never being garbage collected. > > To see what can be the consequences apart from memory leaks, this can easily > lead > to wrong session windows, as a session that should have been garbage > collected, will > still be around and ready to accept new data. > > With this change, timers that should correctly be deleted will now remain in > the queue of > pending timers, but they will do nothing, while cleanup timers will cleanup > the state of their > corresponding window. > > Other possible solutions like keeping a separate list for cleanup timers > would complicate > the codebase and also introduce memory overheads which can be avoided using > the > solution above (i.e. just ignoring timers the have nothing to do anymore). > > What do you think? > > Kostas >
