Async cluster: `ClusterNode.release()` defers returning an already-closed connection to `_free`, creating a transient false `MaxConnectionsError`

[petyaslavova]

Affected

• redis-py 8.1.0 and earlier (present on master @ 1627c98a)
• Async cluster only (redis/asyncio/cluster.py). The sync cluster and both standalone pools do not have this window — see "Why the other pools don't have this" below.
• Requires an explicitly configured max_connections small enough for a node pool to reach capacity.

Summary

ClusterNode.release() returns a connection marked for reconnect to the pool through a
background task instead of returning it inline. Between the release() call and the task's
first scheduling step, the connection is still in self._connections — so it counts against
max_connections — but is not in self._free, so it cannot be acquired. A concurrent command
arriving in that window gets MaxConnectionsError even though capacity is about to be
returned. The connection has already been closed at that point, so the task performs no I/O:
its only effect is deferring a self._free.append(connection) by one event-loop iteration.

Code path

redis/asyncio/cluster.py:

# ClusterNode.release
def release(self, connection: Connection) -> None:
    if connection.should_reconnect():
        task = asyncio.create_task(self._disconnect_and_release(connection))   # 1759
        self._background_tasks.add(task)
        task.add_done_callback(self._background_tasks.discard)
        return
    self._free.append(connection)

# ClusterNode._disconnect_and_release
async def _disconnect_and_release(self, connection: Connection) -> None:
    try:
        await connection.disconnect()      # no-op: connection is already disconnected
    except Exception as exc:
        ...
        return
    self._free.append(connection)          # the only real work, deferred by one loop turn

acquire_connection() raises MaxConnectionsError whenever _free is empty and
len(self._connections) >= self.max_connections, which is exactly the state during that window.

Why the connection is always already closed when this branch is taken

Every caller of release() first awaits a disconnect — execute_command,
execute_pipeline, TransactionStrategy error path and reset(),
_ClusterNodePoolAdapter.get_connection/release - and
AbstractConnection.disconnect() calls reset_should_reconnect()
(redis/asyncio/connection.py) before its first suspension point, with its inner finally
nulling _reader/_writer even on the timeout path.

So release() can only observe should_reconnect() == True if the connection was
re-marked during that awaited disconnect(), at which point is_connected is already
False and _disconnect_and_release() early-returns at if not self.is_connected.

Reachable triggers for the re-mark

1. update_active_connections_for_reconnect(), called from the
   except (ConnectionError, TimeoutError) handler in _execute_command. Requires two
   error events on the same node closely spaced, the second landing inside the first
   connection's wait_closed() suspension.
2. The SMIGRATED maintenance-notification handler
   (redis/asyncio/maint_notifications.py), which marks all in-use connections on affected
   nodes. Note: this requires CLIENT MAINT_NOTIFICATIONS support
   (redis/asyncio/connection.py), so it does not apply to deployments that do not
   negotiate it.

Both need the node pool to be simultaneously at max_connections.

Impact

One transient MaxConnectionsError per occurrence, recoverable on the caller's next attempt.
It is not self-retried: ERRORS_ALLOW_RETRY (redis/cluster.py) is matched with
type(e) in ..., and MaxConnectionsError is a ConnectionError subclass, so the exact-type
check excludes it — and _execute_command re-raises it deliberately without reinitializing
(redis/asyncio/cluster.py). So it surfaces to application code.

Under high concurrency one event-loop iteration can contain many acquisition attempts, so the
error is observable in practice; it is a spurious error, not a stuck client. This is distinct
from #4110 (a monotonic slot leak that never self-healed, fixed in 8.1.0 via #4111).

Why the other pools don't have this

Both ConnectionPool.release() implementations make the connection available only after
disconnecting, so no window exists:

• sync redis/connection.py — disconnects inline while holding self._lock
• async redis/asyncio/connection.py — await connection.disconnect() inside
  async with self._lock

The async cluster needs the background task only because ClusterNode.release() is def,
not async def.

Proposed fix

Since the connection is guaranteed closed at that point, stop deferring: have release()
append it to _free directly (guarded on is_connected so a still-open connection is not
silently returned), or make release() a coroutine that awaits the disconnect inline, matching
the two pools above. Either removes the window, and the first also removes the
_background_tasks machinery from this path — a net reduction in complexity rather than a new
public contract.

A regression test should drive the real path (execute_command with a concurrent re-mark),
not construct the state by calling node.release() directly.

Related

• PR Wait for pending async cluster pool cleanup #4219 proposes solving this by adding a second, waiting acquisition API
  (acquire_connection_async). That direction adds public surface, an untimed wait that works
  against the deliberate fast-fail above, and a new await in the transaction path, which runs
  inside asyncio.run() on a worker thread while the owning loop is blocked. Filing this issue
  to capture the underlying defect independently of that approach.
• Async RedisCluster: ClusterNode.execute_command leaks a pool connection on every write-side error (send_packed_command outside try/finally) -> permanent MaxConnectionsError #4110 / Fix async cluster node connection release on write errors #4111 — the monotonic slot leak with the same symptom, fixed in 8.1.0.
• RedisClusterClient enters unrecoverable state during AWS ElastiCache upgrades #3929 — original vague report; its persistent variant was Async RedisCluster: ClusterNode.execute_command leaks a pool connection on every write-side error (send_packed_command outside try/finally) -> permanent MaxConnectionsError #4110.

Secondary: hardening, no demonstrated reproduction

Stated explicitly as an invariant gap rather than a confirmed bug: ClusterNode._free is
collections.deque(maxlen=self.max_connections) (1668) and release() carries no ownership or
duplicate-release guard, unlike sync ConnectionPool.release() which drops unowned connections
via _in_use_connections.remove() / owns_connection(). If any path ever released the same
connection twice, append on a full bounded deque silently evicts from the opposite end —
losing a pool slot permanently — or the same connection could be handed to two concurrent
tasks. I did not find a reachable double-release path; recording it so the invariant is
asserted rather than assumed.