[Django] #36455: Cache backends delegate async methods to BaseCache implementations instead of using the backend's native support for incr(), get_many(), etc..

[Django]

#36455: Cache backends delegate async methods to BaseCache implementations instead
of using the backend's native support for incr(), get_many(), etc..
-------------------------------------+-------------------------------------
Reporter: LaughInJar | Type: Bug
Status: new | Component: Core
| (Cache system)
Version: 5.2 | Severity: Normal
Keywords: aincr adecr | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Both the Redis and Memcached backends do not override the async methods of
the `BaseCache` base class. However the base class does not simply wrap
all the sync methods using `sync_to_async` but instead recreate the
functionality of methods like `incr()` or `get_many()` by using `aget()`
and `aset()`.

For example, the `aincr()` method in the BaseCache looks like this:

{{{#!python
async def aincr(self, key, delta=1, version=None):
"""See incr()."""
value = await self.aget(key, self._missing_key, version=version)
if value is self._missing_key:
raise ValueError("Key '%s' not found" % key)
new_value = value + delta
await self.aset(key, new_value, version=version)
return new_value
}}}

However, both Redis and Memcached have native support for incrementing a
key the respective Django backends override the `BaseCache` 's sync
methods to use these native functions. But since they do not override the
async methods, they fall back to the implementation of `BaseCache` which
uses `aget` and `aset`.

This is not only a performance drawback since this requires additional
calls, it also means that the calls to `aincr` are no longer atomic. The
docs say that atomicity is not guaranteed but it it will be atomic when
the backend supports it. Redis and Memcache support it.

This is relevant when e.g. trying to implement a throttle for API calls.

In order to reproduce, recreate a new virtualenv and install Django.

{{{#!python
import asyncio
from django.core.cache.backends.redis import RedisCache

cache = RedisCache("redis://localhost:6379/4", {})

# uncomment this to 'fix' the issue
# from asgiref.sync import sync_to_async
# cache.aincr = sync_to_async(cache.incr)

key = "counter"
cache.add(key, 0, timeout=1)

async def task() -> int:
return await cache.aincr(key)

async def run():
tasks = [task() for _ in range(10)]
return await asyncio.gather(*tasks)

counters = sorted(asyncio.run(run()))

print(counters)
# outputs [1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
# the expected output should be
# [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
}}}

Although I've focused on `aincr()` I can see from the code that other
methods like `ahas_key`, `aset_many`, `aget_many`, `adelete_many`, and
`aget_or_set` are similarily emulated using `aget` and `aset`. Redis and
Memcache have native support for these functions which is utilized when
calling the sync methods. The expectation would be that the async methods
would work in the same manner as the sync methods.
--
Ticket URL: <https://code.djangoproject.com/ticket/36455>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36455: Cache backends delegate async methods to BaseCache implementations instead
of using the backend's native support for incr(), get_many(), etc..
-------------------------------------+-------------------------------------

Reporter: LaughInJar | Owner: (none)

Type: Bug | Status: new
Component: Core (Cache system) | Version: 5.2

Severity: Normal | Resolution:

Keywords: aincr adecr | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by LaughInJar):

I thought I've read the whole documentation carefully beforehand, but just
after hitting send I revisted that section
https://docs.djangoproject.com/en/5.2/topics/cache/#id16

I think it covers the things I've discussed above even when it only
becomes clear after one did the deep dive into the codebase on why the
likes of `aincr` do not work as expected. Maybe this ticket at least saves
somebody else time doing research.
--
Ticket URL: <https://code.djangoproject.com/ticket/36455#comment:1>

[Django]

#36455: Cache backends delegate async methods to BaseCache implementations instead
of using the backend's native support for incr(), get_many(), etc..
-------------------------------------+-------------------------------------
Reporter: LaughInJar | Owner: (none)

Type: Bug | Status: closed

Component: Core (Cache system) | Version: 5.2

Severity: Normal | Resolution: invalid

Keywords: aincr adecr | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* resolution: => invalid
* status: new => closed

Comment:

Thank you
Note there is a ticket for adding native async support to the redis
backend: https://code.djangoproject.com/ticket/33573
--
Ticket URL: <https://code.djangoproject.com/ticket/36455#comment:2>