Open Bug 1065487 Opened 5 years ago Updated 3 years ago

DBUtils.ensureDatabaseIsNotLocked is taking 103ms during first-run

Categories

(Firefox for Android :: Data Providers, defect)

All
Android
defect
Not set

Tracking

()

People

(Reporter: rnewman, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: perf)

Attachments

(1 file)

http://people.mozilla.org/~mfinkle/fennec/profiles/nexuss-home-startup-firstrun-20140910.trace

This probably isn't even limited to first run. Might be time to yank this band-aid.
Keywords: perf
Depends on: 1115075
Here is what the removal patch would look like, after Bug 1115075 confirms or refutes my theories.


We added this workaround in Bug 746444, with some cleanup and extension in Bug 741224 and Bug 749493.

There's some confusion about what "locked" means in the many bugs we have on file.

Bug 947939 and friends did a deeper analysis of lock errors, finding that mainly these were *internal* -- i.e., our concurrency model was screwed up, and we had two open connections. I'm fairly convinced that these are the main culprit.

It's still possible, as Lucas notes in Bug 746444 Comment 8, that there are dangling file locks after a crash.

The code in DBUtils attempts to avoid this by killing processes in a loop. As a side effect, it moves DB init very early in startup. And this will *only* fix dangling file locks.

It's not at all clear whether this works at all (we can end up with files that are locked even with no living process), or if it helps (e.g., if we relaunched, we might just be waiting for a process to die cleanly, so forcing it to die is no better than just waiting).
Attachment #8540863 - Flags: review?(mark.finkle)
Attachment #8540863 - Flags: review?(mark.finkle) → review+
The best way to repro this, it seems: introduce a DB upgrade that has a syntax error.

This causes a crash. The next launch will trigger the locked logic, dumping log statements from the system sqlite lib on each kill attempt.

Either the kill attempts are necessary, or the repeated log output indicates that the kills aren't working.

Either way: if we try those steps and things work, I think we're safe to strip out this method.
My current feeling is that this is still occasionally necessary. More research needed if motivated.
Assignee: rnewman → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.