[Django] #30904: Document caveats of MySQL SELECT FOR UPDATE when filtering against non-indexed columns
Django
indexed columns
-------------------------------------+-------------------------------------
Reporter: Simon | Owner: nobody
Charette |
Type: | Status: new
Cleanup/optimization |
Component: | Version: 2.2
Documentation | Keywords: mysql deadlock
Severity: Normal | select_for_update index unique
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
We recently discovered that MySQL (tested with MySQL 8) will acquire an
exclusive write lock when performing a `SELECT ... FOR UPDATE` filtered on
non-indexed columns.
This was highly unexpected as it's barely documented but it can easily
bring a database to its knees because of the heavy contention it causes.
Given the following model
{{{#!python
class Entry(models.Model):
account = models.IntegerField(db_index=True)
segment = models.IntegerField()
value = models.IntegerField()
}}}
And a view or a command that does
{{{#!python
with transaction.atomic():
entry = list(Entry.objects.filter(account=account,
segment__gt=threshold).select_for_update())
}}}
Will cause an exclusive write lock to be acquired on the full `entry`
table and not only on the row `WHERE account = account AND segment <
threshold` because `segment` is not indexed. This might seem like an
evidence here but because of how queryset accumulate filters and can be
passed around it's not that hard to shoot yourself in the foot.
I suggest we add a mention in the `FOR UPDATE`
[https://docs.djangoproject.com/en/dev/ref/databases/#row-locking-with-
queryset-select-for-update documentation] that goes along the following
lines
> Make sure you filter against at least set of fields contained in a
unique constraint or only against fields covered by indices on MySQL when
using `select_for_update` else an exclusive write lock will be acquired
over the full table for the duration of the transaction.
While a small admonition might not prevent this issue from happening it
will at least provide a description of the problem to those who encounter
a suddenly elevated number of deadlock on write attempts. If this is
deemed too niche to land in the documentation this ticket will at least
provide some search engine indexed background about a possible solution
instead of going on a wild goose chase like we did.
--
Ticket URL: <https://code.djangoproject.com/ticket/30904>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
indexed columns
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: 2.2
Severity: Normal | Resolution:
Keywords: mysql deadlock | Triage Stage: Accepted
select_for_update index unique |
Has patch: 0 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Nick Pope):
* needs_docs: 0 => 1
* easy: 0 => 1
* stage: Unreviewed => Accepted
Comment:
Seems a reasonable addition - I'm all for helping people avoid a
potentially obscure footgun like this.
--
Ticket URL: <https://code.djangoproject.com/ticket/30904#comment:1>
Django
indexed columns
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: 2.2
Severity: Normal | Resolution:
Keywords: mysql deadlock | Triage Stage: Accepted
select_for_update index unique |
Has patch: 0 | Needs documentation: 1
-------------------------------------+-------------------------------------
Comment (by felixxm):
I'm not sure to be honest. I don't think that we should add all databases'
caveats to the Django documentation.
--
Ticket URL: <https://code.djangoproject.com/ticket/30904#comment:2>
Django
indexed columns
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: 2.2
Severity: Normal | Resolution:
Keywords: mysql deadlock | Triage Stage: Accepted
select_for_update index unique |
Has patch: 0 | Needs documentation: 1
-------------------------------------+-------------------------------------
Comment (by felixxm):
We already have a MySQL section in this documentation, so I think it's
fine to add some clarification.
--
Ticket URL: <https://code.djangoproject.com/ticket/30904#comment:3>
Django
indexed columns
-------------------------------------+-------------------------------------
Type: | saadalsaad
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 2.2
Severity: Normal | Resolution:
Keywords: mysql deadlock | Triage Stage: Accepted
select_for_update index unique |
Has patch: 0 | Needs documentation: 1
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => saadalsaad
--
Ticket URL: <https://code.djangoproject.com/ticket/30904#comment:4>
Django
indexed columns
-------------------------------------+-------------------------------------
Type: | saadalsaad
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 2.2
Severity: Normal | Resolution:
Keywords: mysql deadlock | Triage Stage: Accepted
select_for_update index unique |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* needs_docs: 1 => 0
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/11972 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/30904#comment:5>
Django
indexed columns
-------------------------------------+-------------------------------------
Type: | saadalsaad
Component: Documentation | Version: 2.2
Severity: Normal | Resolution: fixed
Keywords: mysql deadlock | Triage Stage: Accepted
select_for_update index unique |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"fc2b1cc926e34041953738e58fa6ad3053059b22" fc2b1cc9]:
{{{
#!CommitTicketReference repository=""
revision="fc2b1cc926e34041953738e58fa6ad3053059b22"
Fixed #30904 -- Doc'd caveat about using filtered queryset with
select_for_update() on MySQL.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30904#comment:6>
Django
indexed columns
-------------------------------------+-------------------------------------
Type: | saadalsaad
Cleanup/optimization | Status: closed
Component: Documentation | Version: 2.2
Severity: Normal | Resolution: fixed
Keywords: mysql deadlock | Triage Stage: Accepted
select_for_update index unique |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"495cdd6add3abc37e2ff4e40c7df7e02eea4ce11" 495cdd6]:
{{{
#!CommitTicketReference repository=""
revision="495cdd6add3abc37e2ff4e40c7df7e02eea4ce11"
[3.0.x] Fixed #30904 -- Doc'd caveat about using filtered queryset with
select_for_update() on MySQL.
Backport of fc2b1cc926e34041953738e58fa6ad3053059b22 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30904#comment:7>
Django
indexed columns
-------------------------------------+-------------------------------------
Type: | saadalsaad
Cleanup/optimization | Status: closed
Component: Documentation | Version: 2.2
Severity: Normal | Resolution: fixed
Keywords: mysql deadlock | Triage Stage: Accepted
select_for_update index unique |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"fb77d53ffefc5c203e6cb93be71fd1154c2a884f" fb77d53]:
{{{
#!CommitTicketReference repository=""
revision="fb77d53ffefc5c203e6cb93be71fd1154c2a884f"
[2.2.x] Fixed #30904 -- Doc'd caveat about using filtered queryset with
select_for_update() on MySQL.
Backport of fc2b1cc926e34041953738e58fa6ad3053059b22 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30904#comment:8>