Repository: redis/redis-py
Pull request #2244 (2022-06-22 13:35:15)

Added dynamic_startup_nodes configuration to RedisCluster.
### Pull Request check-list

_Please make sure to review and check all of these items:_

- [x] Does `$ tox` pass with this change (including linting)?
- [x] Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
- [x] Is the new or changed code fully tested?
- [x] Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
- [ ] Is there an example added to the examples folder (if applicable)?
- [x] Was the change added to CHANGES file?

_NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open._

### Description of change

Added dynamic_startup_nodes configuration to RedisCluster.
By default, when reinitializing the cluster topology, the nodesManager will use only the initial passed startup nodes to refresh the cluster topology. If dynamic_startup_nodes is set to true, the nodesManager will set the startup nodes to all of the discovered nodes and they will be used to determine the cluster nodes-slots mapping in the next topology refresh.

When true, the initial passed startup nodes will be removed if their endpoints aren't listed in the CLUSTER SLOTS output. For example, if dynamic DNS endpoints are used for startup nodes, but CLUSTER SLOTS lists specific IP addresses, the passed startup nodes will be overridden with the specific nodes' IP. 
Therefore, it is recommended to keep dynamic_startup_nodes set to false with DNS endpoints.



barshaul: > @barshaul tests fails after merge master in.

taking a look.

