Commit graph

273 commits

Author SHA1 Message Date
Evan Morikawa
a7bd1d66b7 [local-sync] decode quoted-printable encoded attachments
Summary:
Needed to stream process quoted-printable attachments
Fixes T7530

Test Plan: manual

Reviewers: juan, spang

Reviewed By: spang

Maniphest Tasks: T7530

Differential Revision: https://phab.nylas.com/D3690
2017-01-15 15:12:18 -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
078b015202 Fix unit test path. 2017-01-15 14:09:05 -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
Halla Moore
77ad25af24 [local-sync] Stop sync worker before deleting account database
Summary:
Various errors are thrown when the sync worker tries accessing
a database that we've already deleted, so make sure the sync
worker has been stopped before we remove the database. This diff
involves modifying `Interruptible` so that `interrupt()` returns
a promise that resolves once the interrupt has been completed.

Addresses T7472

Test Plan: manual

Reviewers: evan, juan

Reviewed By: evan, juan

Differential Revision: https://phab.nylas.com/D3679
2017-01-15 10:33:12 -08:00
Christine Spang
f06ba78d8a [local-sync] Update comment on _processExistingMessage 2017-01-15 10:06:52 -08:00
Christine Spang
9e426419c5 [local-sync] Correctly add references when processing existing messages 2017-01-15 10:04:29 -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
814581f3d0 [local-sync] 🎨 2017-01-14 17:23:43 -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
bda6a78ae1 [local-sync] Cleanup, use Provider constants 2017-01-14 14:31:51 -08:00
Christine Spang
941c564443 [local-sync] fix comment 2017-01-14 13:31:24 -08:00
Christine Spang
85a4e31980 [local-sync] canonicalize references 2017-01-14 13:31:16 -08:00
Evan Morikawa
cd1548ac26 [local-sync] fix duplicate ensure in sent folder 2017-01-13 19:31:15 -08:00
Juan Tejada
a5fec9e7a8 [local-sync] Separate sending tasks from the sync-loop
Summary:
We want to do this in order to prevent send tasks from blocking the sync loop given that they can take a very long time to run. This is especially true when sending emails with large attachments to multiple recipients.
There is no real way to make sending in these cases faster, but we can prevent it from blocking the sync loop at least, especially because sending is mostly I/O bound.

This is a bit messy actually, but should be fixed when we properly implement a sync scheduler

Also added a limit to the total size of attachments you can upload to try to prevent weird EPIPE errors when sending.
See: D3670.

Also moved and renamed stuff a little

Test Plan: manual

Reviewers: halla, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3669
2017-01-13 19:01:52 -08:00
Evan Morikawa
0f3f5c6ae2 [local-sync] fix reset accounts and data button
Summary: Allows us to reset accounts in local-sync too

Test Plan: manual

Reviewers: mark, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3672
2017-01-13 18:59:14 -08:00
Halla Moore
f4edc55752 [local-sync] Update oldestProcessedDate logic
Summary:
I happened to be testing between Jan 2017 and Dec 2016, so I
missed this logic flaw. Boo.

Test Plan: tested locally

Reviewers: evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3667
2017-01-13 15:42:23 -08:00
Halla Moore
b40ff948ca [local-sync] Make sure we're saving sent messages with 'SEEN' flag
Summary: We did this for gmail, but not for other providers.

Test Plan: tested locally

Reviewers: juan, spang

Reviewed By: spang

Differential Revision: https://phab.nylas.com/D3665
2017-01-13 15:03:29 -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
Halla Moore
30037f915d [local-sync] Add oldestProcessedDate to Folder.syncState
Summary:
Only updated within month precision. We can use this to show how
far back a folder has been synced.

Test Plan: tested locally

Reviewers: juan, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3662
2017-01-13 12:05:45 -08:00
Evan Morikawa
e197de86e0 [local-sync] fix attribute scanning in sync worker
Summary:
Fixes https://phab.nylas.com/T7435

The old deepScan (now `scanForAttributeChanges`) and shallowScan (now
`fetchLatestAttributeChanges`) had some fatal flaws.

If you deep scanned it would attempt to load the message attributes of all
messages ever and cause very bad memory leaks.

Also, if you left a mailbox running for a long time, there was a query
that would eventually run `Message.findAll` and, even though it was just
returning the headers, would still run insanely expensive operations

This fixes (and renames) these issues.

Test Plan: manual

