From b961e71ddde807d21796abcd42aebc33f41a4e07 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 11 Sep 2019 11:11:48 +0100 Subject: [PATCH] Clean up some code in the retry logic * remove some unused code * make things which were constants into constants for efficiency and clarity --- changelog.d/6017.misc | 1 + synapse/storage/transactions.py | 20 -------------------- synapse/util/retryutils.py | 29 +++++++++++++---------------- 3 files changed, 14 insertions(+), 36 deletions(-) create mode 100644 changelog.d/6017.misc diff --git a/changelog.d/6017.misc b/changelog.d/6017.misc new file mode 100644 index 000000000000..5ccab9c6ca91 --- /dev/null +++ b/changelog.d/6017.misc @@ -0,0 +1 @@ +Clean up some code in the retry logic. diff --git a/synapse/storage/transactions.py b/synapse/storage/transactions.py index b3c3bf55bc75..d81ace0ece0e 100644 --- a/synapse/storage/transactions.py +++ b/synapse/storage/transactions.py @@ -250,26 +250,6 @@ def _set_destination_retry_timings( }, ) - def get_destinations_needing_retry(self): - """Get all destinations which are due a retry for sending a transaction. - - Returns: - list: A list of dicts - """ - - return self.runInteraction( - "get_destinations_needing_retry", self._get_destinations_needing_retry - ) - - def _get_destinations_needing_retry(self, txn): - query = ( - "SELECT * FROM destinations" - " WHERE retry_last_ts > 0 and retry_next_ts < ?" - ) - - txn.execute(query, (self._clock.time_msec(),)) - return self.cursor_to_dict(txn) - def _start_cleanup_transactions(self): return run_as_background_process( "cleanup_transactions", self._cleanup_transactions diff --git a/synapse/util/retryutils.py b/synapse/util/retryutils.py index 0862b5ca5aaa..5b16a8161735 100644 --- a/synapse/util/retryutils.py +++ b/synapse/util/retryutils.py @@ -22,6 +22,15 @@ logger = logging.getLogger(__name__) +# the intial backoff, after the first transaction fails +MIN_RETRY_INTERVAL = 10 * 60 * 1000 + +# how much we multiply the backoff by after each subsequent fail +RETRY_MULTIPLIER = 5 + +# a cap on the backoff +MAX_RETRY_INTERVAL = 24 * 60 * 60 * 1000 + class NotRetryingDestination(Exception): def __init__(self, retry_last_ts, retry_interval, destination): @@ -112,9 +121,6 @@ def __init__( clock, store, retry_interval, - min_retry_interval=10 * 60 * 1000, - max_retry_interval=24 * 60 * 60 * 1000, - multiplier_retry_interval=5, backoff_on_404=False, backoff_on_failure=True, ): @@ -130,12 +136,6 @@ def __init__( retry_interval (int): The next retry interval taken from the database in milliseconds, or zero if the last request was successful. - min_retry_interval (int): The minimum retry interval to use after - a failed request, in milliseconds. - max_retry_interval (int): The maximum retry interval to use after - a failed request, in milliseconds. - multiplier_retry_interval (int): The multiplier to use to increase - the retry interval after a failed request. backoff_on_404 (bool): Back off if we get a 404 backoff_on_failure (bool): set to False if we should not increase the @@ -146,9 +146,6 @@ def __init__( self.destination = destination self.retry_interval = retry_interval - self.min_retry_interval = min_retry_interval - self.max_retry_interval = max_retry_interval - self.multiplier_retry_interval = multiplier_retry_interval self.backoff_on_404 = backoff_on_404 self.backoff_on_failure = backoff_on_failure @@ -196,13 +193,13 @@ def __exit__(self, exc_type, exc_val, exc_tb): else: # We couldn't connect. if self.retry_interval: - self.retry_interval *= self.multiplier_retry_interval + self.retry_interval *= RETRY_MULTIPLIER self.retry_interval *= int(random.uniform(0.8, 1.4)) - if self.retry_interval >= self.max_retry_interval: - self.retry_interval = self.max_retry_interval + if self.retry_interval >= MAX_RETRY_INTERVAL: + self.retry_interval = MAX_RETRY_INTERVAL else: - self.retry_interval = self.min_retry_interval + self.retry_interval = MIN_RETRY_INTERVAL logger.info( "Connection to %s was unsuccessful (%s(%s)); backoff now %i",