feat: improve watch by fetching only new/changed messages instead of fetching everything (Mirador)#43
Open
kylefeng28 wants to merge 1 commit intopimalaya:masterfrom
Open
feat: improve watch by fetching only new/changed messages instead of fetching everything (Mirador)#43kylefeng28 wants to merge 1 commit intopimalaya:masterfrom
kylefeng28 wants to merge 1 commit intopimalaya:masterfrom
Conversation
…nstead of fetching all messages (RFC 4549)
Member
|
Thanks for diving into this. Indeed I would advise to wait for the refactor and see how these changes could be integrated. Can't wait to try new IMAP lib inside Mirador and other mail-related CLIs 🙂 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR improves Mirador's
watchfeature by avoiding fetching all messages on every idle loop, but instead only fetching new messages and changed/expunged messages.@soywod I know you're pretty busy doing a major IO-free refactor of pimalaya/io-imap, so feel free to hold off on reviewing this until the refactor is done. (Just was excited to send this out since I was playing around with Mirador and finally got this working). I can rebase on the new code whenever it is released, looking forward 😁
(There's also some stuff I wasn't sure about, like if
exec_hooks()should handle changed messages or not, but maybe we can discuss that later as well.)This basically implements the basic sync algorithm from RFC 4549 - Synchronization Operations for Disconnected IMAP4 Clients, particularly particularly section 4.3.1. This doesn't make of the
CONDSTORE/CHANGEDSINCE/HIGHESTMODSEQfeatures directly (which would be the improved sync algorithm from section 6.1), but this should be an improvement beyond the existing Mirador watch feature which fetches all messages every loop:FETCH <lastseenuid+1>:*(where is the latest UID of the latest message fetched from the last sync)UID FETCH 1:<lastseenuid> (FLAGS)This should partially address some other issues related to Mirador watch functionality:
IDLEevents from the server and have fallback logic on the polling mechanismControl-Cbehavior since we're not waiting on a full fetch every timeNote: Even though though this PR doesn't make use of
CONDSTOREexplicitly, it seems that some IMAP servers (e.g. Gmail) will return both new and updated messages after aFETCH <lastseenuid+1>:*. I didn't see this documented in the RFC, but maybe this is some IMAP behavior related to the intersection of a persistent IMAP connection +IDLE+CONDSTORE?email_lib::imap::ImapClient:
fetch_flags()andfetch_flags_map()fetch_all_envelopes()to usefetch_all_envelopes("1:*")instead offetch_envelopes_by_sequence("1:*")to use UIDs instead of sequence IDs