id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
32590	Add option to avoid subtransactions by default when using transaction.atomic()	Alex Rattray	nobody	"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). "	New feature	closed	Database layer (models, ORM)	3.1	Normal	wontfix			Unreviewed	0	0	0	0	0	0