From 3a40e83f368b678d898066528e24fabcedc0b05e Mon Sep 17 00:00:00 2001
From: Shay <hillerys@element.io>
Date: Fri, 9 Jun 2023 00:00:46 -0700
Subject: [PATCH 01/10] Allow for the configuration of max request retries and
 min/max retry delays in the matrix federation client (#12504)

Co-authored-by: Mathieu Velten <mathieuv@matrix.org>
Co-authored-by: Erik Johnston <erik@matrix.org>
---
 changelog.d/12504.misc                        |  1 +
 .../configuration/config_documentation.md     | 26 +++++++++++++++++++
 synapse/config/federation.py                  | 10 +++++++
 synapse/http/matrixfederationclient.py        | 21 ++++++++-------
 tests/http/test_matrixfederationclient.py     | 20 +++++++++++++-
 5 files changed, 68 insertions(+), 10 deletions(-)
 create mode 100644 changelog.d/12504.misc

diff --git a/changelog.d/12504.misc b/changelog.d/12504.misc
new file mode 100644
index 000000000000..0bebaa213d9e
--- /dev/null
+++ b/changelog.d/12504.misc
@@ -0,0 +1 @@
+Allow for the configuration of max request retries and min/max retry delays in the matrix federation client.
diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md
index 0cf6e075ff11..8426de04179b 100644
--- a/docs/usage/configuration/config_documentation.md
+++ b/docs/usage/configuration/config_documentation.md
@@ -1196,6 +1196,32 @@ Example configuration:
 allow_device_name_lookup_over_federation: true
 ```
 ---
+### `federation`
+
+The federation section defines some sub-options related to federation.
+
+The following options are related to configuring timeout and retry logic for one request,
+independently of the others.
+Short retry algorithm is used when something or someone will wait for the request to have an
+answer, while long retry is used for requests that happen in the background,
+like sending a federation transaction.
+
+* `client_timeout`: timeout for the federation requests in seconds. Default to 60s.
+* `max_short_retry_delay`: maximum delay to be used for the short retry algo in seconds. Default to 2s.
+* `max_long_retry_delay`: maximum delay to be used for the short retry algo in seconds. Default to 60s.
+* `max_short_retries`: maximum number of retries for the short retry algo. Default to 3 attempts.
+* `max_long_retries`: maximum number of retries for the long retry algo. Default to 10 attempts.
+
+Example configuration:
+```yaml
+federation:
+  client_timeout: 180
+  max_short_retry_delay: 7
+  max_long_retry_delay: 100
+  max_short_retries: 5
+  max_long_retries: 20
+```
+---
 ## Caching
 
 Options related to caching.
diff --git a/synapse/config/federation.py b/synapse/config/federation.py
index 336fca578aa1..d21f7fd02a5e 100644
--- a/synapse/config/federation.py
+++ b/synapse/config/federation.py
@@ -22,6 +22,8 @@ class FederationConfig(Config):
     section = "federation"
 
     def read_config(self, config: JsonDict, **kwargs: Any) -> None:
+        federation_config = config.setdefault("federation", {})
+
         # FIXME: federation_domain_whitelist needs sytests
         self.federation_domain_whitelist: Optional[dict] = None
         federation_domain_whitelist = config.get("federation_domain_whitelist", None)
@@ -49,5 +51,13 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
             "allow_device_name_lookup_over_federation", False
         )
 
+        # Allow for the configuration of timeout, max request retries
+        # and min/max retry delays in the matrix federation client.
+        self.client_timeout = federation_config.get("client_timeout", 60)
+        self.max_long_retry_delay = federation_config.get("max_long_retry_delay", 60)
+        self.max_short_retry_delay = federation_config.get("max_short_retry_delay", 2)
+        self.max_long_retries = federation_config.get("max_long_retries", 10)
+        self.max_short_retries = federation_config.get("max_short_retries", 3)
+
 
 _METRICS_FOR_DOMAINS_SCHEMA = {"type": "array", "items": {"type": "string"}}
diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index abb5ae581521..ed36825b671c 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -95,8 +95,6 @@
 )
 
 
-MAX_LONG_RETRIES = 10
-MAX_SHORT_RETRIES = 3
 MAXINT = sys.maxsize
 
 
@@ -406,7 +404,12 @@ def __init__(
         self.clock = hs.get_clock()
         self._store = hs.get_datastores().main
         self.version_string_bytes = hs.version_string.encode("ascii")
-        self.default_timeout = 60
+        self.default_timeout = hs.config.federation.client_timeout
+
+        self.max_long_retry_delay = hs.config.federation.max_long_retry_delay
+        self.max_short_retry_delay = hs.config.federation.max_short_retry_delay
+        self.max_long_retries = hs.config.federation.max_long_retries
+        self.max_short_retries = hs.config.federation.max_short_retries
 
         self._cooperator = Cooperator(scheduler=_make_scheduler(self.reactor))
 
@@ -583,9 +586,9 @@ async def _send_request(
             # XXX: Would be much nicer to retry only at the transaction-layer
             # (once we have reliable transactions in place)
             if long_retries:
-                retries_left = MAX_LONG_RETRIES
+                retries_left = self.max_long_retries
             else:
-                retries_left = MAX_SHORT_RETRIES
+                retries_left = self.max_short_retries
 
             url_bytes = request.uri
             url_str = url_bytes.decode("ascii")
@@ -730,12 +733,12 @@ async def _send_request(
 
                     if retries_left and not timeout:
                         if long_retries:
-                            delay = 4 ** (MAX_LONG_RETRIES + 1 - retries_left)
-                            delay = min(delay, 60)
+                            delay = 4 ** (self.max_long_retries + 1 - retries_left)
+                            delay = min(delay, self.max_long_retry_delay)
                             delay *= random.uniform(0.8, 1.4)
                         else:
-                            delay = 0.5 * 2 ** (MAX_SHORT_RETRIES - retries_left)
-                            delay = min(delay, 2)
+                            delay = 0.5 * 2 ** (self.max_short_retries - retries_left)
+                            delay = min(delay, self.max_short_retry_delay)
                             delay *= random.uniform(0.8, 1.4)
 
                         logger.debug(
diff --git a/tests/http/test_matrixfederationclient.py b/tests/http/test_matrixfederationclient.py
index 0dfc03ce50f4..8565f8ac64ad 100644
--- a/tests/http/test_matrixfederationclient.py
+++ b/tests/http/test_matrixfederationclient.py
@@ -40,7 +40,7 @@
 from synapse.util import Clock
 
 from tests.server import FakeTransport
-from tests.unittest import HomeserverTestCase
+from tests.unittest import HomeserverTestCase, override_config
 
 
 def check_logcontext(context: LoggingContextOrSentinel) -> None:
@@ -640,3 +640,21 @@ def test_build_auth_headers_rejects_falsey_destinations(self) -> None:
             self.cl.build_auth_headers(
                 b"", b"GET", b"https://example.com", destination_is=b""
             )
+
+    @override_config(
+        {
+            "federation": {
+                "client_timeout": 180,
+                "max_long_retry_delay": 100,
+                "max_short_retry_delay": 7,
+                "max_long_retries": 20,
+                "max_short_retries": 5,
+            }
+        }
+    )
+    def test_configurable_retry_and_delay_values(self) -> None:
+        self.assertEqual(self.cl.default_timeout, 180)
+        self.assertEqual(self.cl.max_long_retry_delay, 100)
+        self.assertEqual(self.cl.max_short_retry_delay, 7)
+        self.assertEqual(self.cl.max_long_retries, 20)
+        self.assertEqual(self.cl.max_short_retries, 5)

From 4a28be9c7548dda42e64d6ba328420ff6035054e Mon Sep 17 00:00:00 2001
From: Mathieu Velten <matmaul@gmail.com>
Date: Tue, 13 Jun 2023 23:48:02 +0200
Subject: [PATCH 02/10] Use parse_duration for newly introduced options

---
 synapse/config/federation.py              | 12 ++++++--
 synapse/http/matrixfederationclient.py    | 35 +++++++++++------------
 tests/http/test_matrixfederationclient.py |  6 ++--
 3 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/synapse/config/federation.py b/synapse/config/federation.py
index d21f7fd02a5e..e42fa8063864 100644
--- a/synapse/config/federation.py
+++ b/synapse/config/federation.py
@@ -53,9 +53,15 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
 
         # Allow for the configuration of timeout, max request retries
         # and min/max retry delays in the matrix federation client.
-        self.client_timeout = federation_config.get("client_timeout", 60)
-        self.max_long_retry_delay = federation_config.get("max_long_retry_delay", 60)
-        self.max_short_retry_delay = federation_config.get("max_short_retry_delay", 2)
+        self.client_timeout = Config.parse_duration(
+            federation_config.get("client_timeout", "60s")
+        )
+        self.max_long_retry_delay = Config.parse_duration(
+            federation_config.get("max_long_retry_delay", "60s")
+        )
+        self.max_short_retry_delay = Config.parse_duration(
+            federation_config.get("max_short_retry_delay", "2s")
+        )
         self.max_long_retries = federation_config.get("max_long_retries", 10)
         self.max_short_retries = federation_config.get("max_short_retries", 3)
 
diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index ed36825b671c..1ac3cbca5c0c 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -404,10 +404,10 @@ def __init__(
         self.clock = hs.get_clock()
         self._store = hs.get_datastores().main
         self.version_string_bytes = hs.version_string.encode("ascii")
-        self.default_timeout = hs.config.federation.client_timeout
+        self.default_timeout = hs.config.federation.client_timeout / 1000
 
-        self.max_long_retry_delay = hs.config.federation.max_long_retry_delay
-        self.max_short_retry_delay = hs.config.federation.max_short_retry_delay
+        self.max_long_retry_delay = hs.config.federation.max_long_retry_delay / 1000
+        self.max_short_retry_delay = hs.config.federation.max_short_retry_delay / 1000
         self.max_long_retries = hs.config.federation.max_long_retries
         self.max_short_retries = hs.config.federation.max_short_retries
 
@@ -538,10 +538,10 @@ async def _send_request(
             logger.exception(f"Invalid destination: {request.destination}.")
             raise FederationDeniedError(request.destination)
 
-        if timeout:
-            _sec_timeout = timeout / 1000
-        else:
-            _sec_timeout = self.default_timeout
+        if timeout is None:
+            timeout = int(self.default_timeout)
+
+        _sec_timeout = timeout / 1000
 
         if (
             self.hs.config.federation.federation_domain_whitelist is not None
@@ -946,10 +946,9 @@ async def put_json(
             timeout=timeout,
         )
 
-        if timeout is not None:
-            _sec_timeout = timeout / 1000
-        else:
-            _sec_timeout = self.default_timeout
+        if timeout is None:
+            timeout = int(self.default_timeout)
+        _sec_timeout = timeout / 1000
 
         if parser is None:
             parser = cast(ByteParser[T], JsonParser())
@@ -1135,10 +1134,9 @@ async def get_json(
             timeout=timeout,
         )
 
-        if timeout is not None:
-            _sec_timeout = timeout / 1000
-        else:
-            _sec_timeout = self.default_timeout
+        if timeout is None:
+            timeout = int(self.default_timeout)
+        _sec_timeout = timeout / 1000
 
         if parser is None:
             parser = cast(ByteParser[T], JsonParser())
@@ -1211,10 +1209,9 @@ async def delete_json(
             ignore_backoff=ignore_backoff,
         )
 
-        if timeout is not None:
-            _sec_timeout = timeout / 1000
-        else:
-            _sec_timeout = self.default_timeout
+        if timeout is None:
+            timeout = int(self.default_timeout)
+        _sec_timeout = timeout / 1000
 
         body = await _handle_response(
             self.reactor, _sec_timeout, request, response, start_ms, parser=JsonParser()
diff --git a/tests/http/test_matrixfederationclient.py b/tests/http/test_matrixfederationclient.py
index 8565f8ac64ad..9eea7870b8b6 100644
--- a/tests/http/test_matrixfederationclient.py
+++ b/tests/http/test_matrixfederationclient.py
@@ -644,9 +644,9 @@ def test_build_auth_headers_rejects_falsey_destinations(self) -> None:
     @override_config(
         {
             "federation": {
-                "client_timeout": 180,
-                "max_long_retry_delay": 100,
-                "max_short_retry_delay": 7,
+                "client_timeout": "180s",
+                "max_long_retry_delay": "100s",
+                "max_short_retry_delay": "7s",
                 "max_long_retries": 20,
                 "max_short_retries": 5,
             }

From 0c9a5f5adf39e9b9376ab7fee44fca467953a67e Mon Sep 17 00:00:00 2001
From: Mathieu Velten <mathieuv@matrix.org>
Date: Thu, 15 Jun 2023 13:27:38 +0200
Subject: [PATCH 03/10] Add unit

---
 synapse/config/federation.py           | 6 +++---
 synapse/http/matrixfederationclient.py | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/synapse/config/federation.py b/synapse/config/federation.py
index e42fa8063864..0e1cb8b6e30c 100644
--- a/synapse/config/federation.py
+++ b/synapse/config/federation.py
@@ -53,13 +53,13 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
 
         # Allow for the configuration of timeout, max request retries
         # and min/max retry delays in the matrix federation client.
-        self.client_timeout = Config.parse_duration(
+        self.client_timeout_ms = Config.parse_duration(
             federation_config.get("client_timeout", "60s")
         )
-        self.max_long_retry_delay = Config.parse_duration(
+        self.max_long_retry_delay_ms = Config.parse_duration(
             federation_config.get("max_long_retry_delay", "60s")
         )
-        self.max_short_retry_delay = Config.parse_duration(
+        self.max_short_retry_delay_ms = Config.parse_duration(
             federation_config.get("max_short_retry_delay", "2s")
         )
         self.max_long_retries = federation_config.get("max_long_retries", 10)
diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 1ac3cbca5c0c..b2935c3d9fe6 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -404,10 +404,10 @@ def __init__(
         self.clock = hs.get_clock()
         self._store = hs.get_datastores().main
         self.version_string_bytes = hs.version_string.encode("ascii")
-        self.default_timeout = hs.config.federation.client_timeout / 1000
+        self.default_timeout = hs.config.federation.client_timeout_ms / 1000
 
-        self.max_long_retry_delay = hs.config.federation.max_long_retry_delay / 1000
-        self.max_short_retry_delay = hs.config.federation.max_short_retry_delay / 1000
+        self.max_long_retry_delay = hs.config.federation.max_long_retry_delay_ms / 1000
+        self.max_short_retry_delay = hs.config.federation.max_short_retry_delay_ms / 1000
         self.max_long_retries = hs.config.federation.max_long_retries
         self.max_short_retries = hs.config.federation.max_short_retries
 

From 497e09afa5432326273fbf21ad15697022ae74d7 Mon Sep 17 00:00:00 2001
From: Mathieu Velten <mathieuv@matrix.org>
Date: Thu, 15 Jun 2023 13:30:57 +0200
Subject: [PATCH 04/10] Rename changelog

---
 changelog.d/15783.misc | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/15783.misc

diff --git a/changelog.d/15783.misc b/changelog.d/15783.misc
new file mode 100644
index 000000000000..0bebaa213d9e
--- /dev/null
+++ b/changelog.d/15783.misc
@@ -0,0 +1 @@
+Allow for the configuration of max request retries and min/max retry delays in the matrix federation client.

From 752f48345f7fe9f560f5e73347f179f0872230e8 Mon Sep 17 00:00:00 2001
From: Mathieu Velten <mathieuv@matrix.org>
Date: Thu, 15 Jun 2023 13:38:42 +0200
Subject: [PATCH 05/10] lint

---
 changelog.d/12504.misc                 | 1 -
 synapse/http/matrixfederationclient.py | 4 +++-
 2 files changed, 3 insertions(+), 2 deletions(-)
 delete mode 100644 changelog.d/12504.misc

diff --git a/changelog.d/12504.misc b/changelog.d/12504.misc
deleted file mode 100644
index 0bebaa213d9e..000000000000
--- a/changelog.d/12504.misc
+++ /dev/null
@@ -1 +0,0 @@
-Allow for the configuration of max request retries and min/max retry delays in the matrix federation client.
diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index b2935c3d9fe6..5c5a003c9c31 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -407,7 +407,9 @@ def __init__(
         self.default_timeout = hs.config.federation.client_timeout_ms / 1000
 
         self.max_long_retry_delay = hs.config.federation.max_long_retry_delay_ms / 1000
-        self.max_short_retry_delay = hs.config.federation.max_short_retry_delay_ms / 1000
+        self.max_short_retry_delay = (
+            hs.config.federation.max_short_retry_delay_ms / 1000
+        )
         self.max_long_retries = hs.config.federation.max_long_retries
         self.max_short_retries = hs.config.federation.max_short_retries
 

From 4f694f55e9cabddc116ac826e59c3b8bdf8ecce8 Mon Sep 17 00:00:00 2001
From: Mathieu Velten <mathieuv@matrix.org>
Date: Fri, 16 Jun 2023 12:59:44 +0200
Subject: [PATCH 06/10] Fix doc

---
 docs/usage/configuration/config_documentation.md | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md
index 8426de04179b..26d7c7900cbe 100644
--- a/docs/usage/configuration/config_documentation.md
+++ b/docs/usage/configuration/config_documentation.md
@@ -1206,18 +1206,18 @@ Short retry algorithm is used when something or someone will wait for the reques
 answer, while long retry is used for requests that happen in the background,
 like sending a federation transaction.
 
-* `client_timeout`: timeout for the federation requests in seconds. Default to 60s.
-* `max_short_retry_delay`: maximum delay to be used for the short retry algo in seconds. Default to 2s.
-* `max_long_retry_delay`: maximum delay to be used for the short retry algo in seconds. Default to 60s.
+* `client_timeout`: timeout for the federation requests. Default to 60s.
+* `max_short_retry_delay`: maximum delay to be used for the short retry algo. Default to 2s.
+* `max_long_retry_delay`: maximum delay to be used for the short retry algo. Default to 60s.
 * `max_short_retries`: maximum number of retries for the short retry algo. Default to 3 attempts.
 * `max_long_retries`: maximum number of retries for the long retry algo. Default to 10 attempts.
 
 Example configuration:
 ```yaml
 federation:
-  client_timeout: 180
-  max_short_retry_delay: 7
-  max_long_retry_delay: 100
+  client_timeout: 180s
+  max_short_retry_delay: 7s
+  max_long_retry_delay: 100s
   max_short_retries: 5
   max_long_retries: 20
 ```

From bebdb26a6fbfb86f84d57daea1e0d472ff350ca1 Mon Sep 17 00:00:00 2001
From: Mathieu Velten <mathieuv@matrix.org>
Date: Fri, 16 Jun 2023 15:02:42 +0200
Subject: [PATCH 07/10] Wrong unit assumed

---
 synapse/http/matrixfederationclient.py | 29 ++++++++++++++------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 5c5a003c9c31..753569302aca 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -540,10 +540,10 @@ async def _send_request(
             logger.exception(f"Invalid destination: {request.destination}.")
             raise FederationDeniedError(request.destination)
 
-        if timeout is None:
-            timeout = int(self.default_timeout)
-
-        _sec_timeout = timeout / 1000
+        if timeout:
+            _sec_timeout = timeout / 1000
+        else:
+            _sec_timeout = self.default_timeout
 
         if (
             self.hs.config.federation.federation_domain_whitelist is not None
@@ -948,9 +948,10 @@ async def put_json(
             timeout=timeout,
         )
 
-        if timeout is None:
-            timeout = int(self.default_timeout)
-        _sec_timeout = timeout / 1000
+        if timeout:
+            _sec_timeout = timeout / 1000
+        else:
+            _sec_timeout = self.default_timeout
 
         if parser is None:
             parser = cast(ByteParser[T], JsonParser())
@@ -1136,9 +1137,10 @@ async def get_json(
             timeout=timeout,
         )
 
-        if timeout is None:
-            timeout = int(self.default_timeout)
-        _sec_timeout = timeout / 1000
+        if timeout:
+            _sec_timeout = timeout / 1000
+        else:
+            _sec_timeout = self.default_timeout
 
         if parser is None:
             parser = cast(ByteParser[T], JsonParser())
@@ -1211,9 +1213,10 @@ async def delete_json(
             ignore_backoff=ignore_backoff,
         )
 
-        if timeout is None:
-            timeout = int(self.default_timeout)
-        _sec_timeout = timeout / 1000
+        if timeout:
+            _sec_timeout = timeout / 1000
+        else:
+            _sec_timeout = self.default_timeout
 
         body = await _handle_response(
             self.reactor, _sec_timeout, request, response, start_ms, parser=JsonParser()

From b99441438388b45363caae1458580ec4de40478c Mon Sep 17 00:00:00 2001
From: Mathieu Velten <mathieuv@matrix.org>
Date: Fri, 16 Jun 2023 15:04:09 +0200
Subject: [PATCH 08/10] Back to previous code

---
 synapse/http/matrixfederationclient.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 753569302aca..c222f4d68a75 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -540,7 +540,7 @@ async def _send_request(
             logger.exception(f"Invalid destination: {request.destination}.")
             raise FederationDeniedError(request.destination)
 
-        if timeout:
+        if timeout is not None:
             _sec_timeout = timeout / 1000
         else:
             _sec_timeout = self.default_timeout
@@ -948,7 +948,7 @@ async def put_json(
             timeout=timeout,
         )
 
-        if timeout:
+        if timeout is not None:
             _sec_timeout = timeout / 1000
         else:
             _sec_timeout = self.default_timeout
@@ -1029,7 +1029,7 @@ async def post_json(
             ignore_backoff=ignore_backoff,
         )
 
-        if timeout:
+        if timeout is not None:
             _sec_timeout = timeout / 1000
         else:
             _sec_timeout = self.default_timeout
@@ -1137,7 +1137,7 @@ async def get_json(
             timeout=timeout,
         )
 
-        if timeout:
+        if timeout is not None:
             _sec_timeout = timeout / 1000
         else:
             _sec_timeout = self.default_timeout
@@ -1213,7 +1213,7 @@ async def delete_json(
             ignore_backoff=ignore_backoff,
         )
 
-        if timeout:
+        if timeout is not None:
             _sec_timeout = timeout / 1000
         else:
             _sec_timeout = self.default_timeout

From 89e2b30f9914de4fef7c35b687cc9e5c0c1800e0 Mon Sep 17 00:00:00 2001
From: Mathieu Velten <mathieuv@matrix.org>
Date: Tue, 20 Jun 2023 14:34:53 +0200
Subject: [PATCH 09/10] Add unit to var names

---
 synapse/http/matrixfederationclient.py    | 24 ++++++++++++-----------
 tests/http/test_matrixfederationclient.py |  6 +++---
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index c222f4d68a75..9860213b5f08 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -404,10 +404,12 @@ def __init__(
         self.clock = hs.get_clock()
         self._store = hs.get_datastores().main
         self.version_string_bytes = hs.version_string.encode("ascii")
-        self.default_timeout = hs.config.federation.client_timeout_ms / 1000
+        self.default_timeout_seconds = hs.config.federation.client_timeout_ms / 1000
 
-        self.max_long_retry_delay = hs.config.federation.max_long_retry_delay_ms / 1000
-        self.max_short_retry_delay = (
+        self.max_long_retry_delay_seconds = (
+            hs.config.federation.max_long_retry_delay_ms / 1000
+        )
+        self.max_short_retry_delay_seconds = (
             hs.config.federation.max_short_retry_delay_ms / 1000
         )
         self.max_long_retries = hs.config.federation.max_long_retries
@@ -543,7 +545,7 @@ async def _send_request(
         if timeout is not None:
             _sec_timeout = timeout / 1000
         else:
-            _sec_timeout = self.default_timeout
+            _sec_timeout = self.default_timeout_seconds
 
         if (
             self.hs.config.federation.federation_domain_whitelist is not None
@@ -736,11 +738,11 @@ async def _send_request(
                     if retries_left and not timeout:
                         if long_retries:
                             delay = 4 ** (self.max_long_retries + 1 - retries_left)
-                            delay = min(delay, self.max_long_retry_delay)
+                            delay = min(delay, self.max_long_retry_delay_seconds)
                             delay *= random.uniform(0.8, 1.4)
                         else:
                             delay = 0.5 * 2 ** (self.max_short_retries - retries_left)
-                            delay = min(delay, self.max_short_retry_delay)
+                            delay = min(delay, self.max_short_retry_delay_seconds)
                             delay *= random.uniform(0.8, 1.4)
 
                         logger.debug(
@@ -951,7 +953,7 @@ async def put_json(
         if timeout is not None:
             _sec_timeout = timeout / 1000
         else:
-            _sec_timeout = self.default_timeout
+            _sec_timeout = self.default_timeout_seconds
 
         if parser is None:
             parser = cast(ByteParser[T], JsonParser())
@@ -1032,7 +1034,7 @@ async def post_json(
         if timeout is not None:
             _sec_timeout = timeout / 1000
         else:
-            _sec_timeout = self.default_timeout
+            _sec_timeout = self.default_timeout_seconds
 
         body = await _handle_response(
             self.reactor, _sec_timeout, request, response, start_ms, parser=JsonParser()
@@ -1140,7 +1142,7 @@ async def get_json(
         if timeout is not None:
             _sec_timeout = timeout / 1000
         else:
-            _sec_timeout = self.default_timeout
+            _sec_timeout = self.default_timeout_seconds
 
         if parser is None:
             parser = cast(ByteParser[T], JsonParser())
@@ -1216,7 +1218,7 @@ async def delete_json(
         if timeout is not None:
             _sec_timeout = timeout / 1000
         else:
-            _sec_timeout = self.default_timeout
+            _sec_timeout = self.default_timeout_seconds
 
         body = await _handle_response(
             self.reactor, _sec_timeout, request, response, start_ms, parser=JsonParser()
@@ -1268,7 +1270,7 @@ async def get_file(
 
         try:
             d = read_body_with_max_size(response, output_stream, max_size)
-            d.addTimeout(self.default_timeout, self.reactor)
+            d.addTimeout(self.default_timeout_seconds, self.reactor)
             length = await make_deferred_yieldable(d)
         except BodyExceededMaxSize:
             msg = "Requested file is too large > %r bytes" % (max_size,)
diff --git a/tests/http/test_matrixfederationclient.py b/tests/http/test_matrixfederationclient.py
index 9eea7870b8b6..b5f4a60fe500 100644
--- a/tests/http/test_matrixfederationclient.py
+++ b/tests/http/test_matrixfederationclient.py
@@ -653,8 +653,8 @@ def test_build_auth_headers_rejects_falsey_destinations(self) -> None:
         }
     )
     def test_configurable_retry_and_delay_values(self) -> None:
-        self.assertEqual(self.cl.default_timeout, 180)
-        self.assertEqual(self.cl.max_long_retry_delay, 100)
-        self.assertEqual(self.cl.max_short_retry_delay, 7)
+        self.assertEqual(self.cl.default_timeout_seconds, 180)
+        self.assertEqual(self.cl.max_long_retry_delay_seconds, 100)
+        self.assertEqual(self.cl.max_short_retry_delay_seconds, 7)
         self.assertEqual(self.cl.max_long_retries, 20)
         self.assertEqual(self.cl.max_short_retries, 5)

From 8be814f4ec15a518198ac581e4937e2054cc8dff Mon Sep 17 00:00:00 2001
From: Mathieu Velten <mathieuv@matrix.org>
Date: Tue, 20 Jun 2023 14:36:51 +0200
Subject: [PATCH 10/10] More units

---
 synapse/http/matrixfederationclient.py | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 9860213b5f08..ffc85af5ff33 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -737,24 +737,34 @@ async def _send_request(
 
                     if retries_left and not timeout:
                         if long_retries:
-                            delay = 4 ** (self.max_long_retries + 1 - retries_left)
-                            delay = min(delay, self.max_long_retry_delay_seconds)
-                            delay *= random.uniform(0.8, 1.4)
+                            delay_seconds = 4 ** (
+                                self.max_long_retries + 1 - retries_left
+                            )
+                            delay_seconds = min(
+                                delay_seconds, self.max_long_retry_delay_seconds
+                            )
+                            delay_seconds *= random.uniform(0.8, 1.4)
                         else:
-                            delay = 0.5 * 2 ** (self.max_short_retries - retries_left)
-                            delay = min(delay, self.max_short_retry_delay_seconds)
-                            delay *= random.uniform(0.8, 1.4)
+                            delay_seconds = 0.5 * 2 ** (
+                                self.max_short_retries - retries_left
+                            )
+                            delay_seconds = min(
+                                delay_seconds, self.max_short_retry_delay_seconds
+                            )
+                            delay_seconds *= random.uniform(0.8, 1.4)
 
                         logger.debug(
                             "{%s} [%s] Waiting %ss before re-sending...",
                             request.txn_id,
                             request.destination,
-                            delay,
+                            delay_seconds,
                         )
 
                         # Sleep for the calculated delay, or wake up immediately
                         # if we get notified that the server is back up.
-                        await self._sleeper.sleep(request.destination, delay * 1000)
+                        await self._sleeper.sleep(
+                            request.destination, delay_seconds * 1000
+                        )
                         retries_left -= 1
                     else:
                         raise