#37156 closed New feature (needsnewfeatureprocess)
Change async Redis methods implementation to use Redis async primitives
| Reported by: | Johannes Maron | Owned by: | |
|---|---|---|---|
| Component: | Core (Cache system) | Version: | 6.0 |
| Severity: | Normal | Keywords: | redis, asyncio, async |
| Cc: | Johannes Maron | Triage Stage: | Unreviewed |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
The backend API already implements aget, aset, β¦ methods.
However, the native RedisCache backend, doesn't reimplement those methods to use the asyncio API of the Redis SDK, but rather the asgiref fallback.
As discussed before, the extra threading comes with a significant overhead and disrupts things like full-page caching of otherwise async views.
This proposal does not alter Django's public API, but rather optimizes internal implementation, which is why I put this on Trac.
Change History (9)
comment:1 by , 3 weeks ago
comment:2 by , 3 weeks ago
... the extra threading comes with a significant overhead
Sigh. This is essentially a myth, at least these days. Unless you're repeatedly crossing the boundary in a hot-loop β which, don't! β overhead is basically negligible.
Redis-py's async client is no faster than the sync client (indeed it seems slightly slower, but I need to verify that before stating definitively βΒ it's negligible too though). Similar to the way psycopg's async cursors serialise queries, throughput is limited by pool size. And do enable pipelining!
I shall put writing this up on the backlog, but it's not at all clear that the argument to remove (particularly) sync_to_async calls throughout is/remains sound.
...core devs won't want to...
... core devs don't like ...
This phrasing isn't helpful. (TBH it verges on prompting a CoC report, since it's frequent.) The original design decisions were to support both sync and async views (and so code) under both WSGI and ASGI, and (given that) duplicating the entire code tree with pure async versions is not a sustainable strategy. Nothing has changed there. It's not about "want to" or "not liking".
comment:3 by , 3 weeks ago
Ok, I should have withheld any performance claims w/o proof. But since I ran out of my monthly token rations, let's park that discussion.
---
In my humble opinion, I thought it to be reasonable to assume that RedisCache.aget actually calls the async stack of the Redis SDK.
I certainly did and tripped on the fact it wasn't. I am aware that connection management is a complication. But just because something is hard has never stopped us before. <3
comment:4 by , 3 weeks ago
| Description: | modified (diff) |
|---|
comment:5 by , 3 weeks ago
FWIW pending full write up: overhead of sync_to_async is in the low microseconds range, and there's no measurable gain (in fact a small loss) at the redis client level.
The ergonomics here are that RedisCache offers an async API. Whether it "actually calls the async stack of the Redis SDK" is an implementation detail. There's a serious question as to whether there is any benefit to us maintaining parallel/duplicate code paths here.
As I say, I'll write up more, but that's how I'm leaning.
follow-up: 7 comment:6 by , 3 weeks ago
| Owner: | set to |
|---|---|
| Status: | new β assigned |
comment:7 by , 3 weeks ago
| Owner: | removed |
|---|---|
| Status: | assigned β new |
Replying to B V HITESH SAI:
Great that you are so enthusiastic about this proposal. However, we usually await triage before assigning issues.
comment:8 by , 3 weeks ago
| Resolution: | β needsnewfeatureprocess |
|---|---|
| Status: | new β closed |
| Type: | Cleanup/optimization β New feature |
Thanks everyone for the discussion. I have read all the comments and did some research to gather context.
My view is that this is ultimately a new feature request, regardless of whether it requires changes to Django's public API. The proposal is not simply an internal optimization; it would introduce a distinct implementation strategy with different connection-management constraints and trade-offs. This is a big change with non-trivial side effects.
I also agree with Carlton that framing arguments in terms of what "core devs" do or do not want is not particularly helpful. Django is a community-driven project, and proposals are evaluated on their technical merits, maintenance costs, and alignment with the project's goals rather than on the preferences of any particular group.
Stepping back, I'm not yet convinced that changing the existing RedisCache backend is the right direction. The current implementation already provides an async API, and the discussion so far has not established a clear benefit that would justify the additional complexity of maintaining separate sync and async Redis implementations.
If there is a genuine need for a cache backend optimized specifically for ASGI deployments and native asyncio Redis connections, I firmly believe that a separate backend (in a third party app at firtst_ may be a cleaner approach. That would allow ASGI users to opt into a different set of trade-offs without introducing additional complexity or behavioral differences into Django's built-in RedisCache backend, which continues to support both WSGI and ASGI environments.
comment:9 by , 3 weeks ago
| Summary: | Properly implement async Redis methods β Change async Redis methods implementation to use Redis async primitives |
|---|
Hey. Happy to help get rid of the
async_to_syncstuff in Django π I'll leave a few thoughts below, hope they help.To get a real async implementation, we basically just need to implement
AsyncRedisClient. Django already depends on redis, which ships an async client. But there's a catch. πThe async redis client relies on event-loop-bound connections, which means the code won't work under WSGI. Django supports running async code under WSGI, and core devs won't want to drop that. So the options are:
AsyncRedisCacheso people on ASGI can opt into it. Avoids breaking changes, but core devs don't like adding a new class for this.I looked into the async_to_sync performance degradation, and it looks like creating threads doesn't cause significant degradation on its own. what matters more is how frequently you hit it during the request cycle. If you're seeing something like that, could you share your setup (RPS, sync_to_async calls per request, avg CPU per pod)? That'd be helpful.