Opened 7 years ago

Last modified 7 months ago

#29280 closed New feature

Fix for a class of SQLite "database is locked" problems — at Version 1

Reported by: Ove Kåven Owned by: nobody
Component: Database layer (models, ORM) Version: 2.0
Severity: Normal Keywords: sqlite, databases
Cc: Aymeric Augustin Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Ove Kåven)

I'd like to propose a change like this, which I think would fix a class of SQLite "database is locked" problems. But it's possible you want to add a config option, or an argument to transaction.atomic(), or something of the kind.

diff --git a/django/db/backends/sqlite3/base.py b/django/db/backends/sqlite3/base.py
index 3989028930..391a50789e 100644
--- a/django/db/backends/sqlite3/base.py
+++ b/django/db/backends/sqlite3/base.py
@@ -272,7 +272,7 @@ class DatabaseWrapper(BaseDatabaseWrapper):
         Staying in autocommit mode works around a bug of sqlite3 that breaks
         savepoints when autocommit is disabled.
         """
-        self.cursor().execute("BEGIN")
+        self.cursor().execute("BEGIN IMMEDIATE")
 
     def is_in_memory_db(self):
         return self.creation.is_in_memory_db(self.settings_dict['NAME'])

Explanation: Consider a type of transaction consisting of, for example, an update_or_create() call, which you run in a "with transaction.atomic()" context. Suppose two threads or processes run such a transaction at exactly the same time (but with different keys, doesn't matter, SQLite locks the whole database anyway).

  1. The current transaction.atomic() implementation results in a "BEGIN", which tells SQLite to start deferred transactions, i.e., to *not* acquire any locks before it has to.
  2. Normally, update_or_create would first do a "SELECT FOR UPDATE", but SQLite doesn't support that, so a plain "SELECT" is done. Both threads acquire a shared read lock, and so the SELECTs succeed for both threads.
  3. Next, update_or_create needs to do an INSERT or UPDATE (not important which), so the threads needs to upgrade the read locks to write locks. Unfortunately, only one thread can have the write lock.

Now, the designers of SQLite apparently realized that if you already have the read lock, then it's not a good idea to start a blocking wait on the write lock. If you wait while holding the read lock, then nobody will ever be able to acquire exclusive access to the database, and everything will just hang. On the other hand, if you drop the read lock and then wait, then you lose the 'serializable' isolation guarantee that SQLite tries to give. Hence, SQLite has only one choice.

  1. The thread that didn't get the write lock immediately gets a "database is locked" error. Its transaction is aborted.

The timeout mentioned in the Django documentation will have absolutely no effect on this, and it doesn't matter how short-lived your transactions are. It only matters that they do a read before they do a write.

I can see three possible solutions to this problem:

  1. "Don't do that". Don't use SQLite, or don't do concurrency. (Many answers on StackOverflow.com and such are essentially this.)
  2. Treat the "database is locked" as if it were a "serialization error", which it kind of is. That is, the app must retry the transaction until it commits. That works, but is somewhat unsatisfactory.
  3. Grab the write lock immediately (like "SELECT FOR UPDATE" would have, if SQLite had supported it). That's what "BEGIN IMMEDIATE", in the above patch, does. After all, if you're not holding the read lock when you try to grab the write lock, then this particular problem won't happen. (Of course, the lock timeout thing that's mentioned in the Django documentation can still happen if your transactions are too long-lived, but that's different.)

And I think that if you're using "with transaction.atomic()" in the first place, then you most likely want to write to the database, in which case there's not much reason not to always grab the write lock right away. But even if it's decided that some config option would be good, it probably shouldn't be too hard to add? Or?

Change History (1)

comment:1 by Ove Kåven, 7 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.
Back to Top