Closed
Bug 608422
Opened 14 years ago
Closed 14 years ago
cookies.sqlite-wal takes too much space for Fennec
Categories
(Core :: Networking: Cookies, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
fennec | 2.0b2+ | --- |
People
(Reporter: mbrubeck, Assigned: dwitte)
References
Details
Attachments
(2 files)
(deleted),
patch
|
sdwilsh
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
Using the latest nightly build of Fennec for Android, the cookies.sqlite-wal file in the profile directory grows to over 25 MB after about ten minutes of browsing to about a dozen different sites. After exiting Fennec, the file is removed and the storage is freed.
Because disk space is scarce on many mobile devices, we want to prevent this file from growing so large in Fennec.
Reporter | ||
Updated•14 years ago
|
tracking-fennec: --- → ?
Assignee | ||
Comment 1•14 years ago
|
||
Shawn, drh, any idea why this would grow so large? Cookie databases themselves are typically 1MB. Are there (pathological or otherwise) use patterns of sqlite where this is known to happen?
Comment 3•14 years ago
|
||
Got this from drh via e-mail:
The WAL file can fail to be truncated if a read transaction is being held
open. This could be due to a unfinalized statement, or it could happen if
there are multiple readers that overlap and there is never a moment when
nobody is reading the database.
The WAL file is truncated a checkpoint operation occurs while no reads are
taking place. If there is an active reader, then it is not possible for the
checkpoint to truncate the WAL since doing so would remove the WAL out from
under the reader.
Now, why is it 25MB, I don't know. Seeing the contents of that file would possibly be interesting.
In the future, please don't make the first action of a possible issue with SQLite be to cc the primary developer of SQLite. Most issues we see end up being bugs in our own code, and I'd hate to waste upstream's time. cc'ing me is generally sufficient.
Reporter | ||
Comment 4•14 years ago
|
||
(In reply to comment #3)
> Now, why is it 25MB, I don't know. Seeing the contents of that file would
> possibly be interesting.
cookies.sqlite-wal is 32MB in my Minefield profile after a day's use in the latest nightly. If you'd like me to send you the file, let me know (or I can generate one from a throwaway profile and post a link here). Or you can check your own profile to see if you see the same thing.
Assignee | ||
Comment 5•14 years ago
|
||
(In reply to comment #3)
> The WAL file can fail to be truncated if a read transaction is being held
> open. This could be due to a unfinalized statement, or it could happen if
> there are multiple readers that overlap and there is never a moment when
> nobody is reading the database.
Thanks -- I'll take a look at what's going on. It's most likely that we have a statement sticking around.
> In the future, please don't make the first action of a possible issue with
> SQLite be to cc the primary developer of SQLite. Most issues we see end up
> being bugs in our own code, and I'd hate to waste upstream's time. cc'ing me
> is generally sufficient.
I didn't at all mean to imply this was a sqlite issue -- I was merely asking what would lead to this behavior.
Assignee | ||
Comment 6•14 years ago
|
||
From http://www.sqlite.org/wal.html:
"The default strategy is to allow successive write transactions to grow the WAL until the WAL becomes about 1000 pages in size."
See also: http://mxr.mozilla.org/mozilla-central/source/storage/public/mozIStorageConnection.idl#72
So it's going to grow to 32MB. We need to use 'PRAGMA wal_autocheckpoint=N' to drop it down.
Assignee | ||
Comment 7•14 years ago
|
||
Patch forthcoming. Can you guys test Fennec with this and make sure it does what you want? WAL size should be limited to around 500 - 700K.
Assignee | ||
Comment 8•14 years ago
|
||
Assignee: nobody → dwitte
Status: NEW → ASSIGNED
Comment 9•14 years ago
|
||
after rebuilding with this patch, browsing around and restarting my cookies.sqlite-wal is down to 590kb
Updated•14 years ago
|
tracking-fennec: ? → 2.0b2+
Assignee | ||
Updated•14 years ago
|
Attachment #487140 -
Flags: review?(sdwilsh)
Reporter | ||
Comment 10•14 years ago
|
||
This patch is working for me too.
Comment 11•14 years ago
|
||
Comment on attachment 487140 [details] [diff] [review]
patch
>+ // Use write-ahead-logging for performance. We cap the autocheckpoint limit at
>+ // 16 pages (around 500KB).
> mDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING("PRAGMA journal_mode = WAL"));
>+ mDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
>+ "PRAGMA wal_autocheckpoint=16"));
nit: use a space around '=' like you do everywhere else
Can you also add a test so we don't accidentally regress this in the future? Shouldn't be too hard to check the WAL file's file size while doing a bunch of cookie operations.
r=sdwilsh
Attachment #487140 -
Flags: review?(sdwilsh) → review+
Comment 12•14 years ago
|
||
pushed http://hg.mozilla.org/mozilla-central/rev/97ea3fe9232a
dwitte said he'd follow up with a test
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 13•14 years ago
|
||
Pushed to Fennec 4.0b2 relbranch (GECKO20b7pre_20101029_RELBRANCH):
http://hg.mozilla.org/mozilla-central/rev/57bc64a3949f
Comment 14•14 years ago
|
||
(In reply to comment #12)
> pushed http://hg.mozilla.org/mozilla-central/rev/97ea3fe9232a
>
> dwitte said he'd follow up with a test
So do we have an ETA for the test? I understand you guys want to move fast, but my review was conditional on having a test.
Comment 15•14 years ago
|
||
(In reply to comment #14)
> (In reply to comment #12)
> > pushed http://hg.mozilla.org/mozilla-central/rev/97ea3fe9232a
> >
> > dwitte said he'd follow up with a test
> So do we have an ETA for the test? I understand you guys want to move fast,
> but my review was conditional on having a test.
Noted. We'll make sure the test lands soon.
Dan - someone from mobile can work on the test. Let me know if you have particular ideas for the test, otherwise, we'll add a simple file size check at the end of some cookie tests.
Assignee | ||
Comment 16•14 years ago
|
||
I'll write one today!
Assignee | ||
Comment 17•14 years ago
|
||
Attachment #487681 -
Flags: review?(mark.finkle)
Comment 18•14 years ago
|
||
Comment on attachment 487681 [details] [diff] [review]
test
Simple but effective. Not checking for exactly 500K is wise too. I'm sure there is some "play" in the values returned by some filesystems.
Attachment #487681 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 19•14 years ago
|
||
Landed the test. http://hg.mozilla.org/mozilla-central/rev/0b1dd08f8aa4
You need to log in
before you can comment on or make changes to this bug.
Description
•