Opened 3 weeks ago

Closed 3 weeks ago

Last modified 3 weeks ago

#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 Johannes Maron)

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 Mykhailo Havelia, 3 weeks ago

Hey. Happy to help get rid of the async_to_sync stuff 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:

  • Open and close a connection for each command. Works everywhere, but kills performance.
  • Implement a separate AsyncRedisCache so people on ASGI can opt into it. Avoids breaking changes, but core devs don't like adding a new class for this.
  • Use a third-party library like ​https://github.com/Andrew-Chen-Wang/django-async-redis, without official Django support.

As discussed before, the extra threading comes with a significant overhead

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.

comment:2 by Carlton Gibson, 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 essentially 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".

Version 2, edited 3 weeks ago by Carlton Gibson (previous) (next) (diff)

comment:3 by Johannes Maron, 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 Johannes Maron, 3 weeks ago

Description: modified (diff)

comment:5 by Carlton Gibson, 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.

comment:6 by B V HITESH SAI, 3 weeks ago

Owner: set to B V HITESH SAI
Status: new β†’ assigned

in reply to:  6 comment:7 by Johannes Maron, 3 weeks ago

Owner: B V HITESH SAI 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 Natalia Bidart, 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 Natalia Bidart, 3 weeks ago

Summary: Properly implement async Redis methods β†’ Change async Redis methods implementation to use Redis async primitives
Note: See TracTickets for help on using tickets.
Back to Top