888955 - [email/backend] Enhance account migration/upgrade logic to avoid potential data-loss

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[Andrew Sutherland [:asuth] (he/him)]

See https://bugzilla.mozilla.org/show_bug.cgi?id=887700#c9 for some analysis, but basically:

Potential for data-loss:
- Our database migration destroys the database in a separate IndexedDB transaction from when we re-populate the list of accounts.
- Our database migration logic will discard local drafts.
- Our database migration will discard enqueued operations like moving messages or altering message flags


Potential for inefficency:
- We do throw away downloaded state.  Although usually, if we want to change the database, there is some type of problem with our state where we need to re-get information from the server anyways.


I think the key thing we'll want for the next time we rev our schema is to eliminate the data-loss cases.


Key notes:

- Because of the IndexedDB transaction model and the fact that our accounts are loaded on-demand, we will either need to abort the upgrade transaction by throwing an exception, or we will need to track our actual database version slightly independently of the IndexedDB schema revision.

- Along the lines of the previous; it might make sense for us to track something like a 'revision' number on a per-account basis.  As part of account startup/loading, the account would check its current revision number.  If it is behind the times, it would apply whatever steps are required to bring itself up-to-date, and then only indicate load success once it has completed this work, coincidentally updating its state in the DB at the same time.

This would allow for more dramatic things like purging the contents of all FolderStorages, or more benign, small changes, like applying small fix-ups to existing operations, etc.

Comment 1

[Alex Wissmann [:awiss]]

Attachment: GELAM patch

I've added in all of the upgrade logic, as well as tests. However, it is still contained on the same branch as the unread messages code since the upgrade logic itself is related to that.

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8469612 [details] [review]
GELAM patch

Imminently there!  I'm really happy with the level of tests we'll have with the fix, thank you!  I think we're good to go once you address the just-added feedback.

Github seems to think a clean merge isn't possible, so I fear you'll need to rebase your patch series off of gelam/master for your next round of fixes.  I'd appreciate it if you could keep your next set of fixes as a single commit on top of what's already there, but feel free to squash everything that precedes the new commit.  (And feel free to squash any new stuff into a single commit prior to re-requesting review.)

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

landed on gaia-email-libs-and-more/master:
(note, different PR for expediency, all commits correctly attributed, etc)
https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/328
https://github.com/mozilla-b2g/gaia-email-libs-and-more/commit/9a185f67d55de1301c200f54e7fe3bb1d742cd33

landed in gaia/master:
https://github.com/mozilla-b2g/gaia/pull/23058
https://github.com/mozilla-b2g/gaia/commit/9d60607435e6dc6aec5fedfabc50a2dfdc4c877a