django.core.cache.backends.db has bad transaction handling
|Reported by:||Glenn||Owned by:||aaugustin|
|Component:||Core (Cache system)||Version:||master|
|Has patch:||no||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
(This problem is manifesting in core.cache, but the real problem is in db.transaction, so I've marked this as a DB problem.)
The database backend for caching handles transactions badly.
When inserting a cache key, it searches for an existing cache key. If found, it does an UPDATE. If not found, it does an INSERT. The INSERT case can lead to a constraint violation, if another thread inserts that key between the SELECT and the INSERT. The code expects this, and catches DatabaseError.
This rolls back the whole transaction, including anything the caller is doing. It needs a savepoint for this, so it only rolls back its own work.
To handle this sanely, transaction support really needs to require savepoints, so it can expose a helper to start and commit a nested transaction, eg. db.transaction.run_in_transaction(func):
- If a transaction is started already, start a savepoint; when finished, release the savepoint.
- If a transaction is not started, start one (typically a no-op due to non-autocommit); when finished, commit.
- On exception, rollback whichever it started.
Savepoints are supported by all production-quality databases (including MySQL, Postgres and SQLite), so requiring them in the backend seems reasonable. I've hit the need for nested transactions in Django so many times I had to write my own wrapper to implement this.
Change History (10)
comment:1 Changed 5 years ago by Alex
- Needs documentation unset
- Needs tests unset
- Patch needs improvement unset
- Triage Stage changed from Unreviewed to Design decision needed
Changed 2 years ago by gerdemb
comment:6 Changed 2 years ago by aaugustin
- Component changed from Database layer (models, ORM) to Core (Cache system)
- Triage Stage changed from Design decision needed to Accepted
comment:8 Changed 13 months ago by aaugustin
- Owner changed from nobody to aaugustin
- Status changed from new to assigned