Allow transaction.atomic to work in async contexts.

[alex]

Wouldn't it be possible to add something like:

    __aenter__ = sync_to_async(Atomic.__enter__, thread_sensitive=True)
    __aexit__ = sync_to_async(Atomic.__exit__, thread_sensitive=True)

to the Atomic class to support async calls?

[Carlton Gibson]

Hi Alex.

Can I ask you to take the time to explain your issue report in full please? (Likely it's clear to you, but it needs more detail, perhaps starting from the beginning…)

Is it a duplicate of #32409?

Wouldn't it be possible to add something like...

Did you try this, perhaps with some test cases to exercise the new code?

[alex]

ok sorry was in a hurry. The code isn't tested yet so I cannot assure it is working.
Details follow

[alex]

django supports db transactions with rollback via the
django.db.transaction.atomic function which can either be used as decorator or as contextmanager.
Everything wrapped within is executed as transaction, see:

​https://docs.djangoproject.com/en/4.0/topics/db/transactions/

That is very nice but now there is a little problem:
it isn't async safe:

​https://forum.djangoproject.com/t/is-it-possible-to-use-transaction-atomic-with-async-functions/8924

When looking through the contextmanager (which is actually a class named Atomic in the same file) I saw that __enter__ and __exit__ use simple db operations for starting and commiting the transaction.
If we use the wrappers like suggested (__aenter__ and __aexit__) then everything should be fine in theory. In praxis I need to test the code (I have not much time so it may be easier if somebody else tries this out)

I build a wrapper class like this (of course it would be better to have it in Atomic itself so the atomic function can be used and no manual initialization is required):

from asgiref.sync import sync_to_async
from django.db.transaction import Atomic

class AsyncAtomic(Atomic):
    __aenter__ = sync_to_async(Atomic.__enter__, thread_sensitive=True)
    __aexit__ = sync_to_async(Atomic.__exit__, thread_sensitive=True)

Warning: untested

I must confess: the use case is very rare and is only useful in combination with select_for_update for delaying row updates (otherwise it is an antipattern as it causes slow transactions). And this is why I haven't a good example code.

[Carlton Gibson]

OK, thanks for the update Alex. I'm going to Accept this, since it's a desirable feature that transactions work in an async-context.

I'm half-inclined towards closing as needsinfo or marking as Someday/Maybe as I suspect the implementation will require a bit more than just adding the sync_to_async() wrappers. 🤔

I guess the first step would be to write some test cases for that and see what issues arise and what the performance looks like. (From there it's easier to see what's really involved.)

[HMaker]

I opened an issue at the channels repo ​https://github.com/django/channels/issues/1937 related to this.

The use case is not rare, it's pretty common on an async codebase. As you know, async code requires new API definitions on new namespaces, sync (blocking) and async code can't be mixed. Actually Django forces you to run the full transaction block inside @sync_to_async decorated functions, the async API becomes useless, you can't reuse any async code.

[rajdesai24]

Hey guys, I wanted to get your suggestion on this as I had been working on this and testing it out

class AsyncAtomic:
    def __init__(self, using=None, savepoint=True, durable=True):
        self.using = using
        self.savepoint = savepoint
        self.durable = durable

    async def __aenter__(self):
        self.atomic = transaction.Atomic(self.using, self.savepoint, self.durable)
        await sync_to_async(self.atomic.__enter__)()
        return self.atomic

    async def __aexit__(self, exc_type, exc_value, traceback):
        await sync_to_async(self.atomic.__exit__)(exc_type, exc_value, traceback)

---

class AsyncAtomicTestCase(TestCase):
    async def test_atomic(self):
        async with AsyncAtomic():
            # Create a new object within the transaction
            await sync_to_async(SimpleModel.objects.create)(
            field=4,
            created=datetime(2022, 1, 1, 0, 0, 0),
            )
        # Verify that the object was created within the transaction
        count = await sync_to_async(SimpleModel.objects.count)()
        self.assertEqual(count, 1)

[rajdesai24]

I made a new class called Async Atomic and used it as a context decorator, which worked but you still need to use the sync_to_async

[Mike Lissner]

I'm quite bad at async things generally, but I thought I'd chime in to say that I'm surprised async atomic transactions aren't more of a priority. A few of the comments above seem to imply that this isn't an important feature or that it's an antipattern (maybe?).

I just turned down part of a PR where a developer is converting our code to async because to do so required that we drop the @transaction.atomic decorator. I said, "Sorry, we can't covert this to async because given the choice between correctness and performance, I have to choose correctness."

Am I missing something big — Isn't this a big gap in Django's support for real applications converting fully to async?

Thanks all, sorry I don't have more to add! If I were better at async, I'd take a crack at actually fixing it.

[André S. Hansen]

I am hesitant going for async django until transaction support is added or a cleaner solution than having to use sync_to_async is available. The problem with this workaround is that it forces you to write all code inside the transaction as sync. Which is going to be a mess when transaction and non-transaction scopes want to use the same code/logic.

The following example illustrate the problem

async def post():
    await sync_to_async(_post_sync_transaction)  # <-- Messy, but maybe acceptable


@transaction.atomic
def _post_sync_transaction():
   logic1()
   async_to_sync(alogic2) # <-- Very messy, not acceptable
   async_to_sync(alogic3) # <-- Very messy, not acceptable


async def get():
   await alogic2()
   await alogic3()


def logic1():
   # ...


async def alogic2():
   #...


async def alogic3():
   # ...

There are two problems with this pattern:

• You have to write helper functions to handle transactions
• You have to make sure to convert all async contexes to sync, using async_to_sync, within the transaction helper

[Denis Orehovsky]

I agree. Why hasn't it been fixed yet? Multiple years have passed already.

[Natalia Bidart]

Replying to Denis Orehovsky:

I agree. Why hasn't it been fixed yet? Multiple years have passed already.

Hello Denis! Please note that this is not a helpful comment, I understand that you may feel frustrated but I encourage you to consider our ​Code of Conduct in your comments:

Be friendly and patient.
Be welcoming.
Be considerate.
Be respectful.
Be careful in the words that you choose.

Django is developed by volunteers who contribute their time when they can. If you're interested in this feature, it would be fantastic if you could help by contributing to a solution.

Thank you for your understanding!

[Ahmed Ibrahim]

I can take this one, but how would I test it after implementation? (manual testing I mean)

[Hedley Roos]

The suggested async atomic decorator does work in my use cases, but because the code has no tests, I cannot fully trust it. Incidentally, the decirator with thread_sensitive=True fails under high load. I can simulate that failure locally.

As for common use cases for async transaction support:

1. Distributed row-level lock through select_for_update. Crucial for financial systems.
2. transaction.on_commit for comms to external systems. This can be as simple as sending a task to Celery on a successful commit.
3. A method where we modify X related objects needs to be atomic.

Django is very nearly there with full async support. I use Connexion | FastAPI + Django + Flutter as a toolchain, because Django has the best ORM by far, and many other best in class features. I wish I understood the async transactional part a bit better, then I would gladly jump in and fix this. As it is I'm hoping that the person who wrote the original transaction management can jump in here and help us.

[Hedley Roos]

I have created scripts to illustrate how the proposed solution fails under load. Django calls close_old_connections on every request, and I simulate it in a simpler environment so you don't have to bootstrap a whole Django. Also, it's easier to introduce load via scripts.

Download the failure_* attachments on this ticket. Run the server script, and then the client script. You will of course need Django and requests in your Python. You should see randomly occurring connection errors. Removing the aatomic decorator from the server script also removes these errors, so clearly it introduces a problem. By the way, I adapted the decorator from ​https://stackoverflow.com/questions/74575922/how-to-use-transaction-with-async-functions-in-django.

Now, if I am misunderstanding the point of close_old_connections, then this entire post is moot. Please let me know if I am doing something wrong.

In my particular use-case I have resorted to a horrible hack where I randomly call close_old_connections, instead of on every request.

[Carlton Gibson]

I'm going to see if I can get this working at Django on the Med this year.

[Carlton Gibson]

We've discussed the prospects for this ticket at Django on the Med in Pescara.

Actually enabling async with transaction.atomic() is relatively simple: we can
implement __aenter__ and __aexit__ to call into the existing sync variants, applying
their transaction control to the connection of the current context's thread-sensitive worker thread.

The trouble is this immediately introduces issues with transaction isolation. Within a single task the transaction begin, act, and commit/rollback phases are well ordered, but with multiple tasks running concurrently operations are interleaved on the shared connection.

Imagine a slow Task 1 opening a transaction, performing some work, which errors and then rolling back, whilst a fast Task 2 makes a single write on the same connection, which is then also rolled back as Task 1 exits.

These kinds of issues are easy to construct and, if we were to stop there, would constitute a foot-gun by design. Ensuring that database queries are correctly serialised, and that transactions are isolated is (as soon as you start thinking through async transaction scenarios) the key feature of the existing advice to move your database operations into a sync function and then using sync_to_async, within which no suspension points can occur, if needing transactions under async.

Nonetheless, the API remains desirable. For standard async view examples, normally running as a single asyncio task, being able to group related ORM operations into a transaction, without needing to move away from async methods, would essentially complete the ORM's async API.

As such, we concluded that two additional steps are required in order to make the API addition plausible.

First, any task entering a transaction, via async with transaction.atomic() will require a dedicated database connection, rather than using the shared connection of the current context's thread-sensitive worker thread. We will adjust asgiref's ThreadSenstiveContext to allow creating a nested context, with it's own worker thread and connection, that will apply only to the transaction during its lifetime. (This approach leverage Django's existing connection management flows.)

