Closed Bug 1162342 Opened 5 years ago Closed 5 years ago
Enable sqlite WAL journaling in Cache API
In bug 1160013 we see that Cache is very slow in the wpt cache-match.https.html tests on windows. These tests do a lot of write I/O to populate ~20 caches simultaneously. The tests are timing out on windows even with a 1 minute timeout. This is a fair amount of work and the disks on these machines are slow, but still that just sucks. We should enable WAL journaling in Cache. This will cut write I/O in about half and also reduce the number of fsyncs. Overall Cache should favor reading, though, so we should tune the WAL journal to be fairly small. (Large WAL journals penalize read performance.)
Work-in-progress patch. Unfortunately it currently hits some kind of QuotaObject assert if combined with the connection reuse patch in bug 1134671. I need to investigate whats going on.
Attachment #8602457 - Attachment is patch: true
The QuotaObject issue appears to be unique to bug 1134671. It has appeared over there without the WAL patch.
Enabling WAL transactions for the Cache API reduces the success times for cache-match.https.html from ~55s to ~20s on win7/win8. It reduces the run time on linux from ~5s to ~2s. There is also a releng bug to investigate why file system access is so slow on just win7 and win8 as well. See bug 1164464.
This is a more conservative change than what I reference in comment 3, but local testing shows that it still provides a significant speed up. I think this should fix the windows timeout issues in automation. https://treeherder.mozilla.org/#/jobs?repo=try&revision=5088e47f19b4 I expect to further tweak the WAL settings once I have a better benchmarking system setup.
Last try finally got far enough to show that win8 in automation is down from ~55s to ~30s with the reviewed patch. I just need to do one more patch to remove the WAL journal file when we wipe the db.
Actually, I just verified sqlite removes the -wal journal file when it opens a new empty database. So I'll just add a comment to that effect instead of explicitly removing it.
Comment on attachment 8605489 [details] [diff] [review] Enable sqlite WAL transactions in Cache API. r=ehsan Approval Request Comment [Feature/regressing bug #]: This fix is needed to silently persistent intermittents as tracked in bug 1160013. [User impact if declined]: Minimal end user impact. This is mainly to make life easier for the sheriffs. End users will see a slight speed up in the Cache API with this fix. [Describe test coverage new/current, TreeHerder]: Mochitests and web-platform-tests. [Risks and why]: Minimal. Its changing the journal mode on an existing sqlite database. No functional changes. [String/UUID change made/needed]: None.
Attachment #8605489 - Flags: approval-mozilla-aurora?
Comment on attachment 8605489 [details] [diff] [review] Enable sqlite WAL transactions in Cache API. r=ehsan Approved for aurora. Looks good on m-c. We want happy sheriffs.
Attachment #8605489 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.