[Django] #32590: Add option to avoid subtransactions by default when using transaction.atomic()
Django
transaction.atomic()
-------------------------------------+-------------------------------------
Reporter: Alex | Owner: nobody
Rattray |
Type: | Status: new
Uncategorized |
Component: Database | Version: 3.1
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Inspired by [https://twitter.com/jacobian/status/1374492590830096387
@jacobian on twitter]
> I totally co-sign the “subtransactions are cursed” takeaway. I kinda
wish Django had an option to fail loudly if you made nested atomic()
calls; I’ve been bitten by that more than once.
This is in reference to [https://buttondown.email/nelhage/archive/notes-
on-some-postgresql-implementation-details/ a recent story about a very
surprising Postgres performance cliff] caused by
subtransansactions/savepoints resulting from nested `transaction.atomic()`
calls.
Summarizing quotes from the post:
> Our application ended up inadvertently containing recursive calls to
Django’s transaction.atomic, which Django implements by emitting the SQL
SAVEPOINT statement, implemented in Postgres using subtransactions.
> locking a row explicitly and updating a row both take out locks, which
must be accounted for slightly differently. If both locks are held by the
same transaction this can be managed by just updating the tuple flags, but
when the update is performed in a subtransaction, Postgres degrades to
storing the two locks separately using a MultiXact ID
> One fun fact about the MultiXact store is that entries are immutable; if
a new transaction locks a row that is already locked, a new MultiXact ID
must be created, containing all the old transaction IDs as well as the new
ID. This means that having N processes locking a row concurrently requires
potentially quadratic work, since the list of transaction IDs must be
copied in full, each time! This behavior is usually fine, since it’s rare
to have many transactions locking the same row at once, but already
suggests that SELECT FOR SHARE has some potentially-disastrous performance
cliffs.
> We changed the inner transaction.atomic to pass savepoint=False, which
turns it into a no-op if already inside an open transaction, and our
problems immediately disappeared.
> Subtransactions are basically cursed.
I myself haven't used Django in some time, but it seems like there should
either be a way to throw on nested transaction.atomic() calls, or to have
them be no-ops by default, since this is a very surprising and difficult-
to-debug performance consequence. The user could then pass
`savepoint=True` to specifically enable savepoints for a given a nested
`transaction.atomic()`.
In https://code.djangoproject.com/ticket/32220, an option for a given call
is provided to throw if nested, but some users may want this to be enabled
by default for all calls – or to simply be a no-op when nested – to avoid
the performance cliff mentioned in the above piece (and other cursed
aspects of subtransactions).
--
Ticket URL: <https://code.djangoproject.com/ticket/32590>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
transaction.atomic()
-------------------------------------+-------------------------------------
Type: Uncategorized | Status: new
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Adam Johnson):
> In https://code.djangoproject.com/ticket/32220, an option for a given
call is provided to throw if nested, but some users may want this to be
enabled by default for all calls – or to simply be a no-op when nested –
to avoid the performance cliff mentioned in the above piece (and other
cursed aspects of subtransactions).
Such users can create their own alias:
{{{
atomic = functools.partial(atomic, durable=True)
}}}
I think adding any kind of setting to activate this globally would be
harmful as it could break third party app code using transactions.
--
Ticket URL: <https://code.djangoproject.com/ticket/32590#comment:1>
Django
transaction.atomic()
-------------------------------------+-------------------------------------
Type: New feature | Status: closed
Component: Database layer | Version: 3.1
(models, ORM) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* type: Uncategorized => New feature
* resolution: => wontfix
Comment:
I agree with Adam.
--
Ticket URL: <https://code.djangoproject.com/ticket/32590#comment:2>