Bug 100274 - [Linux] SQLite databases are slow on filesystems with write barriers enabled
Summary: [Linux] SQLite databases are slow on filesystems with write barriers enabled
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords:
Depends on:
Blocks: 103890
  Show dependency treegraph
 
Reported: 2012-10-24 11:32 PDT by Zan Dobersek
Modified: 2016-02-02 06:30 PST (History)
6 users (show)

See Also:


Attachments
Patch (1.69 KB, patch)
2012-10-24 11:34 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2012-10-24 11:32:19 PDT
[Linux] SQLite databases are slow on filesystems with write barriers enabled
Comment 1 Zan Dobersek 2012-10-24 11:34:31 PDT
Created attachment 170440 [details]
Patch
Comment 2 Zan Dobersek 2012-10-24 11:35:02 PDT
(In reply to comment #0)
> [Linux] SQLite databases are slow on filesystems with write barriers enabled

Explained here:
https://bugs.webkit.org/show_bug.cgi?id=96862#c9
https://bugs.webkit.org/show_bug.cgi?id=96862#c11
Comment 3 Balazs Kelemen 2012-11-14 02:37:07 PST
*** Bug 101614 has been marked as a duplicate of this bug. ***
Comment 4 Balazs Kelemen 2012-11-14 02:38:32 PST
This will solve an annoying issue with WTR as well: bug 101614. Somebody please put an r+ on this! :)
Comment 5 Jocelyn Turcotte 2012-11-14 03:08:30 PST
This makes sense for testing but I don't think this is good for end users.

Wouldn't it be better to let the WTR injected bundle control this setting instead?
Comment 6 Balazs Kelemen 2012-11-14 04:13:31 PST
(In reply to comment #5)
> This makes sense for testing but I don't think this is good for end users.
> 
> Wouldn't it be better to let the WTR injected bundle control this setting instead?

In CookiJarQt we use this setting for end users as well. Synchronized mode is very slow and also it seems unstable so I would vote for turning it off in for users as well, not because it's easier to implement but because it is better for them.
Comment 7 Jocelyn Turcotte 2012-11-14 04:28:37 PST
(In reply to comment #6)
> In CookiJarQt we use this setting for end users as well. Synchronized mode is very slow and also it seems unstable so I would vote for turning it off in for users as well, not because it's easier to implement but because it is better for them.

I don't think it's right either for the cookie jar, we have no code dealing with the possibility of a corrupted databases and it _will_ happen out there somewhere.
Synchronized is set to normal by default for a reason, because it's better in most cases. No, it's not unstable. For a network cache DB I understand that it could make sense to disable it (no real data is lost), but for user data IMO it's wrong.

Tell my why it's better for them to risk having their data corrupted? In this case the user will most likely not even notice the delay added by fsync.
Comment 8 Balazs Kelemen 2012-11-14 05:34:31 PST
By unstability I was thinking about bug 101614. If the fsync can crash than it adds risk not reduce it. Actually we have code to deal with data corruption, see IconDatabase::checkIntegrity. Btw IconDatabase is also a cache so IMHO data loss is not fatal as long as we can handle it.
Comment 9 Jocelyn Turcotte 2012-11-14 06:02:16 PST
(In reply to comment #8)
> By unstability I was thinking about bug 101614. If the fsync can crash than it adds risk not reduce it.

With the data given on that bug, I have difficulties believing that this is a bug in SQLite. Could you reproduce it on any other machine than yours?

The real problem that we have to fix is that the tests take longer to run than they should.

> Actually we have code to deal with data corruption, see IconDatabase::checkIntegrity. Btw IconDatabase is also a cache so IMHO data loss is not fatal as long as we can handle it.

Right now we don't, not for CookiJarQt, and neither for other uses of SQLiteDatabase. If we handle it it's fine, but having users asking on forums and getting told that the solution to their obscure problem is to delete some ~/.local/someDatabase.db is what I would like to avoid.
Comment 10 Brady Eidson 2012-11-14 09:01:53 PST
(In reply to comment #8)
>  Btw IconDatabase is also a cache so IMHO data loss is not fatal as long as we can handle it.

I can assure you that users don't see the IconDatabase as a cache, and we will reject any attempt to treat it as such on our platform.
Comment 11 Brady Eidson 2012-11-14 09:03:01 PST
(In reply to comment #10)
> (In reply to comment #8)
> >  Btw IconDatabase is also a cache so IMHO data loss is not fatal as long as we can handle it.
> 
> I can assure you that users don't see the IconDatabase as a cache, and we will reject any attempt to treat it as such on our platform.

(I know this bug is only about Linux, I'm just warning anyone who - in the future - might think this type of change is solid cross platform)
Comment 12 Zan Dobersek 2012-12-03 03:14:11 PST
(Based on the loosely formed consensus that the 'problematic' behavior should only be avoided in testing.)

I was thinking of how to implement this and am currently holding an opinion that it would require complex paths taken that wouldn't justify the trade-off:
- a setting in an injected bundle that's then handled in WebKit2,
- a WebKit2-specific setting that is then communicated to WebCore,
- a new setting in WebCore::Settings interface that is not even reachable directly from SQLiteDatabase.

I'm at the moment in favor of putting information on wiki about disabling the write barriers in fstab.

Opinions?
Comment 13 Allan Sandfeld Jensen 2012-12-03 06:43:23 PST
(In reply to comment #12)
> (Based on the loosely formed consensus that the 'problematic' behavior should only be avoided in testing.)
> 
> I was thinking of how to implement this and am currently holding an opinion that it would require complex paths taken that wouldn't justify the trade-off:
> - a setting in an injected bundle that's then handled in WebKit2,
> - a WebKit2-specific setting that is then communicated to WebCore,
> - a new setting in WebCore::Settings interface that is not even reachable directly from SQLiteDatabase.
> 
> I'm at the moment in favor of putting information on wiki about disabling the write barriers in fstab.
> 
> Opinions?

I believe the Cookie-database would be better as an injected bundle, or fetched over  IPC. Having it hard-coded as an SQLite database has always been an too inflexible choice when we want QtWebKit to be a toolkit for a browser and not a browser in itself.
Comment 14 Zan Dobersek 2013-01-23 02:32:48 PST
(In reply to comment #13)
> (In reply to comment #12)
> > (Based on the loosely formed consensus that the 'problematic' behavior should only be avoided in testing.)
> > 
> > I was thinking of how to implement this and am currently holding an opinion that it would require complex paths taken that wouldn't justify the trade-off:
> > - a setting in an injected bundle that's then handled in WebKit2,
> > - a WebKit2-specific setting that is then communicated to WebCore,
> > - a new setting in WebCore::Settings interface that is not even reachable directly from SQLiteDatabase.
> > 
> > I'm at the moment in favor of putting information on wiki about disabling the write barriers in fstab.
> > 
> > Opinions?
> 
> I believe the Cookie-database would be better as an injected bundle, or fetched over  IPC. Having it hard-coded as an SQLite database has always been an too inflexible choice when we want QtWebKit to be a toolkit for a browser and not a browser in itself.

This bug is not really about how and where the cookie database should operate. Rather than that it tries to address the problem of write barriers being enabled on some filesystems and how to work around that.