Changed get_default_backoff to default_backoff, removed retry_on_error and connection_error_retry_attempts from RedisCluster, default retry changed to no retries

Author: barshaul

redis/__init__.py

@@ -1,6 +1,6 @@
 import sys
 
-from redis.backoff import get_default_backoff
+from redis.backoff import default_backoff
 from redis.client import Redis, StrictRedis
 from redis.cluster import RedisCluster
 from redis.connection import (
@@ -65,7 +65,7 @@ def int_or_str(value):
     "ConnectionPool",
     "DataError",
     "from_url",
-    "get_default_backoff",
+    "default_backoff",
     "InvalidResponse",
     "PubSubError",
     "ReadOnlyError",

redis/asyncio/__init__.py

@@ -15,7 +15,7 @@
     SentinelManagedSSLConnection,
 )
 from redis.asyncio.utils import from_url
-from redis.backoff import get_default_backoff
+from redis.backoff import default_backoff
 from redis.exceptions import (
     AuthenticationError,
     AuthenticationWrongNumberOfArgsError,
@@ -44,7 +44,7 @@
     "ConnectionPool",
     "DataError",
     "from_url",
-    "get_default_backoff",
+    "default_backoff",
     "InvalidResponse",
     "PubSubError",
     "ReadOnlyError",

redis/asyncio/cluster.py

@@ -26,7 +26,7 @@
 )
 from redis.asyncio.parser import CommandsParser
 from redis.asyncio.retry import Retry
