Skip to content

async ValkeyCluster drains startup_nodes to empty and never reconnects after failover — port redis-py #3917 #327

Description

@bsimms804

Summary

The async cluster client removes a node from startup_nodes on every ConnectionError/TimeoutError and only ever repopulates it on a successful rediscovery. If a transient event makes all nodes unreachable at once (an ElastiCache failover/scaling event, a brief DNS/network blip), every seed is popped, startup_nodes becomes empty, and from then on every command raises:

valkey.exceptions.ValkeyClusterException: Valkey Cluster cannot be connected.
Please provide at least one reachable node: None

The client never recovers on its own even after the cluster is healthy again — the process must be restarted. The literal : None is the tell: NodesManager.initialize() iterates an empty startup_nodes, so exception is never assigned and str(None) is interpolated.

This is the same bug reported and fixed in redis-py (redis/redis-py#3284, redis/redis-py#3310; fixed in redis-py ≥ 7.2.0 via redis/redis-py#3917). valkey-py forked the code and has not ported the fix.

Environment

  • valkey-py: 6.1.1 (also present on main)
  • Python: 3.11
  • Server: AWS ElastiCache Serverless for Valkey (engine 8.1), cluster mode, TLS. CLUSTER SLOTS returns a single shard with a primary (:6379) and replica (:6380) that share one hostname.

Why Serverless is especially affected

Because the primary and replica share a single hostname, any endpoint-level blip (failover, scale, DNS) makes both nodes unreachable simultaneously, so the "all seeds popped" condition is easy to hit. Multi-shard provisioned clusters have many seed nodes and rarely drain all of them, which is likely why this is under-reported.

Root cause (valkey-py 6.1.1, valkey/asyncio/cluster.py)

  1. On a connection error, ValkeyCluster._execute_command removes the node from startup_nodes (line ~821):

    except (ConnectionError, TimeoutError):
        self.nodes_manager.startup_nodes.pop(target_node.name, None)
        await self.aclose()
        raise
  2. NodesManager.initialize() only iterates startup_nodes (line ~1296); with an empty map the loop body never runs, exception stays None, and it raises (lines ~1389-1393):

    for startup_node in tuple(self.startup_nodes.values()):   # empty -> body never runs
        ...
    if not startup_nodes_reachable:
        raise ValkeyClusterException(
            f"...Please provide at least one reachable node: {str(exception)}"   # -> "None"
        )
  3. startup_nodes is only ever repopulated on a successful init, and only when dynamic_startup_nodes is set (lines ~1409-1411) — which cannot happen once it is empty:

    if self._dynamic_startup_nodes:
        self.set_nodes(self.startup_nodes, self.nodes_cache, remove_old=True)

The failure is independent of dynamic_startup_nodes (both True and False drain to empty and fail identically — verified) and independent of retry (a default Retry is already applied; retries only delay the eventual pop). The sync client (valkey/cluster.py) has the same pattern.

Minimal reproduction

The unrecoverable state is exactly what the library's own error handler produces when all nodes are briefly unreachable. It can be demonstrated deterministically:

import asyncio
from valkey.asyncio.cluster import ValkeyCluster

async def main():
    c = ValkeyCluster.from_url("rediss://<cluster-endpoint>:6379")
    await c.set("k", "v")                                   # discovers topology

    # What _execute_command does on a ConnectionError against every node
    # (e.g. all nodes unreachable together during a failover/scale event):
    for name in list(c.nodes_manager.startup_nodes):
        c.nodes_manager.startup_nodes.pop(name, None)

    # Any subsequent command forces rediscovery, which now has no seed to try:
    await c.nodes_manager.initialize()
    # -> ValkeyClusterException: Valkey Cluster cannot be connected.
    #    Please provide at least one reachable node: None
    #    ...and it stays this way for every command until the process restarts,
    #    even after the cluster is fully healthy again.

asyncio.run(main())

(In production the pops happen inside _execute_command during a real all-nodes-unreachable window — no manual popping required.)

Expected behavior

The client should retain at least one seed (the configured endpoint) so it can rediscover the topology once the cluster is healthy again, instead of requiring a restart.

Suggested fix

Port redis-py#3917: never remove the last seed. redis-py replaced the draining pop with move_node_to_end_of_cached_nodes(node_name), which pops-and-reinserts only when len(startup_nodes) > 1, so the seed set can never empty:

def move_node_to_end_of_cached_nodes(self, node_name: str) -> None:
    if node_name in self.startup_nodes and len(self.startup_nodes) > 1:
        node = self.startup_nodes.pop(node_name)
        self.startup_nodes[node_name] = node   # move to end; never drop the last one

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Status
    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions