Async factory hands redis.asyncio.Redis the sync Retry, so enabled retries never retry on async clients

[AlexeyShalaev]

Found while measuring #25. build_redis_retry builds redis.retry.Retry, and create_async_redis_client hands that object to redis.asyncio.Redis. The async client does await conn.retry.call_with_retry(...); the sync class's call_with_retry is a plain function that calls do() once and returns the coroutine it got back, so the client awaits that coroutine outside any loop and the retry never sees the failure. redis.asyncio.retry.Retry has been a separate class with async def call_with_retry since 4.5.0 and is not a subclass of the sync one on any version in the pinned range.

Measured on redis-py 8.1.0 against a paused redis:7-alpine, socket_timeout=0.5, one GET per redis.asyncio.Redis:

  sync  Retry(ExponentialBackoff(1.0, 0.1), 3)     -> TimeoutError      0.50 s
  async Retry(ExponentialBackoff(1.0, 0.1), 3)     -> TimeoutError      3.41 s
  sync  Retry(NoBackoff(), 0)                      -> TimeoutError      0.50 s
  async Retry(NoBackoff(), 0)                      -> TimeoutError      0.50 s

So on the async side retry.enabled=True, max_attempts=3 has never retried a single-node client. The async cluster is half affected: its command loop only reads get_retries() off the object, so that part counts, but the pipeline path awaits call_with_retry and does not. The zero-retry object #25 introduces for the disabled path is unaffected, as the last two rows show — no retries need no loop.

The fix is not one line because build_redis_retry and build_base_redis_kwargs are public and shared by both factories, and they have to know which flavour they are building. Two shapes I can see: a keyword on both helpers selecting the class, or a second pair of helpers for the async side, mirroring the aio/sync split the rest of the package already has. Either way the async factory ends up handing redis.asyncio.retry.Retry, rule 6 on the agents page gets a line about it, and the async test that asserts isinstance(kwargs["retry"], redis.retry.Retry) flips to the async class. Anyone with retries enabled on an async client will see their commands start retrying, which is what they asked for, so it is a fix: whose changelog line says so.

Script

"""Does a sync redis.retry.Retry retry at all when handed to an async client?"""
import asyncio
import time

from redis.asyncio import Redis
from redis.asyncio.retry import Retry as AsyncRetry
from redis.backoff import ExponentialBackoff, NoBackoff
from redis.retry import Retry as SyncRetry
from testcontainers.redis import RedisContainer


async def timed(label, coro):
    started = time.perf_counter()
    try:
        result = await asyncio.wait_for(coro, timeout=30)
        print(f"  {label:<48} -> {result!s:<15} {time.perf_counter() - started:6.2f} s")
    except Exception as error:  # noqa: BLE001
        print(f"  {label:<48} -> {type(error).__name__:<15} {time.perf_counter() - started:6.2f} s")


async def main(host, port, wrapped):
    common = dict(host=host, port=port, socket_timeout=0.5, socket_connect_timeout=0.5)
    clients = {
        "sync  Retry(ExponentialBackoff(1.0, 0.1), 3)": Redis(**common, retry=SyncRetry(ExponentialBackoff(cap=1.0, base=0.1), 3)),
        "async Retry(ExponentialBackoff(1.0, 0.1), 3)": Redis(**common, retry=AsyncRetry(ExponentialBackoff(cap=1.0, base=0.1), 3)),
        "sync  Retry(NoBackoff(), 0)": Redis(**common, retry=SyncRetry(NoBackoff(), 0)),
        "async Retry(NoBackoff(), 0)": Redis(**common, retry=AsyncRetry(NoBackoff(), 0)),
    }
    for client in clients.values():
        await client.ping()
    wrapped.pause()
    print("--- Redis paused ---")
    for label, client in clients.items():
        await timed(label, client.get("k"))
        pool = client.connection_pool
        available = list(pool._available_connections)
        print(f"      pool after failure: available={len(available)} connected={[c.is_connected for c in available]} in_use={len(pool._in_use_connections)}")
    wrapped.unpause()
    for client in clients.values():
        await client.aclose()


with RedisContainer("redis:7-alpine") as container:
    asyncio.run(main(container.get_container_host_ip(), int(container.get_exposed_port(6379)), container.get_wrapped_container()))

[AlexeyShalaev]

Checked one venv per version, the same ten as for #25: redis.asyncio.retry.Retry is there from 4.5.0 through 8.1.0, never a subclass of redis.retry.Retry, always with async def call_with_retry and the same (backoff, retries, supported_errors) constructor, and both Connection.connect and Redis.execute_command on the async side await call_with_retry on every one of them. So nothing here touches the floor, and one object shape works across the range.

Plan: a keyword on both helpers rather than a second pair. build_redis_retry(settings, *, asyncio=False) and build_base_redis_kwargs(settings, *, asyncio=False) build redis.retry.Retry by default, exactly as today, and redis.asyncio.retry.Retry with asyncio=True — for the exponential object and for the zero-retry one from #27 alike. The async factory passes asyncio=True for single and cluster; the sync factory is untouched. build_redis_retry gets Literal overloads, so a sync caller still sees redis.retry.Retry and an async caller sees the async class, with no union to narrow. No new names, one flag, and the flag reads as the redis-py namespace it selects.

The two other shapes: substituting the class inside the async factory alone leaves build_base_redis_kwargs(settings) handing a sync Retry to anyone assembling redis.asyncio.Redis(**kwargs) by hand, the same trap #27 closed for the disabled path; a second pair of helpers is two more root exports and two more rows on the agents page for one bit of information.

Tests: the async resilience test flips to redis.asyncio.retry.Retry, the retry-disabled test in aio/test_client.py flips too for single and cluster, test_utils.py covers asyncio=True on both helpers, and an integration test against the paused container asserts that an async client with the fake settings' max_attempts=3, backoff_base=0.1, backoff_cap=1.0 and socket_timeout=0.5 takes four attempts plus 1.4 s of backoff, about 3.4 s, rather than 0.5 — with a sync twin that already passes and pins the sync path. The API rows and rule 5 on the agents page and the retry section of the configuration guide follow the code, and a "common mistakes" block shows the by-hand trap.

[AlexeyShalaev]

#29 is up. Both helpers take asyncio=False now: build_redis_retry(settings, asyncio=True) and build_base_redis_kwargs(settings, asyncio=True) build redis.asyncio.retry.Retry, and the async factory passes it for single and cluster, so the exponential object and the zero-retry one from #27 both arrive in the right flavour. The sync factory and the default of both helpers are unchanged. The same measurement on the kit's own clients, retries enabled as in your script against the paused container: the async client fails in 0.50 s on master and 3.41 s on the branch, the sync client 3.42 s on both. One venv per version, redis.asyncio.retry.Retry with async def call_with_retry is there from 4.5.0 to 8.1.0, so nothing moves the floor.

Nothing to change on your side unless you build kwargs for a redis.asyncio client by hand — then pass asyncio=True. Rule 5 and the two API rows on the agents page, and the retry section of the configuration guide, say so.