Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ClusterPipeline Doesn't Handle ConnectionError for Dead Hosts #2224

Closed
grippy opened this issue Jun 9, 2022 · 1 comment
Closed

ClusterPipeline Doesn't Handle ConnectionError for Dead Hosts #2224

grippy opened this issue Jun 9, 2022 · 1 comment

Comments

@grippy
Copy link
Contributor

grippy commented Jun 9, 2022

Version: What redis-py and what redis version is the issue happening on?

4.3.2

Platform: What platform / version? (For example Python 3.5.1 on Windows 7 / Ubuntu 15.10 / Azure)

py3.10 / Ubuntu (v???) / AWS ElastiCache (redis v6.2.6)

Description: Description of your issue, stack traces from errors and code that reproduces the issue

  1. Some long processes instantiates a RedisCluster client.
  2. Redis cluster is auto-scaled down by AWS.
  3. This error bubbles up while working with a ClusterPipeline:
OSError: [Errno 113] No route to host
  File "redis/connection.py", line 609, in connect
    sock = self.retry.call_with_retry(
  File "redis/retry.py", line 46, in call_with_retry
    return do()
  File "redis/connection.py", line 610, in <lambda>
    lambda: self._connect(), lambda error: self.disconnect(error)
  File "redis/connection.py", line 675, in _connect
    raise err
  File "redis/connection.py", line 663, in _connect
    sock.connect(socket_address)

ConnectionError: Error 113 connecting to xxx.xx.xxx.xx:6379. No route to host.
  File "some_script.py", line 140, in get_users_from_cache
    users = await read_cache(keys)
  File "some_script.py", line 93, in read_cache
    return multi.execute()
  File "redis/cluster.py", line 1869, in execute
    return self.send_cluster_commands(stack, raise_on_error)
  File "redis/cluster.py", line 1928, in send_cluster_commands
    return self._send_cluster_commands(
  File "redis/cluster.py", line 1976, in _send_cluster_commands
    connection = get_connection(redis_node, c.args)
  File "redis/cluster.py", line 49, in get_connection
    return redis_node.connection or redis_node.connection_pool.get_connection(
  File "redis/connection.py", line 1383, in get_connection
    connection.connect()
  File "redis/connection.py", line 615, in connect
    raise ConnectionError(self._error_message(e))

Context:
https://github.com/Flipboard/redis-py/blob/a2365d1582dc59d28c262cd410c0b126a4e5adc6/redis/cluster.py#L1961-L1981

Since node_manager is passed into ClusterPipeline, this might be recoverable from the RedisCluster parent if calling commands other than .pipeline().

Somewhere inside _send_cluster_commands needs to account for ConnectionError while calling get_connection (just haven't figured out the best approach yet). Open to ideas and working on a solution for this.

@grippy grippy changed the title ClusterPipeline Doesn't Handle ConnectionError ClusterPipeline Doesn't Handle ConnectionError for Dead Hosts Jun 10, 2022
@dvora-h
Copy link
Collaborator

dvora-h commented Aug 2, 2022

Fixed in #2225

@dvora-h dvora-h closed this as completed Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants