Add option to avoid subtransactions by default when using transaction.atomic()

[Alex Rattray]

Inspired by ​@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 ​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).

[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.

[Mariusz Felisiak]

I agree with Adam.