#30904 closed Cleanup/optimization (fixed)
Document caveats of MySQL SELECT FOR UPDATE when filtering against non-indexed columns
Reported by: | Simon Charette | Owned by: | saadalsaad |
---|---|---|---|
Component: | Documentation | Version: | 2.2 |
Severity: | Normal | Keywords: | mysql deadlock select_for_update index unique |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
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
class Entry(models.Model): account = models.IntegerField(db_index=True) segment = models.IntegerField() value = models.IntegerField()
And a view or a command that does
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
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.
Change History (8)
comment:1 by , 5 years ago
Easy pickings: | set |
---|---|
Needs documentation: | set |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 5 years ago
I'm not sure to be honest. I don't think that we should add all databases' caveats to the Django documentation.
comment:3 by , 5 years ago
We already have a MySQL section in this documentation, so I think it's fine to add some clarification.
comment:4 by , 5 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Seems a reasonable addition - I'm all for helping people avoid a potentially obscure footgun like this.