Re: [Django] #33882: Allow transaction.atomic to work in async contexts.

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Description changed by Tim Graham:

Old description:

> 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?

New description:

Wouldn't it be possible to add something like:
{{{ #!python
__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?

--

--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:7>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Hugo Osvaldo Barrera):

* cc: Hugo Osvaldo Barrera (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:8>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Heraldo Lucena):

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:9>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:

| rajdesai24
Type: New feature | Status: assigned

Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by rajdesai24):

* owner: nobody => rajdesai24
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:10>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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)

}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:11>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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

--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:12>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by rajdesai24):

* cc: rajdesai24 (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:13>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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.

--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:14>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by moritz89):

* cc: moritz89 (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:15>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Tyson Clugg):

* cc: Tyson Clugg (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:16>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Julien Enselme):

* cc: Julien Enselme (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:17>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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 sync. Which is going to be a mess when transaction
and non-transaction scopes want to use the same code/logic.

Imagine the following simple example that illustrate the mess one will get
in.

{{{
async def post():
await sync_to_async(_post_sync_transaction)


@transaction.atomic
def _post_sync_transaction():
logic1()
async_to_sync(alogic2) # <-- Messy maintaining
async_to_sync(alogic3) # <-- Messy maintaining


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


def logic1():
# ...


async def alogic2():
#...


async def alogic3():
# ...
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:18>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by denisorehovsky):

I agree. Why hasn't it been fixed yet? Multiple years have passed already.
--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:19>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Natalia Bidart):

Replying to [comment:19 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
[https://www.djangoproject.com/conduct/ 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!
--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:20>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Ahmed Ibrahim):

I can take this one, but how would I test it after implementation? (manual
testing I mean)
--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:21>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:22>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Hedley Roos):

* Attachment "failure_server.py" added.

--
Ticket URL: <https://code.djangoproject.com/ticket/33882>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Hedley Roos):

* Attachment "failure_client.py" added.

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Hedley Roos):

* Attachment "failure_settings.py" added.

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Hedley Roos):

I have create 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:23>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Charles Roelli):

* cc: Charles Roelli (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:24>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by tumbl3w33d):

* cc: tumbl3w33d (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:25>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by laggron42):

* cc: laggron42 (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:26>

[Django]

#33882: Allow transaction.atomic to work in async contexts.
-------------------------------------+-------------------------------------
Reporter: alex | Owner:
| rajdesai24
Type: New feature | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Irt291):

* cc: Irt291 (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:27>