[RCD] keep-alive changes
thomas at roundcube.net
Tue May 1 13:49:50 CEST 2012
> Digging into session issues I've found this keep-alive thing a little
> bit overcomplicated and unclear. The keep-alive setting handles two
> things: session keeping and new-mail checking. Some plugins also bind to
> keep-alive to do some recurring actions (e.g. to display calendar
> alarms, etc.).
Right. The keep-alive is replaced in mail by check-recent in order to not
have two independently running requests which may cause race-conditions.
> I think we should split these functionalities. So keep-alive action
> should be used only for session keeping. In such case we need to execute
> keep-alive action only after some inactivity time (e.g. session_lifetime
> - 1min [or 5%]). It means timer should be resetted on every reequest.
> Also it means we don't need a separate config option, because the
> interval would be calculated from session_lifetime. For
> session_lifetime=0 there would be no keep-alive requests.
I agree that two config options are not really necessary. But keep-alive
has to run more frequently than just session_lifetime - 5% because it may
(which I eventually see happening when the browser tab is not active).
> Another thing is recurring check for new-mail. This would work as is,
> but... see below.
> Last but not least. We should provide some "global" recurring action
> which would work across all tasks so, plugins could bind to it. One
> "global" action to minimize session race-conditions when few plugins
> will create their own recurring requests on the same time. In mail task
> we could "bind" our check-recent action to this global action.
That's a reasonable suggestion and would certainly make the code clearer.
> In my opinion this would be a much clearer solution and would give as
> some additional possibilities:
> - possibility to disable recurring checks (for new mail etc.) at all,
> - better overall performance because keep-alive out of mail task will be
> executed less frequently
I'm not sure if this would run less frequently. It actually has to run with
the minimum interval of all recurring checks which need to be done.
> I think also, we could rename keep_alive option description ("Check for
> new messages on") into something like "Update system state (check for
> new mail, etc.) on", and move it to User Interface section, so we could
> use it as our global action interval.
"Update system state" is something no user understand but "Check for new
messages" certainly is. However, this is just a label but more important is
that it doesn't set the keep-alive interval (which is to be derived from
session_lifetime) but the actual check-recent inrerval.
I basically agree to you suggestions but let's see if the recent changes
help avoiding the session timeouts before we add more major changes to it.
More information about the dev