1658597 - Recovery mechanism for IndexedDB errors during sync?

Status: RESOLVED FIXED

(Firefox :: Remote Settings Client, enhancement, P3)

Description

[Mathieu Leplatre [:leplatrem]]

In Bug 1658574, we mentioned that in the case of low-level failure of IndexedDB collections cannot be updated anymore.
On read we introduced some fallback code, but during sync we don't do anything except from reporting Telemetry.

In case Bug 1658574 ends up being a wontfix, it may become relevant to have some kind of recovery mechanism in Remote Settings (delete database and resync?).

Comment 1

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

(In reply to Mathieu Leplatre [:leplatrem] from comment #0)

In case Bug 1658574 ends up being a wontfix, it may become relevant to have some kind of recovery mechanism in Remote Settings (delete database and resync?).

I've tried to provide more context in bug 1659464, but most pragmatically, yes, in the event of an UnknownError the database should be deleted and re-constructed. It might make sense to only do that once per browsing session, treating storage as broken if you find you want to do it twice. And when bug 1659458 is implemented (which is one of the easier things to do) it might make sense to move entirely to the fallback as long as QuotaManager says it is (edit: NOT) broken.

Comment 2

[Mathieu Leplatre [:leplatrem]]

We now have, since Bug 1732056, a proof that some users have consistently failing synchronization. See this Telemetry query for actual numbers.

I'm tempted to use this use this "broken sync" indicator to delete the local DB completely with deleteDatabase().

Andrew, do you think you could get to Bug 1659458 anytime soon?

Comment 3

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

I should say there's absolutely no harm in deleting the database if it's not working for you, and it should fix corruption/problems with that specific database.

Comment 4

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

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #3)

I should say there's absolutely no harm in deleting the database if it's not working for you, and it should fix corruption/problems with that specific database.

I should elaborate on this with more context:

• System-principaled QuotaManager users like remote-settings (whose databases live under PROFILE/storage/permanent/) are not subject to the same flavor of QuotaManager of breakage as code using a codebase principal (like web-extensions). While there are still potentially cases in which QM could be systemically broken, I believe they should be more rare.
• My suggestion about checking to make sure QuotaManager thinks it's operational first is mainly about avoiding a situation where nightly has a bad regression for a single build and even system-principaled QM has problems. In that case, the deletion would be data-lossy. But there's a good chance I guess the deletion might not even take in that case.
• Previously there was a potentially meaningful set of users whose profiles might have a broken QuotaManager. Thanks to a lot of hard work, any such set of profiles is now very small and ever-decreasing. But this was also really only about content (codebase) principals, not system principals. My apologies about creating/propagating the implication that remote settings would be as impacted by this as content (codebase) principals.

Comment 5

[Mathieu Leplatre [:leplatrem]]

Attachment: Bug 1658597 - Distinguish API errors from local errors r?gbeckley

Comment 6

[Mathieu Leplatre [:leplatrem]]

Attachment: Bug 1658597 - Destroy local DB when synchronization is broken r?mossop,barret

Depends on D147753

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/decd4d22a8fd
Distinguish API errors from local errors r=gbeckley
https://hg.mozilla.org/integration/autoland/rev/2a715807444c
Destroy local DB when synchronization is broken r=barret

Comment 8

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/decd4d22a8fd
https://hg.mozilla.org/mozilla-central/rev/2a715807444c

Comment 9

[Jan Varga [:janv]]

This seems to be addressed.