From 9e4c1fffe6466d6246e5d2f15e49759f73880d18 Mon Sep 17 00:00:00 2001 From: Andy McCurdy Date: Sun, 28 Jul 2019 13:36:05 -0700 Subject: Pipelines shouldn't retry ConnectionErrors implicitly --- CHANGES | 5 +++++ redis/client.py | 39 ++++++++++++++++++++++++--------------- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/CHANGES b/CHANGES index b8cdd3e..c60e9c9 100644 --- a/CHANGES +++ b/CHANGES @@ -31,6 +31,11 @@ before any command if the underlying connection has been idle for more than N seconds. ConnectionErrors and TimeoutErrors are automatically retried once for health checks. + * Changed the PubSubWorkerThread to use a threading.Event object rather + than a boolean to control the thread's life cycle. Thanks Timothy + Rule. #1194/#1195. + * Fixed a bug in Pipeline error handling that would incorrectly retry + ConnectionErrors. * 3.2.1 * Fix SentinelConnectionPool to work in multiprocess/forked environments. * 3.2.0 diff --git a/redis/client.py b/redis/client.py index fb1297a..c13ccff 100755 --- a/redis/client.py +++ b/redis/client.py @@ -3499,16 +3499,26 @@ class Pipeline(Redis): return self.parse_response(conn, command_name, **options) except (ConnectionError, TimeoutError) as e: conn.disconnect() - if not conn.retry_on_timeout and isinstance(e, TimeoutError): + # if we were already watching a variable, the watch is no longer + # valid since this connection has died. raise a WatchError, which + # indicates the user should retry this transaction. + if self.watching: + self.reset() + raise WatchError("A ConnectionError occured on while watching " + "one or more keys") + # if retry_on_timeout is not set, or the error is not + # a TimeoutError, raise it + if not (conn.retry_on_timeout and isinstance(e, TimeoutError)): + self.reset() raise - # if we're not already watching, we can safely retry the command + + # retry_on_timeout is set, this is a TimeoutError and we are not + # already WATCHing any variables. retry the command. try: - if not self.watching: - conn.send_command(*args) - return self.parse_response(conn, command_name, **options) - except ConnectionError: - # the retry failed so cleanup. - conn.disconnect() + conn.send_command(*args) + return self.parse_response(conn, command_name, **options) + except (ConnectionError, TimeoutError): + # a subsequent failure should simply be raised self.reset() raise @@ -3667,18 +3677,17 @@ class Pipeline(Redis): return execute(conn, stack, raise_on_error) except (ConnectionError, TimeoutError) as e: conn.disconnect() - if not conn.retry_on_timeout and isinstance(e, TimeoutError): - raise # if we were watching a variable, the watch is no longer valid # since this connection has died. raise a WatchError, which - # indicates the user should retry his transaction. If this is more - # than a temporary failure, the WATCH that the user next issues - # will fail, propegating the real ConnectionError + # indicates the user should retry this transaction. if self.watching: raise WatchError("A ConnectionError occured on while watching " "one or more keys") - # otherwise, it's safe to retry since the transaction isn't - # predicated on any state + # if retry_on_timeout is not set, or the error is not + # a TimeoutError, raise it + if not (conn.retry_on_timeout and isinstance(e, TimeoutError)): + raise + # retry a TimeoutError when retry_on_timeout is set return execute(conn, stack, raise_on_error) finally: self.reset() -- cgit v1.2.1