From 05828bb7a6b9b96adf3a1e3b143e5bf19a4db6cb Mon Sep 17 00:00:00 2001 From: Willian Moreira Date: Wed, 21 Feb 2024 14:08:42 -0300 Subject: [PATCH 01/10] change if the cluster-mode is enabled by trying run CLUSTER SLOT insted of INFO --- redis/cluster.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/redis/cluster.py b/redis/cluster.py index ba25b92246..bef10e88a7 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -1525,11 +1525,12 @@ def initialize(self): ) self.startup_nodes[startup_node.name].redis_connection = r # Make sure cluster mode is enabled on this node - if bool(r.info().get("cluster_enabled")) is False: + try: + cluster_slots = str_if_bytes(r.execute_command("CLUSTER SLOTS")) + except ResponseError as e: raise RedisClusterException( "Cluster mode is not enabled on this node" ) - cluster_slots = str_if_bytes(r.execute_command("CLUSTER SLOTS")) startup_nodes_reachable = True except Exception as e: # Try the next startup node. From 8e121265b64f23daddb92479d01b2d36472301f2 Mon Sep 17 00:00:00 2001 From: Willian Moreira Date: Wed, 21 Feb 2024 14:24:38 -0300 Subject: [PATCH 02/10] fix typo --- redis/cluster.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redis/cluster.py b/redis/cluster.py index bef10e88a7..ce27c35ae7 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -1529,7 +1529,7 @@ def initialize(self): cluster_slots = str_if_bytes(r.execute_command("CLUSTER SLOTS")) except ResponseError as e: raise RedisClusterException( - "Cluster mode is not enabled on this node" + "Cluster mode is not enabled on this node." ) startup_nodes_reachable = True except Exception as e: From 0723b3dbff17ccbe7aebcde74be6dff4c0dba0a6 Mon Sep 17 00:00:00 2001 From: Willian Moreira Date: Thu, 22 Feb 2024 16:08:40 -0300 Subject: [PATCH 03/10] fixing cluster mode is not enabled on this node tests --- tests/test_asyncio/test_cluster.py | 10 +++++++--- tests/test_cluster.py | 14 ++++++++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/tests/test_asyncio/test_cluster.py b/tests/test_asyncio/test_cluster.py index a57d32f5d2..d9a496d1c7 100644 --- a/tests/test_asyncio/test_cluster.py +++ b/tests/test_asyncio/test_cluster.py @@ -127,7 +127,7 @@ async def slowlog(r: RedisCluster) -> None: await r.config_set("slowlog-max-len", old_max_length_value) -async def get_mocked_redis_client(*args, **kwargs) -> RedisCluster: +async def get_mocked_redis_client(cluster_raise_error = False, *args, **kwargs) -> RedisCluster: """ Return a stable RedisCluster object that have deterministic nodes and slots setup to remove the problem of different IP addresses @@ -140,8 +140,11 @@ async def get_mocked_redis_client(*args, **kwargs) -> RedisCluster: async def execute_command(*_args, **_kwargs): if _args[0] == "CLUSTER SLOTS": - mock_cluster_slots = cluster_slots - return mock_cluster_slots + if cluster_raise_error: + raise ResponseError() + else: + mock_cluster_slots = cluster_slots + return mock_cluster_slots elif _args[0] == "COMMAND": return {"get": [], "set": []} elif _args[0] == "INFO": @@ -2458,6 +2461,7 @@ async def test_init_slots_cache_cluster_mode_disabled(self) -> None: """ with pytest.raises(RedisClusterException) as e: rc = await get_mocked_redis_client( + host=default_host, port=default_port, cluster_enabled=False ) await rc.aclose() diff --git a/tests/test_cluster.py b/tests/test_cluster.py index 8a44d45ea3..01868604f7 100644 --- a/tests/test_cluster.py +++ b/tests/test_cluster.py @@ -151,7 +151,7 @@ def cleanup(): r.config_set("slowlog-max-len", 128) -def get_mocked_redis_client(func=None, *args, **kwargs): +def get_mocked_redis_client(func=None, cluster_raise_error=False, *args, **kwargs): """ Return a stable RedisCluster object that have deterministic nodes and slots setup to remove the problem of different IP addresses @@ -164,8 +164,11 @@ def get_mocked_redis_client(func=None, *args, **kwargs): def execute_command(*_args, **_kwargs): if _args[0] == "CLUSTER SLOTS": - mock_cluster_slots = cluster_slots - return mock_cluster_slots + if cluster_raise_error: + raise ResponseError() + else: + mock_cluster_slots = cluster_slots + return mock_cluster_slots elif _args[0] == "COMMAND": return {"get": [], "set": []} elif _args[0] == "INFO": @@ -2654,7 +2657,10 @@ def test_init_slots_cache_cluster_mode_disabled(self): """ with pytest.raises(RedisClusterException) as e: get_mocked_redis_client( - host=default_host, port=default_port, cluster_enabled=False + cluster_raise_error=True, + host=default_host, + port=default_port, + cluster_enabled=False ) assert "Cluster mode is not enabled on this node" in str(e.value) From 0dc61ed2cb2475eb8e963db68ecb86cbf33a1036 Mon Sep 17 00:00:00 2001 From: Willian Moreira Date: Thu, 22 Feb 2024 16:10:34 -0300 Subject: [PATCH 04/10] remove changes on asyncio --- tests/test_asyncio/test_cluster.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/test_asyncio/test_cluster.py b/tests/test_asyncio/test_cluster.py index d9a496d1c7..a57d32f5d2 100644 --- a/tests/test_asyncio/test_cluster.py +++ b/tests/test_asyncio/test_cluster.py @@ -127,7 +127,7 @@ async def slowlog(r: RedisCluster) -> None: await r.config_set("slowlog-max-len", old_max_length_value) -async def get_mocked_redis_client(cluster_raise_error = False, *args, **kwargs) -> RedisCluster: +async def get_mocked_redis_client(*args, **kwargs) -> RedisCluster: """ Return a stable RedisCluster object that have deterministic nodes and slots setup to remove the problem of different IP addresses @@ -140,11 +140,8 @@ async def get_mocked_redis_client(cluster_raise_error = False, *args, **kwargs) async def execute_command(*_args, **_kwargs): if _args[0] == "CLUSTER SLOTS": - if cluster_raise_error: - raise ResponseError() - else: - mock_cluster_slots = cluster_slots - return mock_cluster_slots + mock_cluster_slots = cluster_slots + return mock_cluster_slots elif _args[0] == "COMMAND": return {"get": [], "set": []} elif _args[0] == "INFO": @@ -2461,7 +2458,6 @@ async def test_init_slots_cache_cluster_mode_disabled(self) -> None: """ with pytest.raises(RedisClusterException) as e: rc = await get_mocked_redis_client( - host=default_host, port=default_port, cluster_enabled=False ) await rc.aclose() From 88d946327b4bba520ab19ae8205109a6ec247d02 Mon Sep 17 00:00:00 2001 From: Willian Moreira Date: Thu, 22 Feb 2024 16:14:46 -0300 Subject: [PATCH 05/10] rename mock flag to be more consistent --- tests/test_cluster.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_cluster.py b/tests/test_cluster.py index 01868604f7..d530a21e81 100644 --- a/tests/test_cluster.py +++ b/tests/test_cluster.py @@ -151,7 +151,7 @@ def cleanup(): r.config_set("slowlog-max-len", 128) -def get_mocked_redis_client(func=None, cluster_raise_error=False, *args, **kwargs): +def get_mocked_redis_client(func=None, cluster_slots_raise_error=False, *args, **kwargs): """ Return a stable RedisCluster object that have deterministic nodes and slots setup to remove the problem of different IP addresses @@ -164,7 +164,7 @@ def get_mocked_redis_client(func=None, cluster_raise_error=False, *args, **kwarg def execute_command(*_args, **_kwargs): if _args[0] == "CLUSTER SLOTS": - if cluster_raise_error: + if cluster_slots_raise_error: raise ResponseError() else: mock_cluster_slots = cluster_slots @@ -2657,7 +2657,7 @@ def test_init_slots_cache_cluster_mode_disabled(self): """ with pytest.raises(RedisClusterException) as e: get_mocked_redis_client( - cluster_raise_error=True, + cluster_slots_raise_error=True, host=default_host, port=default_port, cluster_enabled=False From 974827399060a8e7678e975a952ea3369f3f4523 Mon Sep 17 00:00:00 2001 From: Willian Moreira Date: Sun, 10 Mar 2024 11:48:37 -0300 Subject: [PATCH 06/10] optimizing async cluster creation using CLUSTER SLOT command instead of INFO command --- redis/asyncio/cluster.py | 9 ++++----- redis/cluster.py | 2 +- tests/test_asyncio/test_cluster.py | 15 +++++++++++---- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/redis/asyncio/cluster.py b/redis/asyncio/cluster.py index 4fb2fc4647..1a6d3d6bef 100644 --- a/redis/asyncio/cluster.py +++ b/redis/asyncio/cluster.py @@ -1253,13 +1253,12 @@ async def initialize(self) -> None: for startup_node in self.startup_nodes.values(): try: # Make sure cluster mode is enabled on this node - if not (await startup_node.execute_command("INFO")).get( - "cluster_enabled" - ): + try: + cluster_slots = await startup_node.execute_command("CLUSTER SLOTS") + except ResponseError: raise RedisClusterException( - "Cluster mode is not enabled on this node" + "Cluster mode is not enabled on this node." ) - cluster_slots = await startup_node.execute_command("CLUSTER SLOTS") startup_nodes_reachable = True except Exception as e: # Try the next startup node. diff --git a/redis/cluster.py b/redis/cluster.py index ce27c35ae7..8e48190c11 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -1527,7 +1527,7 @@ def initialize(self): # Make sure cluster mode is enabled on this node try: cluster_slots = str_if_bytes(r.execute_command("CLUSTER SLOTS")) - except ResponseError as e: + except ResponseError: raise RedisClusterException( "Cluster mode is not enabled on this node." ) diff --git a/tests/test_asyncio/test_cluster.py b/tests/test_asyncio/test_cluster.py index a57d32f5d2..ae71761ba1 100644 --- a/tests/test_asyncio/test_cluster.py +++ b/tests/test_asyncio/test_cluster.py @@ -127,7 +127,7 @@ async def slowlog(r: RedisCluster) -> None: await r.config_set("slowlog-max-len", old_max_length_value) -async def get_mocked_redis_client(*args, **kwargs) -> RedisCluster: +async def get_mocked_redis_client(cluster_slots_raise_error=False, *args, **kwargs) -> RedisCluster: """ Return a stable RedisCluster object that have deterministic nodes and slots setup to remove the problem of different IP addresses @@ -139,9 +139,13 @@ async def get_mocked_redis_client(*args, **kwargs) -> RedisCluster: with mock.patch.object(ClusterNode, "execute_command") as execute_command_mock: async def execute_command(*_args, **_kwargs): + if _args[0] == "CLUSTER SLOTS": - mock_cluster_slots = cluster_slots - return mock_cluster_slots + if cluster_slots_raise_error: + raise ResponseError() + else: + mock_cluster_slots = cluster_slots + return mock_cluster_slots elif _args[0] == "COMMAND": return {"get": [], "set": []} elif _args[0] == "INFO": @@ -2458,7 +2462,10 @@ async def test_init_slots_cache_cluster_mode_disabled(self) -> None: """ with pytest.raises(RedisClusterException) as e: rc = await get_mocked_redis_client( - host=default_host, port=default_port, cluster_enabled=False + cluster_slots_raise_error=True, + host=default_host, + port=default_port, + cluster_enabled=False ) await rc.aclose() assert "Cluster mode is not enabled on this node" in str(e.value) From 33052ad606ec0b50c3e7a0cc918ad6da103ca796 Mon Sep 17 00:00:00 2001 From: Willian Moreira Date: Sun, 10 Mar 2024 14:13:02 -0300 Subject: [PATCH 07/10] fixing test. Before INFO and CLUSTER_SLOT was used for performing the connection, now only the CLUSTER_SLOT, so the total commands is minus 1 --- tests/test_asyncio/test_cluster.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_asyncio/test_cluster.py b/tests/test_asyncio/test_cluster.py index ae71761ba1..7748658845 100644 --- a/tests/test_asyncio/test_cluster.py +++ b/tests/test_asyncio/test_cluster.py @@ -2726,10 +2726,10 @@ async def parse_response( async with r.pipeline() as pipe: with pytest.raises(ClusterDownError): await pipe.get(key).execute() - + print(node.parse_response.await_count) assert ( node.parse_response.await_count - == 4 * r.cluster_error_retry_attempts - 3 + == 3 * r.cluster_error_retry_attempts - 2 ) async def test_connection_error_not_raised(self, r: RedisCluster) -> None: From 6afc70650c424ccb56e8dcf1ff73a7c19edf12b4 Mon Sep 17 00:00:00 2001 From: Willian Moreira Date: Sun, 10 Mar 2024 14:17:49 -0300 Subject: [PATCH 08/10] remove dot at the end of string --- redis/asyncio/cluster.py | 2 +- redis/cluster.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/redis/asyncio/cluster.py b/redis/asyncio/cluster.py index 1a6d3d6bef..11c423b848 100644 --- a/redis/asyncio/cluster.py +++ b/redis/asyncio/cluster.py @@ -1257,7 +1257,7 @@ async def initialize(self) -> None: cluster_slots = await startup_node.execute_command("CLUSTER SLOTS") except ResponseError: raise RedisClusterException( - "Cluster mode is not enabled on this node." + "Cluster mode is not enabled on this node" ) startup_nodes_reachable = True except Exception as e: diff --git a/redis/cluster.py b/redis/cluster.py index 8e48190c11..a9213f4235 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -1529,7 +1529,7 @@ def initialize(self): cluster_slots = str_if_bytes(r.execute_command("CLUSTER SLOTS")) except ResponseError: raise RedisClusterException( - "Cluster mode is not enabled on this node." + "Cluster mode is not enabled on this node" ) startup_nodes_reachable = True except Exception as e: From c6ae58869d2ce5e70b522d647d55b5f452649362 Mon Sep 17 00:00:00 2001 From: Willian Moreira Date: Sun, 10 Mar 2024 14:19:45 -0300 Subject: [PATCH 09/10] remove unecessary print from test --- tests/test_asyncio/test_cluster.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_asyncio/test_cluster.py b/tests/test_asyncio/test_cluster.py index 7748658845..eb5f087296 100644 --- a/tests/test_asyncio/test_cluster.py +++ b/tests/test_asyncio/test_cluster.py @@ -2726,7 +2726,6 @@ async def parse_response( async with r.pipeline() as pipe: with pytest.raises(ClusterDownError): await pipe.get(key).execute() - print(node.parse_response.await_count) assert ( node.parse_response.await_count == 3 * r.cluster_error_retry_attempts - 2 From 0b35c47b2e0752e5cdc117842bf29a0227173050 Mon Sep 17 00:00:00 2001 From: Willian Moreira Date: Mon, 11 Mar 2024 10:27:23 -0300 Subject: [PATCH 10/10] fix lint problems --- tests/test_asyncio/test_cluster.py | 6 ++++-- tests/test_cluster.py | 10 ++++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/test_asyncio/test_cluster.py b/tests/test_asyncio/test_cluster.py index eb5f087296..d7554b12a5 100644 --- a/tests/test_asyncio/test_cluster.py +++ b/tests/test_asyncio/test_cluster.py @@ -127,7 +127,9 @@ async def slowlog(r: RedisCluster) -> None: await r.config_set("slowlog-max-len", old_max_length_value) -async def get_mocked_redis_client(cluster_slots_raise_error=False, *args, **kwargs) -> RedisCluster: +async def get_mocked_redis_client( + cluster_slots_raise_error=False, *args, **kwargs +) -> RedisCluster: """ Return a stable RedisCluster object that have deterministic nodes and slots setup to remove the problem of different IP addresses @@ -2465,7 +2467,7 @@ async def test_init_slots_cache_cluster_mode_disabled(self) -> None: cluster_slots_raise_error=True, host=default_host, port=default_port, - cluster_enabled=False + cluster_enabled=False, ) await rc.aclose() assert "Cluster mode is not enabled on this node" in str(e.value) diff --git a/tests/test_cluster.py b/tests/test_cluster.py index d530a21e81..1f505b816d 100644 --- a/tests/test_cluster.py +++ b/tests/test_cluster.py @@ -151,7 +151,9 @@ def cleanup(): r.config_set("slowlog-max-len", 128) -def get_mocked_redis_client(func=None, cluster_slots_raise_error=False, *args, **kwargs): +def get_mocked_redis_client( + func=None, cluster_slots_raise_error=False, *args, **kwargs +): """ Return a stable RedisCluster object that have deterministic nodes and slots setup to remove the problem of different IP addresses @@ -2658,9 +2660,9 @@ def test_init_slots_cache_cluster_mode_disabled(self): with pytest.raises(RedisClusterException) as e: get_mocked_redis_client( cluster_slots_raise_error=True, - host=default_host, - port=default_port, - cluster_enabled=False + host=default_host, + port=default_port, + cluster_enabled=False, ) assert "Cluster mode is not enabled on this node" in str(e.value)