dovecot
storage: remove 500 ms debounce on IMAP IDLE notifications
This commit removes hardcoded 500 ms debounce from storage that delays all storage notification subscribers such as IDLE and NOTIFY commands.
500 ms debounce constant NOTIFY_DELAY_MSECS was added in 2009 [1]. Before that Dovecot was only delivering notifications when a second-resolution timer is changed by at least 1, so IDLE notifications were delayed by half a second on average.
[1] https://github.com/dovecot/core/commit/56fb5d09955b6097f77b341717fd9b70e9f13e7
I have made a patch that actually deletes 500 ms delay from mailbox-watch.c rather than reducing it to 1 ms.
I then looked into adding debounce to src/imap/cmd-idle.c, but there I cannot be sure that mailbox pointer passed to idle_callback() will not be freed by the time debounce timer expires. If we want to have debounce in IDLE, it should be kept in mailbox-watch.c, but reset via special API from cmd-idle.c every time IDLE is started.
Then I thought about the issue again and am pretty sure debounce is not needed for IDLE. If an offline client gets online and synchronizes a lot of flags, it usually can do it in one command. If it does not, because of bad implementation or because flags are different for different messages, it will send multiple commands. In this case IDLE-ing client will receive multiple updates, but this is not bad for performance. If IDLE-ing client does not ignore these updates and actually processes them, worst case it will immediately get busy processing the first incoming update. But by the time it finishes processing the first update it will receive many updates and process them in batch, effectively debouncing on the client even if there is no special code doing this.
So unless there is an IMAP client that is known to benefits from debouncing, I suggest that no debouncing is added for IDLE.
See related issue at https://github.com/deltachat/chatmail/issues/72 for the "chatmail" Postfix+Dovecot setup that needs reduced delay for chatting. There is a related thread on the Dovecot mailing list: https://dovecot.org/mailman3/archives/list/[email protected]/thread/J2L67F75QW5MJBIRKMBGA2AKNJHRC33X/
This is now deployed on https://c20.testrun.org/ account and works without problems.
The worry I have related to performance isn't about IMAP client behavior, but rather Dovecot's mailbox_sync() calls. With no delay IDLEing imap process would call mailbox_sync() every time there is a write to dovecot.index.log, whatever that reason is. If another sync is already running by another process, it would wait a bit for the exclusive lock, but otherwise it would be syncing as rapidly as there are .log file writes. Although syncing isn't necessarily expensive, it's not trivially cheap either. So this might increase the server's CPU usage. I'm not sure if that's more of a theoretical problem, but over the years I've grown to be wary of performance regressions, since they sometimes pop up rather unexpectedly.
Without measuring I also cannot tell how much it affects performance and have no idea how to benchmark Dovecot myself.
An option that was discussed in the mailing list was to notify immediately if started IDLE has not notified anything yet and add new APIs to mailbox-watch to reset the timer from IDLE implementation. I do not like this option because it would not work for NOTIFY extension which is only started once.
I have another proposal. Would it be possible to reset the debounce timer every time a MessageNew event is received? This will ensure that MUAs are notified about incoming messages immediately, but other changes to maildir or other storage like setting flags are still debounced. As long as the inbox is not constantly flooded with incoming messages, nothing will change for the server load. I have not looked in detail how to implement it, but imagine it should be possible to do the same as the notify plugin does to subscribe to "mail_save" events.
If nothing else works, we can still resort to adding a configuration option for this delay so we can at least configure it to 0. But most servers will not change the default though and have 500 ms delay, which is unfortunate, I would prefer to solve this for everyone so eventually all servers that are updated will have no delay.