Summary:
Before this commit, we would just establish a single mail listener imap
connection and never check if we needed to re-connect it due to an
expired access token. Even though we correctly refreshed the access
token at the beggining of each sync loop (hidden under
`ensureSMTPConnection`), we would never re-establish the mail listener
connection with the new access token.
This would cause the app to enter an `Invalid Credentials` error loop in
the sync loop, preventing from syncing any mail at all (T8064)
Test Plan: manual
Reviewers: spang, halla, mark, evan
Reviewed By: mark, evan
Differential Revision: https://phab.nylas.com/D4338
Summary:
Given that we keep the imap mail listener conenction open outside of the
imap connection pool, it might close itself after a while or after
putting your computer to sleep.
Previously, if we had the connection object we just assumed it was
connected and proceeded with sync, and if it wasn't, we would fall into
an error loop in the sync worker preventing it from syncing at all (T8065)
To fix this, we just call `connect` every time to ensure the connection is open.
(This was the case before we introduced the connection pool)
Test Plan: manual
Reviewers: mark, spang, halla, evan
Reviewed By: halla, evan
Differential Revision: https://phab.nylas.com/D4334
Summary:
Previously we would always search all mail. Now, if the user has focused
a particular folder we will limit our search to that folder. The inbox
is an exception--it will always search all mail unless the user
explicitly uses an "in:" clause.
Test Plan: Run locally, verify that searching folders returns the correct results.
Reviewers: evan, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4328
Summary:
Check that there aren't any other categories with a role before we try
to assign it to the current category.
Addresses T7835
Test Plan: manual
Reviewers: juan, evan, mark
Reviewed By: mark
Differential Revision: https://phab.nylas.com/D4298
Summary:
Before this patch, the IdentityStore would initialize in our empty hot
window. However, hot windows don't receive any `action-bridge-message`s,
which include DB updates. Since the hot window loads first, it was with a
stale verison of the Identity. The main window fetches a fresh identity,
but that fresh update failed to get to new composers because the hot
window wasn't listening to changes to the DB.
This makes it such that the IdentityStore properly boots up when the
window props change.
Test Plan: manual
Reviewers: halla, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4327
Summary:
This adds feature limit modals (graphics pending) for reminders and send
later
Test Plan: manual
Reviewers: juan, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4322
Summary: This shaves off ~150ms trying to issue this query
Test Plan: manual
Reviewers: evan, spang, mark
Reviewed By: mark
Differential Revision: https://phab.nylas.com/D4317
Summary:
Bodies that already exist on K2 shouldn't be overridden.
See the comment for how this caused open/link tracking to fail in certain
cases when a lot of messages were being sent
Test Plan: manual
Reviewers: halla, mark, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4319
Summary:
We were using a join for the contact ranking query and for whatever
reason on large databases this was extremely slow in SQLite. This diff
splits the query into first finding the sent Folder/Label and then
searching for non-draft Messages in that category.
Test Plan: Run locally, verify the query is faster
Reviewers: juan, evan, spang
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D4315
Summary: It was really slow, and we no longer need it.
Test Plan: Run locally, verify that we no longer do that super slow query
Reviewers: evan, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4312
Summary:
Removes some VERY long running `ANLAYZE` queries. Was taking up to 50
seconds on my 9GB database on every boot
https://sqlite.org/pragma.html#pragma_optimize
Test Plan:
I tested to make sure the app still quits quickly. It does. The SQLite
docs also say this should be fast.
Reviewers: halla, spang, mark, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4314
Summary:
We were using a couple of left outer joins to avoid checking whether we
should be joining on Labels or Folders. We can greatly simplify the query
by just checking which we should use and issuing the correct inner join.
Test Plan: Run locally, verify contact ranking still works.
Reviewers: spang, juan, evan
Reviewed By: juan, evan
Differential Revision: https://phab.nylas.com/D4310
Summary:
This improves window open times by about 70ms
We would spend a very long time in the backend browser process building a
new hot window when we didn't need to do that until much later
Test Plan: manual
Reviewers: halla, mark, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4311
Summary:
Most of the times when removing a sync worker (i.e. when removing an
account), we would see database errors in the console. This happened
because more often than not we would interrupt in the middle of message
processing, but `processMessage` is not interruptible, which means that
the worker would be interrupted right after processing its current
message. However, if `processMessage` would take more than 500ms (the
current timeout for stopping the worker), we would destroy the database
before processing was done and it would throw a bunch of errors.
To fix this, we just don't set a timeout when removing the worker as a consequence
of removing an account. However, when we are removing the worker when we
detect that it is stuck, we set a time out of 5 seconds.
Test Plan: manually test removing accounts, verify that it doesn't error
Reviewers: mark, halla, evan
Reviewed By: halla, evan
Differential Revision: https://phab.nylas.com/D4308
Summary:
Previously, it was possible for the sync worker to continue running after being
interrupted, e.g. it would break out of `performSync` and then try to run
`onSyncCompleted`. This is fine if we were just interrupting to restart the loop,
but when we stop it we don't want it to continue running anything at all.
This also refactors the syncworker to have a single `destroy` method, which sets
a `destroyed` flag and uses that one exclusively instead of the `stopped` flag.
Test Plan: manually check it works
Reviewers: spang, halla, mark, evan
Reviewed By: mark, evan
Differential Revision: https://phab.nylas.com/D4307
Summary:
This prevents us from hitting the endpoint before the database for that
account is initialized and it's actually syncing.
Before this commit, we would throw errors upon adding an account
Test Plan: manually add, remove accounts, run benchmark script
Reviewers: mark, halla, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4305
Summary:
- Change files to .es6 so they recognize the syntax
- Pass arguments down to _run()
- Make sure the responses get returned
Test Plan: manual, even though that somehow failed before. :(
Reviewers: evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4309
Summary:
Creates a new `DatabaseStore.write` method alongside
`DatabaseStore.inTransaction` that doesn't always wrap trivial queries in
a transaction. SQLite does this automatically.
Test Plan: manual
Reviewers: spang, mark, halla, juan
Reviewed By: halla, juan
Differential Revision: https://phab.nylas.com/D4303
Summary: Renamed in prep for next few diffs
Test Plan: manual
Reviewers: juan, mark, spang, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4302
Summary:
Use the wonderful `debug` library instead of all our random flags.
You can now do things like: `DEBUG="app:*,sync:*" npm start` to print out
everything.
Test Plan: manual
Reviewers: mark, halla, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4297
Summary:
Report how long they take and if they're stopped (due to timeout).
Part of T7978
Test Plan: manual
Reviewers: evan, juan
Reviewed By: evan, juan
Differential Revision: https://phab.nylas.com/D4272
Summary: See title
Test Plan: Run locally, verify that attachments are returned
Reviewers: spang, evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4293
Summary:
Make `run()` functions generators and change most awaits to yields
Part of T7978
Test Plan: specs from D4269, but really needs some heavy QA
Reviewers: evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4270
Summary:
Interrupt retryable syncback tasks that are taking too long so that we can
return control to the sync loop. The sync loop will retry the task later.
This diff adds a `forceReject` param to `interrupt()` so that we can return
control immediately instead of waiting for the current operation to finish
(for instance, the syncback task could be stuck in an imap operation, and a
normal interrupt would still have to wait for that to finish before returning
control to the callee)
Part of T7978
Test Plan: specs
Reviewers: evan, spang, mark, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4269
Summary:
Every persistModel would trigger a large number of downstream updates.
These weren't necessary for thread indexing and causing a lot of
unnecessary DB thrashing. This adds a `silent` flag to `persistModel` and
its ilk that just does the write.
significantly improve performance, and also contribute to T8046
Test Plan: manual
Reviewers: spang, mark, halla, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4295
Summary:
This adds better logging to the DB
You can use `ENABLE_SEQUELIZE_DEBUG_LOGGING=true` and
`ENABLE_RXDB_DEBUG_LOGGING=true` to spit out the raw queries of both DBs.
Test Plan: manual
Reviewers: mark, halla, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4294
Summary:
We would mark modified threads for reindexing, the thread search indexer
would reindex them, which would trigger a notification that the thread
had been modified, so we would mark the thread for reindexing, ...
Now we keep track in memory of which threads we've marked for reindexing
so we avoid re-marking them when the update notification arrives later.
Test Plan:
Run locally, verify same threads aren't getting continuously
reindexed and that the in-memory set doesn't grow unboundedly
Reviewers: spang, juan, evan
Reviewed By: juan, evan
Differential Revision: https://phab.nylas.com/D4289
Summary:
This commit makes a few changes to how we log database queries:
- We log queries in 2 different places now:
- When `DatabaseStore._executeLocally` takes longer than 100ms. This might be due to the actual db operation taking long, or it might be due to our retry logic, or potentially other factors.
- When the actual raw database operation takes more than 100ms.
- When raw database queries take more than 100ms, we log out the query plan if it is a `SELECT` query and we are in dev mode.
- If `DEBUG_TO_LOG` is true, we always log all queries (including background queries)
Test Plan: manual
Reviewers: mark, halla, evan
Reviewed By: halla, evan
Differential Revision: https://phab.nylas.com/D4288
Summary:
- Rename `databaseAgent` usage to `background`, in the code and in the logs, which I
believe is less confusing. Often times found myself asking what was this agent thing in
the logs.
- Make messaging more explicit
Test Plan: manual
Reviewers: evan, mark, halla
Reviewed By: mark, halla
Differential Revision: https://phab.nylas.com/D4285
Summary:
This commit makes it so that when we retry database queries after
encountering a db locked error, we back off for at most 500ms, instead
of the current max of 3s. This meant that a single query could
potentially take 3+ seconds and block the transaction queue for that long.
Test Plan: manual
Reviewers: evan, mark, halla
Reviewed By: mark, halla
Differential Revision: https://phab.nylas.com/D4283
Summary:
After introducing retry logic, we would always defer db queries to the
next tick even if there was no retry delay present.
This would cause almost all db queries take more than 100ms even if the
actual db operation was really fast. This affected app performance
negatively
Test Plan: manual
Reviewers: evan, halla, mark
Reviewed By: halla, mark
Differential Revision: https://phab.nylas.com/D4282
Summary:
The problem actually was that we weren't applying the transform for Gmail
messages sent to 1 recipient
Test Plan: manual
Reviewers: halla, juan
Reviewed By: halla, juan
Differential Revision: https://phab.nylas.com/D4291
Summary:
We weren't stripping link and open items from messages we sent properly.
While we stripped it from the first outgoing message, when the sync loop
came back around it would have them back in. This adds it in the message
processor to extra ensure that ANYTHING that comes from us (or one of our
aliases) gets the open/link tracking stripped
Test Plan: New tests
Reviewers: juan, halla
Reviewed By: juan, halla
Differential Revision: https://phab.nylas.com/D4287
Summary:
Prior to this diff, the "in:" search query syntax didn't work for IMAP.
This diff implements "in:" by changing the IMAP search backend to take
folder context into account and emit the appropriate queries for each
folder. Queries that include "in:foo" will replace the corresponding AST
nodes with 'ALL' or 'NOT ALL' depending on whether or not the current folder
is "foo". We also now filter which folders we search based on which
folders are referenced in the query.
Test Plan: Run locally, verify that in: works quickly
Reviewers: evan, juan, spang
Reviewed By: juan, spang
Differential Revision: https://phab.nylas.com/D4284
