Opened 2 years ago

Last modified 5 weeks ago

#33882 assigned New feature

Allow transaction.atomic to work in async contexts.

Reported by: alex Owned by: rajdesai24
Component: Database layer (models, ORM) Version: 4.0
Severity: Normal Keywords: async
Cc: Hugo Osvaldo Barrera, rajdesai24, Moritz Ulmer, Tyson Clugg, Julien Enselme Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Tim Graham)

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?

Change History (21)

comment:1 by Carlton Gibson, 2 years ago

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?

Last edited 2 years ago by Carlton Gibson (previous) (diff)

comment:2 by Mariusz Felisiak, 2 years ago

Resolution: needsinfo
Status: newclosed

comment:3 by alex, 2 years ago

Resolution: needsinfo
Status: closednew

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

comment:4 by alex, 2 years ago

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.

Last edited 2 years ago by alex (previous) (diff)

comment:5 by Carlton Gibson, 2 years ago

Component: UncategorizedDatabase layer (models, ORM)
Keywords: async added
Triage Stage: UnreviewedAccepted

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

comment:6 by Carlton Gibson, 2 years ago

Summary: async transaction.atomicAllow transaction.atomic to work in async contexts.

comment:7 by Tim Graham, 2 years ago

Description: modified (diff)

comment:8 by Hugo Osvaldo Barrera, 2 years ago

Cc: Hugo Osvaldo Barrera added

comment:9 by HMaker, 2 years ago

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.

comment:10 by rajdesai24, 21 months ago

Owner: changed from nobody to rajdesai24
Status: newassigned

comment:11 by rajdesai24, 21 months ago

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)


comment:12 by rajdesai24, 21 months ago

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

comment:13 by rajdesai24, 21 months ago

Cc: rajdesai24 added

comment:14 by Mike Lissner, 14 months ago

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.

comment:15 by Moritz Ulmer, 14 months ago

Cc: Moritz Ulmer added

comment:16 by Tyson Clugg, 12 months ago

Cc: Tyson Clugg added

comment:17 by Julien Enselme, 11 months ago

Cc: Julien Enselme added

comment:18 by André S. Hansen, 2 months ago

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():
   # ...
Version 3, edited 2 months ago by André S. Hansen (previous) (next) (diff)

comment:19 by Denis Orehovsky, 6 weeks ago

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

in reply to:  19 comment:20 by Natalia Bidart, 6 weeks ago

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!

comment:21 by Ahmed Ibrahim, 5 weeks ago

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

Note: See TracTickets for help on using tickets.
Back to Top