843005 - Disable Android SQLite locking in databases for REL 11 and higher

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Data Providers, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

Android SQLiteDatabase has it's own built-in locking mechanism, separate from SQLite's own mechanism. It can be controlled with SQLiteDatabase.setLockingEnabled(boolean):
http://developer.android.com/reference/android/database/sqlite/SQLiteDatabase.html#setLockingEnabled(boolean)

As of SDK16, this is a no-op.

Even before it became a no-op the WebViewDatabase turned it off:
http://androidxref.com/4.0.4/xref/frameworks/base/core/java/android/webkit/WebViewDatabase.java#254

There are other resources that talk about turning off the Android locking becuase it is expensive:
http://www.enterra-inc.com/techzone/handling_sql_issues/

Even the Android source code says it's expensive.

Since we should be accessing the DB from the same GeckoBackgroundThread everywhere, I think it might be worth trying to disable locking.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

I want to run with this patch a bit and make some Try builds to see if anything shakes loose.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch 2: only REL 11 or higher

This patch limits the change to 11 or higher, since we are using write-ahead logging.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

https://tbpl.mozilla.org/?tree=Try&rev=b1e04efe8f0c (only the Android 4.0 run is of any concern)

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 733868 [details] [diff] [review]
patch 2: only REL 11 or higher

Try is really green, even after retriggering the tests. Let's try this on nightly where the DB locking crash is #3 and see if we can affect it.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 733868 [details] [diff] [review]
patch 2: only REL 11 or higher

Review of attachment 733868 [details] [diff] [review]:
-----------------------------------------------------------------

Assuming the panda rc2 comes out green at https://tbpl.mozilla.org/?tree=Try&rev=b1e04efe8f0c then r=me

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Kats reminded me that rc2 has trouble on Android 4.0 and that I could land without waiting.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/07d20c5e4872

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/07d20c5e4872

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 733868 [details] [diff] [review]
patch 2: only REL 11 or higher

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: It's possible this could fix some DB locking failures we see on Android 15 (ICS). Android 16 and higher already no-op the SQL locking. This just does the same for 11 and higher.
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): Based on what we know, this should be low risk, but I honestly don't know. So far so good on m-c.
String or IDL/UUID changes made by this patch: none

I want to see if this has any affect on the beta crash stats. Landing early allows us to test and backout if it causes unexpected problems.

Comment 10

[bhavana bajaj [:bajaj]]

This may help mobile stability where we are having a number of DB related top-crashers in the past few cycles.We are still early in the beta cycle and the risk should be manageable .We still have a couple of opportunities to identify/fix issues or perform backout if needed .

Please land this asap so that this can make it in Beta 2 going to build today.

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/24650a47351c
https://hg.mozilla.org/releases/mozilla-beta/rev/f0810a07641a