From 7902bf1e1d6331e7964ac498988925cc26e18f79 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Wed, 11 Sep 2019 15:14:56 +0100
Subject: [PATCH] Clean up some code in the retry logic (#6017)

* 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 0000000000..5ccab9c6ca
--- /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 b3c3bf55bc..d81ace0ece 100644
--- a/synapse/storage/transactions.py
+++ b/synapse/storage/transactions.py
@@ -250,26 +250,6 @@ class TransactionStore(SQLBaseStore):
                 },
             )
 
-    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 0862b5ca5a..5b16a81617 100644
--- a/synapse/util/retryutils.py
+++ b/synapse/util/retryutils.py
@@ -22,6 +22,15 @@ from synapse.api.errors import CodeMessageException
 
 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 @@ class RetryDestinationLimiter(object):
         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 @@ class RetryDestinationLimiter(object):
             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 @@ class RetryDestinationLimiter(object):
         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 @@ class RetryDestinationLimiter(object):
         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",