-from redis.backoff import get_default_backoff
+from redis.backoff import default_backoff
 from redis.client import EMPTY_RESPONSE, NEVER_DECODE, AbstractRedis
 from redis.cluster import (
     PIPELINE_BLOCKED_COMMANDS,
@@ -138,7 +138,7 @@ class RedisCluster(AbstractRedis, AbstractRedisCluster, AsyncRedisClusterCommand
         | Number of times to retry before reinitializing when :class:`~.TimeoutError`
           or :class:`~.ConnectionError` are encountered.
           The default backoff strategy will be set if Retry object is not passed (see
-          get_default_backoff in backoff.py). To change it, pass a custom Retry object
+          default_backoff in backoff.py). To change it, pass a custom Retry object
           using the "retry" keyword.
     :param max_connections:
         | Maximum number of connections per node. If there are no free connections & the
@@ -309,7 +309,7 @@ def __init__(
         if retry or retry_on_error or connection_error_retry_attempts > 0:
             # Set a retry object for all cluster nodes
             self.retry = retry or Retry(
-                get_default_backoff(), connection_error_retry_attempts
+                default_backoff(), connection_error_retry_attempts
             )
             if retry_on_error is None:
                 # Default errors for retrying
@@ -642,7 +642,9 @@ async def execute_command(self, *args: EncodableT, **kwargs: Any) -> Any:
             target_nodes_specified = True
             retry_attempts = 0
 
-        while True:
+        # Add one for the first execution
+        execute_attempts = 1 + retry_attempts
+        for _ in range(execute_attempts):
             if self._initialize:
                 await self.initialize()
             try:

redis/backoff.py

@@ -1,7 +1,9 @@
 import random
 from abc import ABC, abstractmethod
 
+# Maximum backoff between each retry in seconds
 DEFAULT_CAP = 0.512
+# Minimum backoff between each retry in seconds
 DEFAULT_BASE = 0.008
 
 
@@ -108,5 +110,5 @@ def compute(self, failures):
         return self._previous_backoff
 
 
-def get_default_backoff():
+def default_backoff():
     return EqualJitterBackoff()

redis/cluster.py

@@ -6,7 +6,7 @@
 from collections import OrderedDict
 from typing import Any, Callable, Dict, List, Optional, Tuple, Union
 
-from redis.backoff import get_default_backoff
+from redis.backoff import default_backoff, NoBackoff
 from redis.client import CaseInsensitiveDict, PubSub, Redis, parse_scan
 from redis.commands import READ_COMMANDS, CommandsParser, RedisClusterCommands
 from redis.connection import ConnectionPool, DefaultParser, Encoder, parse_url
@@ -430,9 +430,7 @@ def __init__(
         port: int = 6379,
         startup_nodes: Optional[List["ClusterNode"]] = None,
         cluster_error_retry_attempts: int = 3,
-        connection_error_retry_attempts: int = 3,
         retry: Optional["Retry"] = None,
-        retry_on_error: Optional[List[Exception]] = None,
         require_full_coverage: bool = False,
         reinitialize_steps: int = 10,
         read_from_replicas: bool = False,
@@ -476,13 +474,7 @@ def __init__(
              Number of times to retry before raising an error when
              :class:`~.TimeoutError` or :class:`~.ConnectionError` or
              :class:`~.ClusterDownError` are encountered
-        :param connection_error_retry_attempts:
-            Number of times to retry before reinitializing when :class:`~.TimeoutError`
-            or :class:`~.ConnectionError` are encountered.
-            The default backoff strategy will be set if Retry object is not passed (see
-            get_default_backoff in backoff.py). To change it, pass a custom Retry object
-            using the "retry" keyword.
-        :reinitialize_steps: 'int'
+        :param reinitialize_steps:
             Specifies the number of MOVED errors that need to occur before
             reinitializing the whole cluster topology. If a MOVED error occurs
             and the cluster does not need to be reinitialized on this current
@@ -550,17 +542,11 @@ def __init__(
         self.user_on_connect_func = kwargs.pop("redis_connect_func", None)
         kwargs.update({"redis_connect_func": self.on_connect})
         kwargs = cleanup_kwargs(**kwargs)
-
-        if retry or retry_on_error or connection_error_retry_attempts > 0:
-            # Set a retry object for all cluster nodes
-            self.retry = retry or Retry(
-                get_default_backoff(), connection_error_retry_attempts
-            )
-            if retry_on_error is None:
-                # Default errors for retrying
-                retry_on_error = [ConnectionError, TimeoutError]
-            self.retry.update_supported_errors(retry_on_error)
+        if retry:
+            self.retry = retry
             kwargs.update({"retry": self.retry})
+        else:
+            kwargs.update({"retry": Retry(default_backoff(), 0)})
 
         self.encoder = Encoder(
             kwargs.get("encoding", "utf-8"),
@@ -1019,7 +1005,9 @@ def execute_command(self, *args, **kwargs):
         retry_attempts = (
             0 if target_nodes_specified else self.cluster_error_retry_attempts
         )
-        while True:
+        # Add one for the first execution
+        execute_attempts = 1 + retry_attempts
+        for _ in range(execute_attempts):
             try:
                 res = {}
                 if not target_nodes_specified:

tests/test_asyncio/test_cluster.py

@@ -14,7 +14,7 @@
 from redis.asyncio.connection import Connection, SSLConnection
 from redis.asyncio.parser import CommandsParser
 from redis.asyncio.retry import Retry
-from redis.backoff import ExponentialBackoff, NoBackoff, get_default_backoff
+from redis.backoff import ExponentialBackoff, NoBackoff, default_backoff
 from redis.cluster import PIPELINE_BLOCKED_COMMANDS, PRIMARY, REPLICA, get_node_name
 from redis.crc import REDIS_CLUSTER_HASH_SLOTS, key_slot
 from redis.exceptions import (
@@ -278,7 +278,7 @@ async def test_cluster_retry_object(self, request: FixtureRequest) -> None:
             retry = rc_default.connection_kwargs.get("retry")
             assert isinstance(retry, Retry)
             assert retry._retries == 3
-            assert isinstance(retry._backoff, type(get_default_backoff()))
+            assert isinstance(retry._backoff, type(default_backoff()))
             assert rc_default.get_node("127.0.0.1", 16379).connection_kwargs.get(
                 "retry"
             ) == rc_default.get_node("127.0.0.1", 16380).connection_kwargs.get("retry")

tests/test_cluster.py

@@ -7,7 +7,7 @@
 import pytest
 
 from redis import Redis
-from redis.backoff import ExponentialBackoff, NoBackoff, get_default_backoff
+from redis.backoff import ExponentialBackoff, NoBackoff, default_backoff
 from redis.cluster import (
     PRIMARY,
     REDIS_CLUSTER_HASH_SLOTS,
@@ -770,8 +770,8 @@ def test_cluster_retry_object(self, r) -> None:
         # Test default retry
         retry = r.get_connection_kwargs().get("retry")
         assert isinstance(retry, Retry)
-        assert retry._retries == 3
-        assert isinstance(retry._backoff, type(get_default_backoff()))
+        assert retry._retries == 0
+        assert isinstance(retry._backoff, type(default_backoff()))
         node1 = r.get_node("127.0.0.1", 16379).redis_connection
         node2 = r.get_node("127.0.0.1", 16380).redis_connection
         assert node1.get_retry()._retries == node2.get_retry()._retries
@@ -786,22 +786,6 @@ def test_cluster_retry_object(self, r) -> None:
             == retry._retries
         )
 
-        # Test no connection retries
-        rc_no_retries = RedisCluster(
-            "127.0.0.1", 16379, connection_error_retry_attempts=0
-        )
-        assert (
-            rc_no_retries.get_node("127.0.0.1", 16379).redis_connection.get_retry()
-            is None
-        )
-        rc_no_retries = RedisCluster("127.0.0.1", 16379, retry=Retry(NoBackoff(), 0))
-        assert (
-            rc_no_retries.get_node("127.0.0.1", 16379)
-            .redis_connection.get_retry()
-            ._retries
-            == 0
-        )
-
 
 @pytest.mark.onlycluster
 class TestClusterRedisCommands: