Bug 196827 - Take an assertion if there is open database connection
Summary: Take an assertion if there is open database connection
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-04-11 12:42 PDT by Sihui Liu
Modified: 2019-05-20 08:56 PDT (History)
4 users (show)

See Also:


Attachments
Patch (31.12 KB, patch)
2019-04-11 13:04 PDT, Sihui Liu
cdumez: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sihui Liu 2019-04-11 12:42:54 PDT
Currently network process and web process ask for an assertion when transactionInProgressCount is not zero, and this value is changed in many places where there is a database activity on an open database connection. This could be error-prone and slow. Why don't we just count the open databases instead of running database operations?
Comment 1 Sihui Liu 2019-04-11 13:04:41 PDT
Created attachment 367234 [details]
Patch
Comment 2 Chris Dumez 2019-04-11 13:05:31 PDT
My understanding was that it is OK to suspend while databases are open. It is only not OK when there are pending transactions because the database file is locked when there is an operation running.

I believe holding an assertion whenever there is an open database would be a significant power regression.
Comment 3 Chris Dumez 2019-04-11 13:06:22 PDT
Comment on attachment 367234 [details]
Patch

r- due to my earlier comments. If my understanding is wrong somehow, let me know.
Comment 4 Sihui Liu 2019-04-11 13:29:16 PDT
(In reply to Chris Dumez from comment #2)
> My understanding was that it is OK to suspend while databases are open. It
> is only not OK when there are pending transactions because the database file
> is locked when there is an operation running.
> 
> I believe holding an assertion whenever there is an open database would be a
> significant power regression.

I thought about this. If this is the case, why do we closeAllDatabases when ProcessWillSuspendImminently? We could just let the running transaction be finished or interrupted?

Also, if this causes big power regression, shouldn't we find all the places that could have database connection open for a long time but not actually doing anything and close them? I was told we are not supposed to keep open database connection when there is no database activity.
Comment 5 Brady Eidson 2019-04-11 15:19:34 PDT
(In reply to Chris Dumez from comment #2)
> My understanding was that it is OK to suspend while databases are open. 

Actually simply having an open sqlite3* connection that isn't doing thing makes it so we cannot suspend. *sigh*

That said, I agree with your power observation here and Sihui and I discussed a good way forward.