Second, we will track the current task that opens the transaction, and disable further uses of atomic within that transaction except from that same task. Most standard use-cases run in a single task and are not affected by this restriction. We will document that sub tasks must make proper use of structured concurrency, to ensure correct sequencing relative to the wrapping transaction, and require that savepoint logic is driven from the parent task, in order to ensure sequencing, and reasonability.

That's all a bit of a mouthful, but the summary is that allowing async with transaction.atomic() should be plausible with that in place.

[Caleb Hattingh]

Carlton, based on our discussions at Django on the Med I created:

• asgiref PR: ​https://github.com/django/asgiref/pull/596
• django PR: ​https://github.com/django/django/pull/22074

I am new to Django dev, but hopefully this is a reasonable starting point. I have spent some time looking at the various options.
This issue is fairly complex.

I would like to add a more detailed comment here about the issues when I have time, but briefly for other readers:

• The goal is to get async with transaction.atomic working
• For now we have chosen to retain the current django system of connection management...
• ...which means transaction isolation requires separate connections which requires separate threads (you explained this above)
• The ThreadSensitiveContext is the important location to figure out how to give transaction.atomic its own thread within an async context
• "make it all work"

On re-entrancy: I believe the current POC PRs above allow nested use of atomic, with nested transactions being savepoints. This is standard within the exist sync handling, I believe.
One particular difficulty was handling CurrentThreadExecutor concerns with nesting of alternating sync_to_async and async_to_sync calls. There is a nasty deadlock that can happen, which I think is avoided in the PRs above. It is all quite complex.

Eventually, I ended up looking at two similar designs:

1. ThreadSensitiveContext(force_new_thread=True)
2. ThreadSensitiveContext(executor=<given by Atomic.__aenter__>)

They are nearly equivalent, differing only in where the new executor is made. (I gave some explanation about this on the Django PR description).
I think I prefer 2 because it allows a future optimization where a small pool of executors can be reused, saving time to make threads and connections. This optimization is not in the Django PR above, to keep everything simple. But I do have some code for that optimization in a branch. If you prefer option 1 I can change the PRs to that.

As I mentioned already, I don't mind if ppl want to use parts of these ideas or even go in a different direction altogether.
Long term, I think there should be a separate connection management system for async that doesn't require the coupling to thread creation ;)