Commit graph

15 commits

Author SHA1 Message Date
Juan Tejada
c22703bd6e [local-sync] Fix imap box status check
Summary:
When syncing folders, we check if the folder needs syncing by checking if it has any new messages via the STATUS command (STATUS returns uidnext, highestmodseq among others, and is cheaper than SELECT)

However, we can't issue a STATUS on a box that is already selected. Previously, if the box was already selected, we would just return it, but this was incorrect because we wouldn't get the latest box values (e.g. uidnext), causing us to think that there were no updates available, and skip syncing folders that actually needed to be synced.

Now, if the box is already selected when getting the status, we have to re select it to refresh the latest values

Test Plan: manual

Reviewers: evan, khamidou, spang

Reviewed By: spang

Differential Revision: https://phab.nylas.com/D3697
2017-01-15 16:45:16 -08:00
Evan Morikawa
fa6aec3cee [local-sync] fix lowerbound error where uid set could be zero 2017-01-15 15:35:58 -08:00
Juan Tejada
e2b317d09d [local-sync] Make all syncback tasks interrupt sync so they run fast
Summary:
This commit also lowers the batch size of messages to fetch on folder sync down to 30. This is in order to prevent sync from getting stuck if we queue too many syncback tasks-- given that we only update the range of fetched uids after we've actually fetched and processed messages, if the batch size is too big and we interrupt too often, we might end up never advancing the range and re fetching the same messages over and over.
This also makes the sync loop run faster through all folders in general.

Depends on D3689 to make sure that the batch size actually reflects a message count, i.e. to ensure that we are making /visible/ progress.

Test Plan: manual

Reviewers: spang, khamidou, evan

Reviewed By: evan

Maniphest Tasks: T7477

Differential Revision: https://phab.nylas.com/D3692
2017-01-15 15:07:49 -08:00
Christine Spang
d91992cdb8 [local-sync] Fetch at least the batch size of messages on each sync iteration
Summary:
Because we optimistically fetch UIDs by expanding a range without looking
at the actual UIDs in the inbox and the actual space of UIDs with messages
attached may be sparse due to message moves, we need to track how many
messages we actually download during a range expansion and continue
expanding the range if we haven't downloaded enough messages.

If we reach a large gap where we download no messages at all during a batch, we
pause and check the actual UID list for the folder for the next UID to
download, as otherwise we may spin indefinitely fetching UIDs that don't exist.
(Example: my "Deleted Items" folder had about 300k worth of empty UIDs between
a very small UID and a very large UID. With the new system, this registers as a
completed sync within a single iteration as soon as sync hits the gap.)

Test Plan: manual

Reviewers: juan, evan

Reviewed By: juan, evan

Differential Revision: https://phab.nylas.com/D3689
2017-01-15 14:28:24 -08:00
Karim Hamidou
aadc322ef4 [local-sync] Back off exponentially when getting a sync error
Summary:
This patch changes the sync worker to back off exponentially when there is an issue syncing an account. This has two goals:
- first, it's a bit dangerous to retry immediately. We don't want hundreds of thousands of machines trying to refresh tokens unsuccessfully because our service is struggling.
- second, it's nicer on the CPU to wait a bit between retries.

Currently, we sleep for at most 2 minutes, with some random jitter added.

Test Plan: Tested manually, stared at the code a long time.

Reviewers: evan, juan

Reviewed By: evan, juan

Differential Revision: https://phab.nylas.com/D3684
2017-01-15 11:57:47 -08:00
Evan Morikawa
9381b51746 [local-sync] fix fetch unsynced messages logic looking for min 2017-01-14 18:36:51 -08:00
Juan Tejada
50f00e5174 [local-sync] Fix order of execution in fetch-messages-in-folder 2017-01-14 17:43:42 -08:00
Evan Morikawa
ae981af646 [local-sync] fix performance of attribute changes
Summary:
On MG's machine this function is EXTREMELY non performant and causes
things like archive to lock up when the console is running here for some
reason. Not entirely sure exactly what's causing it, but there were some
simple DB cleanups that will make it faster for large queries.

There's likely other things involved since the sequelize DB being locked
up shouldn't affect the peformLocal of the edgehill db for things like
archive. Still looking into that

Test Plan: manual

Reviewers: juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3683
2017-01-14 17:27:15 -08:00
Juan Tejada
fc433dd6c6 [local-sync] Await instead of yielding when fetching the very first batch of messages in folder sync 2017-01-14 16:32:14 -08:00
Juan Tejada
f4f78f4449 [local-sync] Minor syntax fix 2017-01-14 16:22:53 -08:00
Juan Tejada
7b2e27b87b [local-sync] Skip unecessary folder syncs
Summary:
Before trying to sync a folder, check if we actually need to do so. This will prevent us from doing unnecessary work that slows down the sync loop (like performing SELECT commands)

We will perform a folder sync if any of the following are true
- The folder hasn't been completely synced
- There are new messages (using imap STATUS command)
- There are attribute changes indicated via highestmodseq (using imap STATUS command)
- If server doesn't support highestmodseq, it has passed enough time since we last ran an attribute scan on the folder.

Addresses T7513

Test Plan: manual

Reviewers: evan, halla, spang

Reviewed By: halla, spang

Differential Revision: https://phab.nylas.com/D3675
2017-01-14 15:56:17 -08:00
Christine Spang
ff9c2fd57c [local-sync] grr phab lost this update 2017-01-14 14:57:22 -08:00
Christine Spang
70a4b0fdcf [local-sync] Fetch min UID in each folder for use in sync state
Summary:
Currently, our mail sync strategy of expanding UID ranges from UIDNEXT
backwards until a UID of 1 implicitly assumes that every UID corresponds to an
actual message. This assumption is incorrect, and results in several
significant bugs regarding sync status.

This patch fixes issue 1:

Since UIDs are persistent and, so long as the UIDVALIDITY is valid, ascend
monotonically upward, every time you move a message to a new folder you "lose"
UIDs lower down in the range. In my work Inbox, where I get a lot of mail,
archive all the time, and generally have only a small number of threads in the
mailbox, the smallest UID is over 100k.  This means that, after all my inbox
messages are synced, the sync loop will continue attempting to download
nonexistent old messages in this mailbox for hundreds of sync iterations, and
will not mark the mailbox as fully synced until fetchmin reaches 1, regardless
of the fact that there are no actually messages being pulled down.

This patch needs a small associated patch to N1 to update how sync status is
calculated (coming soon).

The next patch in this series will deal with gaps in the UIDspace that slow
down syncing of a folder.

Test Plan: manual

Reviewers: halla, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3677
2017-01-14 14:52:35 -08:00
Juan Tejada
ffbdfb7fd2 [local-sync] fix build 2017-01-13 15:00:27 -08:00
Juan Tejada
1c37a8b788 [local-sync] Cleanup sync tasks to be more consistent with syncback tasks
Summary: While working on separating send out of the sync loop, I realized sync tasks could use some cleanup to be more consistent with how we implemented syncback tasks. I reorganized and renamed things a little bit. This will also help us move in the direction of the scheduler implementation under which everything is a task.

Test Plan: manual

Reviewers: evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3660
2017-01-13 12:30:43 -08:00
Renamed from packages/local-sync/src/local-sync-worker/imap/fetch-messages-in-folder.es6 (Browse further)