From f259ab5299110229d6de818408b793fd39522d7d Mon Sep 17 00:00:00 2001 From: nialdaly Date: Fri, 17 Jun 2022 23:23:15 +0100 Subject: [PATCH 01/13] removed the logging module and its corresponding methods --- redis/cluster.py | 53 +++--------------------------------------------- 1 file changed, 3 insertions(+), 50 deletions(-) diff --git a/redis/cluster.py b/redis/cluster.py index 8e4c654c6b..7f245aad66 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -1,5 +1,4 @@ import copy -import logging import random import socket import sys @@ -38,8 +37,6 @@ str_if_bytes, ) -log = logging.getLogger(__name__) - def get_node_name(host: str, port: int) -> str: return f"{host}:{port}" @@ -571,7 +568,6 @@ def __init__( " RedisCluster(startup_nodes=[ClusterNode('localhost', 6379)," " ClusterNode('localhost', 6378)])" ) - log.debug(f"startup_nodes : {startup_nodes}") # Update the connection arguments # Whenever a new connection is established, RedisCluster's on_connect # method should be run @@ -701,13 +697,8 @@ def set_default_node(self, node): :return True if the default node was set, else False """ if node is None or self.get_node(node_name=node.name) is None: - log.info( - "The requested node does not exist in the cluster, so " - "the default node was not changed." - ) return False self.nodes_manager.default_node = node - log.info(f"Changed the default cluster node to {node}") return True def monitor(self, target_node=None): @@ -851,8 +842,6 @@ def _determine_nodes(self, *args, **kwargs): else: # get the nodes group for this command if it was predefined command_flag = self.command_flags.get(command) - if command_flag: - log.debug(f"Target node/s for {command}: {command_flag}") if command_flag == self.__class__.RANDOM: # return a random node return [self.get_random_node()] @@ -876,7 +865,6 @@ def _determine_nodes(self, *args, **kwargs): node = self.nodes_manager.get_node_from_slot( slot, self.read_from_replicas and command in READ_COMMANDS ) - log.debug(f"Target for {args}: slot {slot}") return [node] def _should_reinitialized(self): @@ -1094,10 +1082,6 @@ def _execute_command(self, target_node, *args, **kwargs): ) moved = False - log.debug( - f"Executing command {command} on target node: " - f"{target_node.server_type} {target_node.name}" - ) redis_node = self.get_redis_connection(target_node) connection = get_connection(redis_node, *args, **kwargs) if asking: @@ -1114,10 +1098,8 @@ def _execute_command(self, target_node, *args, **kwargs): return response except (RedisClusterException, BusyLoadingError) as e: - log.exception(type(e)) - raise + raise e except (ConnectionError, TimeoutError) as e: - log.exception(type(e)) # ConnectionError can also be raised if we couldn't get a # connection from the pool before timing out, so check that # this is an actual connection before attempting to disconnect. @@ -1135,7 +1117,7 @@ def _execute_command(self, target_node, *args, **kwargs): # Hard force of reinitialize of the node/slots setup # and try again with the new setup self.nodes_manager.initialize() - raise + raise e except MovedError as e: # First, we will try to patch the slots/nodes cache with the # redirected node output and try again. If MovedError exceeds @@ -1145,7 +1127,6 @@ def _execute_command(self, target_node, *args, **kwargs): # the same client object is shared between multiple threads. To # reduce the frequency you can set this variable in the # RedisCluster constructor. - log.exception("MovedError") self.reinitialize_counter += 1 if self._should_reinitialized(): self.nodes_manager.initialize() @@ -1155,17 +1136,12 @@ def _execute_command(self, target_node, *args, **kwargs): self.nodes_manager.update_moved_exception(e) moved = True except TryAgainError: - log.exception("TryAgainError") - if ttl < self.RedisClusterRequestTTL / 2: time.sleep(0.05) except AskError as e: - log.exception("AskError") - redirect_addr = get_node_name(host=e.host, port=e.port) asking = True except ClusterDownError as e: - log.exception("ClusterDownError") # ClusterDownError can occur during a failover and to get # self-healed, we will try to reinitialize the cluster layout # and retry executing the command @@ -1173,11 +1149,8 @@ def _execute_command(self, target_node, *args, **kwargs): self.nodes_manager.initialize() raise e except ResponseError as e: - message = e.__str__() - log.exception(f"ResponseError: {message}") raise e except BaseException as e: - log.exception("BaseException") if connection: connection.disconnect() raise e @@ -1312,11 +1285,6 @@ def get_node(self, host=None, port=None, node_name=None): elif node_name: return self.nodes_cache.get(node_name) else: - log.error( - "get_node requires one of the following: " - "1. node name " - "2. host and port" - ) return None def update_moved_exception(self, exception): @@ -1449,7 +1417,6 @@ def initialize(self): :startup_nodes: Responsible for discovering other nodes in the cluster """ - log.debug("Initializing the nodes' topology of the cluster") self.reset() tmp_nodes_cache = {} tmp_slots = {} @@ -1477,17 +1444,9 @@ def initialize(self): ) cluster_slots = str_if_bytes(r.execute_command("CLUSTER SLOTS")) startup_nodes_reachable = True - except (ConnectionError, TimeoutError) as e: - msg = e.__str__ - log.exception( - "An exception occurred while trying to" - " initialize the cluster using the seed node" - f" {startup_node.name}:\n{msg}" - ) + except (ConnectionError, TimeoutError): continue except ResponseError as e: - log.exception('ReseponseError sending "cluster slots" to redis server') - # Isn't a cluster connection, so it won't parse these # exceptions automatically message = e.__str__() @@ -2049,12 +2008,6 @@ def _send_cluster_commands( # If a lot of commands have failed, we'll be setting the # flag to rebuild the slots table from scratch. # So MOVED errors should correct themselves fairly quickly. - log.exception( - f"An exception occurred during pipeline execution. " - f"args: {attempt[-1].args}, " - f"error: {type(attempt[-1].result).__name__} " - f"{str(attempt[-1].result)}" - ) self.reinitialize_counter += 1 if self._should_reinitialized(): self.nodes_manager.initialize() From b19ede760c84309309a6717b04ebf5b061e7f2cb Mon Sep 17 00:00:00 2001 From: nialdaly Date: Fri, 17 Jun 2022 23:30:52 +0100 Subject: [PATCH 02/13] updated CHANGES --- CHANGES | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index b7e238ebb3..e461806db6 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,4 @@ - + * Remove verbose logging from cluster.py * Allow negative `retries` for `Retry` class to retry forever * Add `items` parameter to `hset` signature * Create codeql-analysis.yml (#1988). Thanks @chayim From 7253f4f47c3b326c8b6a3bbe236a2ae43926f70d Mon Sep 17 00:00:00 2001 From: nialdaly Date: Sun, 19 Jun 2022 11:44:31 +0100 Subject: [PATCH 03/13] except block for RedisClusterException and BusyLoadingError removed --- redis/cluster.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/redis/cluster.py b/redis/cluster.py index 7f245aad66..107dc63420 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -1097,8 +1097,6 @@ def _execute_command(self, target_node, *args, **kwargs): ) return response - except (RedisClusterException, BusyLoadingError) as e: - raise e except (ConnectionError, TimeoutError) as e: # ConnectionError can also be raised if we couldn't get a # connection from the pool before timing out, so check that From c5eae343c0dfd7cb0e55e69f71c6794c83771e71 Mon Sep 17 00:00:00 2001 From: nialdaly Date: Sun, 19 Jun 2022 17:34:02 +0100 Subject: [PATCH 04/13] removed unused import (redis.exceptions.BusyLoadingError) --- redis/cluster.py | 1 - 1 file changed, 1 deletion(-) diff --git a/redis/cluster.py b/redis/cluster.py index 107dc63420..d291ec8e3b 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -13,7 +13,6 @@ from redis.crc import REDIS_CLUSTER_HASH_SLOTS, key_slot from redis.exceptions import ( AskError, - BusyLoadingError, ClusterCrossSlotError, ClusterDownError, ClusterError, From a6721b14484c4294ce545fa03c7ad73b3637d08e Mon Sep 17 00:00:00 2001 From: nialdaly Date: Sun, 19 Jun 2022 19:10:57 +0100 Subject: [PATCH 05/13] empty commit to re-trigger Actions workflow From fd50baace562e8220db9bda67ba7fc50a2457278 Mon Sep 17 00:00:00 2001 From: nialdaly Date: Tue, 21 Jun 2022 18:23:44 +0100 Subject: [PATCH 06/13] replaced BaseException with Exception --- redis/cluster.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/redis/cluster.py b/redis/cluster.py index d291ec8e3b..30cdba84c2 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -1041,7 +1041,7 @@ def execute_command(self, *args, **kwargs): res[node.name] = self._execute_command(node, *args, **kwargs) # Return the processed result return self._process_result(args[0], res, **kwargs) - except BaseException as e: + except Exception as e: if type(e) in self.__class__.ERRORS_ALLOW_RETRY: # The nodes and slots cache were reinitialized. # Try again with the new cluster setup. @@ -1147,7 +1147,7 @@ def _execute_command(self, target_node, *args, **kwargs): raise e except ResponseError as e: raise e - except BaseException as e: + except Exception as e: if connection: connection.disconnect() raise e From 1eabc530d87288513e234cba60f3d563301f7f49 Mon Sep 17 00:00:00 2001 From: nialdaly Date: Thu, 23 Jun 2022 19:49:23 +0100 Subject: [PATCH 07/13] empty commit to re-trigger Actions workflow From 284be913c47dd71067517085f6f2aecf4ae3ad9e Mon Sep 17 00:00:00 2001 From: nialdaly Date: Thu, 23 Jun 2022 20:03:16 +0100 Subject: [PATCH 08/13] empty commit to re-trigger Actions workflow From eaa343589c28b5948d4f2078c1b1859da0ba1797 Mon Sep 17 00:00:00 2001 From: nialdaly Date: Fri, 15 Jul 2022 18:19:53 +0100 Subject: [PATCH 09/13] redundant logic removed --- redis/cluster.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/redis/cluster.py b/redis/cluster.py index 264aae9601..1afa4f08f6 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -13,7 +13,6 @@ from redis.crc import REDIS_CLUSTER_HASH_SLOTS, key_slot from redis.exceptions import ( AskError, - AuthenticationError, ClusterCrossSlotError, ClusterDownError, ClusterError, @@ -1061,8 +1060,6 @@ def _execute_command(self, target_node, *args, **kwargs): ) return response - except AuthenticationError as e: - raise e except (ConnectionError, TimeoutError) as e: # ConnectionError can also be raised if we couldn't get a # connection from the pool before timing out, so check that @@ -1113,8 +1110,6 @@ def _execute_command(self, target_node, *args, **kwargs): time.sleep(0.25) self.nodes_manager.initialize() raise e - except ResponseError as e: - raise e except Exception as e: if connection: connection.disconnect() From 336b4d7acaa6bdcab6ff714b920998c0a97e9313 Mon Sep 17 00:00:00 2001 From: nialdaly Date: Fri, 15 Jul 2022 19:36:28 +0100 Subject: [PATCH 10/13] re-trigger pipeline From 0407f21016a1525d2f717bf291dbed64c985d4c3 Mon Sep 17 00:00:00 2001 From: nialdaly Date: Fri, 15 Jul 2022 20:04:00 +0100 Subject: [PATCH 11/13] reverted changes --- redis/cluster.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/redis/cluster.py b/redis/cluster.py index 1afa4f08f6..264aae9601 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -13,6 +13,7 @@ from redis.crc import REDIS_CLUSTER_HASH_SLOTS, key_slot from redis.exceptions import ( AskError, + AuthenticationError, ClusterCrossSlotError, ClusterDownError, ClusterError, @@ -1060,6 +1061,8 @@ def _execute_command(self, target_node, *args, **kwargs): ) return response + except AuthenticationError as e: + raise e except (ConnectionError, TimeoutError) as e: # ConnectionError can also be raised if we couldn't get a # connection from the pool before timing out, so check that @@ -1110,6 +1113,8 @@ def _execute_command(self, target_node, *args, **kwargs): time.sleep(0.25) self.nodes_manager.initialize() raise e + except ResponseError as e: + raise e except Exception as e: if connection: connection.disconnect() From 223fa0e461a36cd6d3337de35ea09907d4a1a8a3 Mon Sep 17 00:00:00 2001 From: nialdaly Date: Fri, 15 Jul 2022 20:14:10 +0100 Subject: [PATCH 12/13] re-trigger pipeline From 8680c911f7c6dcb3a35b93bc27237453847fdf83 Mon Sep 17 00:00:00 2001 From: nialdaly Date: Fri, 15 Jul 2022 22:52:23 +0100 Subject: [PATCH 13/13] except logic changed --- redis/cluster.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/redis/cluster.py b/redis/cluster.py index 264aae9601..74c14bd2a2 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -1060,9 +1060,8 @@ def _execute_command(self, target_node, *args, **kwargs): response, **kwargs ) return response - - except AuthenticationError as e: - raise e + except AuthenticationError: + raise except (ConnectionError, TimeoutError) as e: # ConnectionError can also be raised if we couldn't get a # connection from the pool before timing out, so check that @@ -1113,8 +1112,8 @@ def _execute_command(self, target_node, *args, **kwargs): time.sleep(0.25) self.nodes_manager.initialize() raise e - except ResponseError as e: - raise e + except ResponseError: + raise except Exception as e: if connection: connection.disconnect()