Opened 3 weeks ago

Last modified 3 weeks ago

#37156 closed New feature

Properly implement async Redis methods — at Version 4

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 (4)

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

Last edited 3 weeks ago by Carlton Gibson (previous) (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)
Note: See TracTickets for help on using tickets.
Back to Top