Reviewers: spang, halla, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3657
2017-01-13 12:00:45 -08:00
Christine Spang
7c8146f81b [local-sync] Fix parsing of existing messages derp 2017-01-13 11:15:24 -08:00
Christine Spang
a23813a11c [local-sync] For generic IMAP, Thread based on Message-Id, In-Reply-To & References
Summary:
This swaps out our generic IMAP threading mechanism to use the threading
headers on the message instead of the prior way of grouping by subject
and then differentiating based on participants, as that design was
somewhat driven by what we could accomplish easily given legacy data
schema decisions and has serious caveats, such as different threads between
the same people with the same subject being misthreaded together. With K2, we
have free reign to change the data format, so we can do it right.

The algorithm is super simple:
- Define "references" as the union of the Message-Id, In-Reply-To, and
References headers on a message, filtered for valid RFC2822 Message-IDs
- On message sync, if any element of the new message's references
matches any element of an existing message's references, thread them
together

In order to accomplish this, we need to store References in a way that
allows each element to be indexed for fast lookup. That meant either
using the sqlite JSON1 extension + expression-based indices, or creating
a new table. I chose the latter as a time-tested and simple solution,
since we don't need the flexibility of JSON here.

Test Plan: manual - unit tests coming

Reviewers: khamidou, evan, juan

Reviewed By: evan, juan

Differential Revision: https://phab.nylas.com/D3651
2017-01-13 10:39:54 -08:00
Christine Spang
0dcbcee06c [local-sync] Messages generated for send should not be unread 2017-01-13 07:46:18 -08:00
Juan Tejada
238b79d06c [local-sync] Don't sync spam until everything else is synced
Summary: Addresses T7426

Test Plan: manual

Reviewers: evan, spang

Reviewed By: spang

Differential Revision: https://phab.nylas.com/D3655
2017-01-12 13:53:17 -08:00
Mark Hahnenberg
59b5961a6c [syncback] Reuse currently open box if possible
Summary:
Previously we would unconditionally issue a SELECT when openBox was
called. Now we check if the currently open box is the one we want first and
return immediately if it is, avoiding the unnecessary SELECT (which can be
quite expensive on large folders like INBOX). We were also calling closeBox
after iterating all the messages in a thread to mark them as read/unread.
This was unnecessary and was causing extra SELECTs to be issued. Now we don't!
This diff is a 5x speedup over the old behavior when marking lots of
threads in the same folder as read all at once.

Test Plan: Run locally, measure perf with log statements

Reviewers: evan, juan

Reviewed By: evan, juan

Differential Revision: https://phab.nylas.com/D3654
2017-01-12 13:16:07 -08:00
Mark Hahnenberg
6a03bbe73f [local-sync] Fix error thrown when Gmail returns no search results
Summary:
We were returning the wrong type in the case that we got no messages
back from the Gmail search API.

Test Plan: Run locally

Reviewers: evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3646
2017-01-12 11:57:07 -08:00
Jackie Luo
21049b2a1f Update Nylas N1 to Nylas Mail
Test Plan: Tested locally.

Reviewers: juan, evan

Reviewed By: juan, evan

Differential Revision: https://phab.nylas.com/D3644
2017-01-12 11:25:39 -08:00
Juan Tejada
ab7f994fef [local-sync] Ensure we get new mail ASAP via dedicated imap conn
Summary:
Add a new dedicated imap connection to listen for any updates or new mail on the inbox.
Previously, we wouldn't be able to receive new mail events on the inbox during the sync loop
because other mailboxes would be open while we sync them. This would cause big
delays in receiving new mail, especially if you have a lot of folders

Test Plan: manual

Reviewers: spang, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3650
2017-01-12 11:16:53 -08:00
Christine Spang
ed57422412 [local-sync] Enable toggling ENABLE_SEQUELIZE_DEBUG_LOGGING on launch via an env var
It's much more convenient to be able to turn this on/off without
editing the code.
2017-01-12 07:43:34 -08:00
Christine Spang
d36ae8db3c [local-sync] Still download messages when they have no body
There are plenty of valid use cases for sending subject-only emails,
and we also want to still download the headers and create message
objects if e.g. the email consists of only an event invitation or
an attachment.
2017-01-11 18:08:20 -08:00
Juan Tejada
3eb450e22c [local-sync] Make sure IMAPConnection times out correctly
Summary:
This commit passes down the `socketTimeout` option to node-imap. However, just
passing this option doesn't seem to work reliably, so this commit manually implements
the socketTimeout option for our IMAPConnection.

