658305 - Use journal_size_limit on places.sqlite

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Alon Zakai (:azakai)]

See bug 625981 comment 66. This option will prevent the SQLite WAL file from staying too big for too long. We should use it when SQLite 3.7.7 is released and we update to it.

Comment 1

[Marco Bonardo [:mak]]

Attachment: patch v1.0

This applies:
- FULL checkpointing (see http://www.sqlite.org/c3ref/wal_checkpoint_v2.html) to ensure we don't dismiss checkpoints, causing the wal journal to grow without control
- journal_size_limit (see http://www.sqlite.org/pragma.html#pragma_journal_size_limit) to ensure that after a successful checkpoint, if the journal is too large, it will be shrinked

The final situation is that we should checkpoint when the journal exceeds 512KB, and truncate the journal if it exceeds 1.5MB

I'll now take a look at bug 658303 to evaluate eventual needs, but I think for places.sqlite it won't make much difference since we have a single writable connection.

Comment 2

[Dietrich Ayala (:dietrich)]

Comment on attachment 557340 [details] [diff] [review]
patch v1.0

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

looks a-ok, r=me

Comment 3

[Marco Bonardo [:mak]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/04545026b4ae

Comment 4

[Marco Bonardo [:mak]]

backed out due to strange crashes on shutdown in Reftests, exactly on threads shutdown. I suspect some blocking wal checkpoint happens really late. I'll split the patch and check both parts on Try.

Comment 5

[Marco Bonardo [:mak]]

Try is saying me that my suspects were wrong, using blocking checkpoints is fine, while using journal_size_limit causes the intermittent crash on shutdown.  I can only guess it may be a threading issue of some sort, where it may try to truncate the file on the just merged thread.  Will require deeper investigation.

Comment 6

[Marco Bonardo [:mak]]

I've tried reproducing the crash on Fedora 14 and Fedora 12, using a similar mozconfig as tinderboxes and running reftests, but I have been unable to.
Now I'm trying with PGO too (at that point my config should be identical to the tinderboxes), but I'm not sure it will change anything.

Comment 7

[Marco Bonardo [:mak]]

Attachment: patch v1.1

This is a reduced version that just sets the size limit, I prefer waiting a bit on the blocking wal for now, since even if it's true it's pretty smart, there is still some risk that a synchronous query may hit SQLITE_BUSY, the ideal situation is where everything is async, but we are not there yet. Will file that apart.

Looks like after recent Places changes the crash disappeared, at least on Try (with some fancy trick to get pgo builds). So, I think I'll push this and keep an eye on the tree for a backout in case Try missed something.

Comment 8

[Marco Bonardo [:mak]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/76a6531600d7

Comment 9

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/76a6531600d7