568742 - signons.sqlite should be opened with PRAGMA synchronous=FULL

Status: RESOLVED WONTFIX

(Toolkit :: Password Manager, defect, P5)

Description

[Justin Dolske [:Dolske]]

sdwilsh has mentioned this before, and I keep forgetting to file.

AIUI, we currently use synchronous=NORMAL, due to previous performance problems. Logins are critical data that doesn't change often, so going to synchronous=FULL shouldn't be a perf concern, and the extra reliability is a win. May want to do this for satchel as well?

Comment 1

[Shawn Wilsher :sdwilsh]

(In reply to comment #0)
> AIUI, we currently use synchronous=NORMAL, due to previous performance
> problems. Logins are critical data that doesn't change often, so going to
> synchronous=FULL shouldn't be a perf concern, and the extra reliability is a
> win. May want to do this for satchel as well?
Probably want to do this *after* you make these statements async?

Comment 2

[Justin Dolske [:Dolske]]

What's the impact of not waiting? I don't think the pwmgr implementation is going to become async any time soon, since the API it exposes is synchronous.

Comment 3

[Shawn Wilsher :sdwilsh]

(In reply to comment #2)
> What's the impact of not waiting? I don't think the pwmgr implementation is
> going to become async any time soon, since the API it exposes is synchronous.
Every transaction (implicit or explicit) does one additional fsync.

Comment 4

[Matthew N. [:MattN]]

We only import from signons.sqlite for desktop and instead use logins.json since bug 853549.

Comment 5

[Richard Newman [:rnewman]]

We're half-way through moving to a Java-side password store: <https://wiki.mozilla.org/Mobile/Projects/Java-side_replacement_for_nsILoginManager>. Bug 946857 is the first step.

Once that process is complete, the SQLite-backed nsILoginManagerStorage can go away, I think, fixing this bug and others.

Comment 6

[Sergey Galich [:serg]]

signons.sqlite is not used anymore