How it works is that basically every operation is wrapped with a timeout by
augmenting  the `createConnectionPromise` construct that already existed.

Test Plan:
Locally, tested by sleeping computer and turning off wifi. The
connection will successfully error and be restarted. It will reconnect when the
network is available again

Reviewers: khamidou, halla, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3642
2017-01-11 16:48:10 -08:00
Christine Spang
e1271cf25c [local-sync] Give Nylas Mail messages a different Message-ID from API messages
Summary:
We may find it useful at some point to be able to tell which messages in
a user's mailbox were sent using N1/the REST API vs Nylas Mail.

Test Plan: manual

Reviewers: evan, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3631
2017-01-11 15:51:12 -08:00
Karim Hamidou
9136b0592b [fix] Add a poor man's two-phase commit algorithm for syncback tasks.
Summary:
Sometimes things go very wrong when trying to syncback an action. For example, the worker window could crash, preventing us from marking the current task as failed. What's worse, another sync iteration could try to run the task which crashed, thinking it a new one. To prevent this, this diff adds a fourth syncback task state, `INPROGRESS`.

New syncback tasks are marked as `INPROGRESS` before being executed. When they complete we mark them as `SUCCEEDED/FAILED`. Stray `INPROGRESS` tasks are automatically marked as `FAILED` at the beginning of every sync iteration, to make sure we don't retry them again.

Test Plan: Tested manually.

Reviewers: evan, juan

Reviewed By: evan, juan

Differential Revision: https://phab.nylas.com/D3635
2017-01-11 12:04:58 -08:00
Evan Morikawa
47f5880d18 [local-sync] fix multiple sync loops starting
A bug allowed multiple sync loop processes to start. This could lead to
double sending and the sync loop appearing as thouogh it couldn't be
interrupted
2017-01-10 18:24:38 -08:00
Christine Spang
9eb1aef139 [local-sync] Properly handle multiple occurrences of From/To/Cc/Bcc headers
Summary:
Well-behaved MUAs don't do this, but it is totally legal and we should
handle it.

The Python sync engine handles this properly also:
7db949fec9/sync-engine/inbox/util/addr.py (L26-L47)

(Found this out while scratching my head over the output format of
mimelib.parseHeaders---turns out it's an array for a reason:

> let to = mimelib.parseHeaders(`To: Christine Spang <spang@nylas.com>
... To: Foo Bar <foo@example.com>`);
undefined
> to
{ to:
   [ 'Christine Spang <spang@nylas.com>',
     'Foo Bar <foo@example.com>' ] }

)

Test Plan: unit test included

Reviewers: halla, jackie, mark

Reviewed By: mark

Differential Revision: https://phab.nylas.com/D3625
2017-01-10 15:07:36 -08:00
Christine Spang
15ac9f795b [local-sync] Remove deprecated comment
With the latest update to N1's CSS, this is no longer true---though a theme
can easily override the font to monospace if so desired.
2017-01-10 13:39:54 -08:00
Juan Tejada
79006f692d [local-sync] Fix check for new mail updates on inbox folder 2017-01-10 12:19:15 -08:00
Juan Tejada
b74aad7314 [local-sync] Run send tasks before other syncback tasks
Summary: Update `_getNewSyncbackTasks` to return any send tasks first, and then others

Test Plan: locally

Reviewers: halla, evan

Reviewed By: halla, evan

Differential Revision: https://phab.nylas.com/D3627
2017-01-10 12:11:34 -08:00
Christine Spang
27b2c41fcc [local-sync] Make comment about Date/INTERNALDATE & message hashes better 2017-01-10 12:05:14 -08:00
Juan Tejada
80708dacbc [local-sync] Make syncback task execution interruptible
Summary:
See title. Got rid of that syncback-worker class which was kind of useless and
made things harder. My b.

Test Plan: locally

Reviewers: evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3624
2017-01-10 10:45:53 -08:00
Christine Spang
a234570118 [local-sync] Rename extract{Snippet,Contacts} in message-factory
Summary:
We have another function called extractContacts, in its own file,
extract-contacts.js, which is used to create Contact objects. This has
confused me a number of times and also leads to grep collisions.

This patch also makes the snippet unit tests pass again after a recent
API change.

Test Plan: included unit tests, manual

Reviewers: halla, evan, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3623
2017-01-10 10:41:35 -08:00