Opened 3 years ago
Closed 3 years ago
#34359 closed New feature (wontfix)
Add setting to disable global thread_sensitive flag for Async ORM and Async cache
| Reported by: | Jameel A. | Owned by: | nobody |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | 4.1 |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Apologies if this has been discussed before, but I couldn't find a related issue.
The way the Async ORM interface is currently implemented is by simply introducing new a-prefixed methods that wrap the original QuerySet methods in sync_to_async. On the surface this is a simple, slightly suboptimal, way of implementing asyncio support.
Under the surface, though, there be dragons. With asgiref enabling the thread_sensitive by default, what actually happens is ALL uses of sync_to_async get queued onto a single thread. This can have disastrous performance implications as we've seen in production (especially when used with native async code).
While thread_sensitive is safer as mentioned in the docs, it's not always necessary. For example, when using Postgres, the psycopg database driver is thread-safe and appears to work fine with the thread_sensitive flag disabled.
The issue here is not whether thread_sensitive should be True by default, but rather there should be a way to disable it when the user knows it's safe to do so. In particular, for the async ORM and async cache interfaces, it would be wonderful to configure thread sensitivity via a setting.
Change History (2)
comment:1 by , 3 years ago
| Summary: | Support disabling global thread_sensitive flag for Async ORM → Add setting to disable global thread_sensitive flag for Async ORM and Async cache |
|---|
comment:2 by , 3 years ago
| Component: | Uncategorized → Database layer (models, ORM) |
|---|---|
| Resolution: | → wontfix |
| Status: | new → closed |
| Type: | Uncategorized → New feature |
Define "appears" :) — I think this needs a proof-of-concept, and discussion on the Internals > Async category of the forum to be able to move forward. (AFAICS there are a number of issues, from connections, to model instances, to signals, to... that mean we can't just toggle this off. Showing that it's safe in circumstance X or Y or Z is prerequisite to change here.) Thanks!