Patch:
diff --git a/CHANGES b/CHANGES
index 4afc7bac..0af421b4 100644
--- a/CHANGES
+++ b/CHANGES
@@ -11,9 +11,9 @@
     * Fix broken connection writer lock-up for asyncio (#2065)
     * Fix auth bug when provided with no username (#2086)
     * Fix missing ClusterPipeline._lock (#2189)
+    * Added dynaminc_startup_nodes configuration to RedisCluster
     * Fix reusing the old nodes' connections when cluster topology refresh is being done
     * Fix RedisCluster to immediately raise AuthenticationError without a retry
-
 * 4.1.3 (Feb 8, 2022)
     * Fix flushdb and flushall (#1926)
     * Add redis5 and redis4 dockers (#1871)
diff --git a/README.md b/README.md
index f5d4ae54..64c9d280 100644
--- a/README.md
+++ b/README.md
@@ -1006,6 +1006,7 @@ a slots cache which maps each of the 16384 slots to the node/s handling them,
 a nodes cache that contains ClusterNode objects (name, host, port, redis connection)
 for all of the cluster's nodes, and a commands cache contains all the server
 supported commands that were retrieved using the Redis 'COMMAND' output.
+See *RedisCluster specific options* below for more.
 
 RedisCluster instance can be directly used to execute Redis commands. When a
 command is being executed through the cluster instance, the target node(s) will
@@ -1245,6 +1246,55 @@ The following commands are not supported:
 
 Using scripting within pipelines in cluster mode is **not supported**.
 
+
+**RedisCluster specific options**
+
+ require_full_coverage: (default=False)
+ 
+    When set to False (default value): the client will not require a
+    full coverage of the slots. However, if not all slots are covered,
+    and at least one node has 'cluster-require-full-coverage' set to
+    'yes,' the server will throw a ClusterDownError for some key-based
+    commands. See -
+    https://redis.io/topics/cluster-tutorial#redis-cluster-configuration-parameters
+    When set to True: all slots must be covered to construct the
+    cluster client. If not all slots are covered, RedisClusterException
+    will be thrown.
+    
+ read_from_replicas: (default=False)
+
+     Enable read from replicas in READONLY mode. You can read possibly
+     stale data.
+     When set to true, read commands will be assigned between the
+     primary and its replications in a Round-Robin manner.
+     
+ dynamic_startup_nodes: (default=False)
+
+     Set the RedisCluster's startup nodes to all of the discovered nodes.
+     If true, the cluster's discovered nodes will be used to determine the
+     cluster nodes-slots mapping in the next topology refresh.
+     It will remove the initial passed startup nodes if their endpoints aren't
+     listed in the CLUSTER SLOTS output.
+     If you use dynamic DNS endpoints for startup nodes but CLUSTER SLOTS lists
+     specific IP addresses, keep it at false.
+     
+ cluster_error_retry_attempts: (default=3)
+
+     Retry command execution attempts when encountering ClusterDownError
+     or ConnectionError
+     
+ reinitialize_steps: (default=10)
+
+    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
+    error handling, only the MOVED slot will be patched with the
+    redirected node.
+    To reinitialize the cluster on every MOVED error, set
+    reinitialize_steps to 1.
+    To avoid reinitializing the cluster on moved errors, set
+    reinitialize_steps to 0.
+
 ### Author
 
 redis-py is developed and maintained by [Redis Inc](https://redis.com). It can be found [here](
diff --git a/redis/cluster.py b/redis/cluster.py
index 1737ec75..c5b6c8da 100644
--- a/redis/cluster.py
+++ b/redis/cluster.py
@@ -482,6 +482,7 @@ class RedisCluster(AbstractRedisCluster, RedisClusterCommands):
         require_full_coverage=False,
         reinitialize_steps=10,
         read_from_replicas=False,
+        dynamic_startup_nodes=False,
         url=None,
         **kwargs,
     ):
@@ -509,6 +510,14 @@ class RedisCluster(AbstractRedisCluster, RedisClusterCommands):
              stale data.
              When set to true, read commands will be assigned between the
              primary and its replications in a Round-Robin manner.
+         :dynamic_startup_nodes: 'bool'
+             Set the RedisCluster's startup nodes to all of the discovered nodes.
+             If true, the cluster's discovered nodes will be used to determine the
+             cluster nodes-slots mapping in the next topology refresh.
+             It will remove the initial passed startup nodes if their endpoints aren't
+             listed in the CLUSTER SLOTS output.
+             If you use dynamic DNS endpoints for startup nodes but CLUSTER SLOTS lists
+             specific IP addresses, keep it at false.
         :cluster_error_retry_attempts: 'int'
              Retry command execution attempts when encountering ClusterDownError
              or ConnectionError
@@ -598,6 +607,7 @@ class RedisCluster(AbstractRedisCluster, RedisClusterCommands):
             startup_nodes=startup_nodes,
             from_url=from_url,
             require_full_coverage=require_full_coverage,
+            dynamic_startup_nodes=dynamic_startup_nodes,
             **kwargs,
         )
 
@@ -1283,6 +1293,7 @@ class NodesManager:
         from_url=False,
         require_full_coverage=False,
         lock=None,
+        dynamic_startup_nodes=False,
         **kwargs,
     ):
         self.nodes_cache = {}
@@ -1292,6 +1303,7 @@ class NodesManager:
         self.populate_startup_nodes(startup_nodes)
         self.from_url = from_url
         self._require_full_coverage = require_full_coverage
+        self._dynamic_startup_nodes = dynamic_startup_nodes
         self._moved_exception = None
         self.connection_kwargs = kwargs
         self.read_load_balancer = LoadBalancer()
@@ -1612,8 +1624,9 @@ class NodesManager:
         self.slots_cache = tmp_slots
         # Set the default node
         self.default_node = self.get_nodes_by_server_type(PRIMARY)[0]
-        # Populate the startup nodes with all discovered nodes
-        self.startup_nodes = tmp_nodes_cache
+        if self._dynamic_startup_nodes:
+            # Populate the startup nodes with all discovered nodes
+            self.startup_nodes = tmp_nodes_cache
         # If initialize was called after a MovedError, clear it
         self._moved_exception = None
 
diff --git a/redis/commands/core.py b/redis/commands/core.py
index a337d4f3..6d67415a 100644
--- a/redis/commands/core.py
+++ b/redis/commands/core.py
@@ -687,6 +687,12 @@ class ManagementCommands(CommandsProtocol):
         Sets the current connection name
 
         For more information see https://redis.io/commands/client-setname
+
+        .. note::
+           This method sets client name only for **current** connection.
+
+           If you want to set a common name for all connections managed
+           by this client, use ``client_name`` constructor argument.
         """
         return self.execute_command("CLIENT SETNAME", name, **kwargs)
 


Test patch:
diff --git a/tests/test_cluster.py b/tests/test_cluster.py
index 438ef73a..03533230 100644
--- a/tests/test_cluster.py
+++ b/tests/test_cluster.py
@@ -673,7 +673,7 @@ class TestRedisClusterObj:
 
             def moved_redirect_effect(connection, *args, **options):
                 # raise a timeout for 5 times so we'll need to reinitilize the topology
-                if count.val >= 5:
+                if count.val == 4:
                     parse_response.side_effect = real_func
                 count.val += 1
                 raise TimeoutError()
@@ -2285,6 +2285,27 @@ class TestNodesManager:
                 assert rc.get_node(host=default_host, port=7001) is not None
                 assert rc.get_node(host=default_host, port=7002) is not None
 
+    @pytest.mark.parametrize("dynamic_startup_nodes", [True, False])
+    def test_init_slots_dynamic_startup_nodes(self, dynamic_startup_nodes):
+        rc = get_mocked_redis_client(
+            host="my@DNS.com",
+            port=7000,
+            cluster_slots=default_cluster_slots,
+            dynamic_startup_nodes=dynamic_startup_nodes,
+        )
+        # Nodes are taken from default_cluster_slots
+        discovered_nodes = [
+            "127.0.0.1:7000",
+            "127.0.0.1:7001",
+            "127.0.0.1:7002",
+            "127.0.0.1:7003",
+        ]
+        startup_nodes = list(rc.nodes_manager.startup_nodes.keys())
+        if dynamic_startup_nodes is True:
+            assert startup_nodes.sort() == discovered_nodes.sort()
+        else:
+            assert startup_nodes == ["my@DNS.com:7000"]
+
 
 @pytest.mark.onlycluster
 class TestClusterPubSubObject: