classification
Title: sqlite3 module: Improved concurrency
Type: performance Stage:
Components: Versions: Python 2.6
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: ghaering Nosy List: amaury.forgeotdarc, ghaering, loewis
Priority: high Keywords: needs review

Created on 2008-09-12 13:57 by ghaering, last changed 2008-09-12 22:50 by ghaering. This issue is now closed.

Files
File name Uploaded Description Edit
sqlite_concurrency_prepare.diff ghaering, 2008-09-12 13:57
Messages (9)
msg73086 - (view) Author: Gerhard Häring (ghaering) * (Python committer) Date: 2008-09-12 13:57
I'd really like this change to get into Python 2.6. It's pretty trivial
(just releases the GIL when compiling SQLite statements), but improves
concurrency for SQLite. This means less "database is locked" errors for
our users.

Could somebody please review this and give me an ok to check it in?
msg73088 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-09-12 14:25
Your patch seems obvious, however:

- Is there the possibility that sqlite3_prepare() calls back into a
function in the _sqlite module or other python code? In this case the
GIL has to be re-acquired.

- What if another thread closes the current connection (or drop a table,
etc.) in-between? Is sqlite really thread-safe?
msg73092 - (view) Author: Gerhard Häring (ghaering) * (Python committer) Date: 2008-09-12 14:58
1. SQLite calling back

Good that you mention it. During sqlite3_prepare, SQLite can call the
authorizer_callback. Fortunately, it already acquires/releases the GIL
appropriately already.

2. Other thread closing connection, etc.

Connections/cursors cannot be shared among Python threads. Well, with
newer SQLite releases I think it is possible. But we have checks in
place that raise an error if you do it. These are the various calls to
pysqilte_check_thread in the code. If the table is dropped,
sqlite3_prepare will just raise an error about the statement not being
valid.

If, however, the table is dropped between sqlite3_prepare and
sqlite3_step, then the SQLITE_SCHEMA error code is returned. We then try
to recompile the statement (it might have just been an ALTER TABLE and
the statement is valid after compiling it again). If that still fails,
we promote the error to Python (cursor.c lines 605ff).
msg73121 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-09-12 19:04
Asking in the same direction: what is the consequence of this patch when
check_same_thread is False? Couldn't that result in very problematic
overlappings, e.g. when two threads try to execute statements on the
same cursor?
msg73124 - (view) Author: Gerhard Häring (ghaering) * (Python committer) Date: 2008-09-12 19:21
Interesting. I was smart enough to not document check_same_thread in the
sqlite3 incarnation of the module.

This has nothing to do with releasing/aquiring the GIL around
sqlite3_prepare, though. Adding the macros was just an oversight.
They're there for all other nontrivial SQLite API calls (sqlite3_step,
sqlite3_reset, sqlite3_open, sqlite3_finalize and also already for the
"BEGIN"/"COMMIT" and "ROLLBACK" versions of sqlite3_prepare in connection.c.
msg73125 - (view) Author: Gerhard Häring (ghaering) * (Python committer) Date: 2008-09-12 19:24
Just to be explicit: check_same_thread is unsupported and while it's
undocumented in sqlite3, the old pysqlite docs say that when you use it,
you have to make sure the connections/cursors are protected otherwise
(via your own mutexes).

[In the current pysqlite docs it's not documented at all at the moment,
because I derived these from the Python sqlite3 ones]
msg73127 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-09-12 19:38
> This has nothing to do with releasing/aquiring the GIL around
> sqlite3_prepare, though. 

You mean, it doesn't make things worse, but I believe they do
(even if so only slightly). If you have two threads simultaneously
attempting an execute on a cursor, one will run into the prepare.

That thread will (now) release the GIL, allowing the second thread
to run into the prepare. That thread will replace the statement object
in the cursor, so when the first thread returns, the statement has
changed underneath, and it will associate any return code with the
incorrect statement object.

In this form, this can't currently happen. Without detailed review,
I can readily believe that there are many other cases where a
False check_same_thread can give surprising results.

> They're there for all other nontrivial SQLite API calls (sqlite3_step,
> sqlite3_reset, sqlite3_open, sqlite3_finalize and also already for the
> "BEGIN"/"COMMIT" and "ROLLBACK" versions of sqlite3_prepare in connection.c.

On the grounds that the code is already full of these GIL release calls,
I'll approve this additional one.

I encourage you to review the entire issue, though, and document
somewhere what promises are made under what conditions. Then a later
review can validate that the promises are really kept.
msg73146 - (view) Author: Gerhard Häring (ghaering) * (Python committer) Date: 2008-09-12 22:35
Thanks, Martin.

Commited as r66414.
msg73150 - (view) Author: Gerhard Häring (ghaering) * (Python committer) Date: 2008-09-12 22:50
Issue3854 was created for documenting sqlite3 vs. threads.
History
Date User Action Args
2008-09-12 22:50:59ghaeringsetstatus: open -> closed
messages: + msg73150
2008-09-12 22:35:03ghaeringsetmessages: + msg73146
2008-09-12 19:40:45loewissetresolution: accepted
2008-09-12 19:38:30loewissetmessages: + msg73127
2008-09-12 19:24:46ghaeringsetmessages: + msg73125
2008-09-12 19:21:59ghaeringsetmessages: + msg73124
2008-09-12 19:04:16loewissetnosy: + loewis
messages: + msg73121
2008-09-12 14:58:32ghaeringsetmessages: + msg73092
2008-09-12 14:25:34amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg73088
2008-09-12 14:02:45ghaeringsetpriority: high
keywords: + needs review, - patch
2008-09-12 13:57:29ghaeringcreate