Opened 2 years ago

Last modified 10 days ago

#33573 new New feature

Add native async support to redis cache backend

Reported by: Christopher Bailey Owned by: nobody
Component: Core (Cache system) Version: dev
Severity: Normal Keywords:
Cc: Andrew Chen Wang, Andrew Godwin, Carlton Gibson, Ülgen Sarıkavak Triage Stage: Someday/Maybe
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Christopher Bailey)

The latest version of redis-py will support both sync and async clients so sync_to_async will no longer be necessary so it would be nice if the out of the box Redis cache backend supports both natively as well.

https://github.com/redis/redis-py/releases/tag/v4.2.0rc1

It may be a bit premature since 4.2.0 is still RC, but I wanted to get a ticket out there so it can be on someones radar.

Change History (10)

comment:1 by Mariusz Felisiak, 2 years ago

Cc: Andrew Chen Wang Andrew Godwin Carlton Gibson added
Triage Stage: UnreviewedAccepted

Tentatively accepted. Do you have an idea for implementation? As far as I'm aware this would require initializing two redis clients: async and non-async.

comment:2 by Mariusz Felisiak, 2 years ago

Triage Stage: AcceptedSomeday/Maybe

Let's wait for the final release first.

comment:3 by Andrew Chen Wang, 2 years ago

We are currently re-implementing the async client (one last time) in a way that allows for aioredis 1.3.1 performance. https://github.com/seandstewart/redis-py-sansio/blob/main/benchmark/conftest.py The RedisLab team will need to approve it first, so I would wait until our new client is either approved/denied https://github.com/redis/redis-py/issues/2039. This won't affect the non-async client as the non-async client will still default on the current redis-py interface with the option to also use the new API.

This isn't to say no one should try implementing the async client now; I have high confidence the new client will be merged in with Redis-py as we're gunning for the much-needed performance gain. Just don't be surprised if a few things shift around.

comment:4 by Christopher Bailey, 2 years ago

Description: modified (diff)

comment:5 by Christopher Bailey, 2 years ago

Description: modified (diff)

Do you have an idea for implementation? As far as I'm aware this would require initializing two redis clients: async and non-async.

I kind of started to implement it, but then realized it was likely going to be a bigger issue then I though. But there is already an abstract RedisClient class that is a wrapper around the base redis.Redis class. It maintains connection polls and handles all of the core commands. It has methods for get, set, add, touch, incr that just calls get_client and makes a new client using the existing connection pull any time it is called.

For Async we can just add something like get_async_client and follow the same pattern within the class. I believe the connection poll should be able to be reused between the sync/async clients, but I am not an expert on Redis.

Last edited 2 years ago by Christopher Bailey (previous) (diff)

comment:6 by Christopher Bailey, 2 years ago

4.2.0 is fully out now.

comment:7 by Andrew Chen Wang, 15 months ago

It doesn't seem like the Sans-IO approach will be adopted anytime soon as work and review has stalled.

I believe the connection poll should be able to be reused between the sync/async clients, but I am not an expert on Redis.

Sorry if I misunderstood, but the ConnectionPool classes for the sync and async versions are different, and the Connection classes are also different.

As far as I'm aware this would require initializing two redis clients: async and non-async.

Assuming you mean initialize redis.Redis and redis.asyncio.Redis in one class, I disagree; I think we should have two RedisCache classes, one for sync and another for async.

  1. Some users may want both a sync and async Redis client. However, the options presented to devs cater to only one of the variants. For instance, if you need to pass in a custom ConnectionPool or provide pool options for the default pools, those could be different for the different environments like an async enabled middleware uses the async Redis but a sync view uses the sync Redis. If a custom Connection class is provided to the pool, then we'd have a problem since the Connection class for the sync and async Redis are different.
  2. The actual network calls like get and set_many look like they will be copy-paste because of the numerous awaits required, so there won't be much if any re-use of code
  3. Not all clients have upgraded to at least Redis 4.2.0.
  4. It sets a paradigm for future client implementations. Regardless of whether packages like psycopg(3) implement both a sync and async variant or just one type like the memcached packages implementing just sync, there should be a separation of clients based on environment to set future expectations of clients not necessarily having a counterpart variant like an async package not having sync support.

Let me know your thoughts. Thanks!

comment:8 by Carlton Gibson, 15 months ago

Hey Andrew, thanks for the follow-up!

If I've understood you correctly, I think the best way forward would be to prototype the async backend in a external module, so we can see what it would look like and install it and give it a run. I think we'd need to have a some discussion as to whether/how we'd adopt that then into Django, and the messaging we'd put around that.

Make sense?

comment:9 by Andrew Chen Wang, 15 months ago

Makes sense. Thanks for the comment!

You can take a look at a practically broken but conceptual implementation here: https://github.com/Andrew-Chen-Wang/django-async-redis/pull/6

The repository itself is designed to be a django-redis replacement that I recently migrated from aioredis to redis-py; in regards to this convo, the places to look are django_async_redis.client.core.py and the test file tests/test_core.py

comment:10 by Ülgen Sarıkavak, 10 days ago

Cc: Ülgen Sarıkavak added
Note: See TracTickets for help on using tickets.
Back to Top