From 411e4d8f549b0dbf0f471c300aaa4b533885ba4a Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Wed, 26 Jun 2019 15:21:21 +1000
Subject: [PATCH 01/19] be a bit more aggressive with not retrying likely-dead
 hosts

---
 synapse/federation/transport/client.py |  1 +
 synapse/http/matrixfederationclient.py | 38 ++++++++++++++++++++++++--
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index aecd1423097b..1608cd4f7358 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.py
@@ -181,6 +181,7 @@ def send_transaction(self, transaction, json_data_callback=None):
             long_retries=True,
             backoff_on_404=True,  # If we get a 404 the other side has gone
             try_trailing_slash_on_400=True,
+            retry_on_dns_fail=False, # If we get DNS errors, the other side has gone
         )
 
         defer.returnValue(response)
diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 5ef8bb60a3ab..1458e9d2079f 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -19,6 +19,8 @@
 import sys
 from io import BytesIO
 
+from OpenSSL import SSL
+
 from six import PY3, raise_from, string_types
 from six.moves import urllib
 
@@ -29,11 +31,12 @@
 from signedjson.sign import sign_json
 from zope.interface import implementer
 
+from twisted.names.error import DNSServerError
 from twisted.internet import defer, protocol
-from twisted.internet.error import DNSLookupError
+from twisted.internet.error import DNSLookupError, ConnectError, ConnectionRefusedError
 from twisted.internet.interfaces import IReactorPluggableNameResolver
 from twisted.internet.task import _EPSILON, Cooperator
-from twisted.web._newclient import ResponseDone
+from twisted.web._newclient import ResponseDone, RequestTransmissionFailed, ResponseNeverReceived
 from twisted.web.http_headers import Headers
 
 import synapse.metrics
@@ -407,6 +410,35 @@ def _send_request(
                             response = yield request_deferred
                     except DNSLookupError as e:
                         raise_from(RequestSendFailed(e, can_retry=retry_on_dns_fail), e)
+                    except DNSServerError as e:
+                        # Their domain's nameserver is busted and can't give us a result
+                        raise_from(RequestSendFailed(e, can_retry=retry_on_dns_fail), e)
+                    except (ConnectError, ConnectionRefusedError) as e:
+                        if e.osError == 113:
+                            # No route to host -- they're gone
+                            raise_from(RequestSendFailed(e, can_retry=False), e)
+                        elif e.osError == 111:
+                            # Refused connection -- they're gone
+                            raise_from(RequestSendFailed(e, can_retry=False), e)
+                        elif e.osError == 99:
+                            # Cannot assign address -- don't try?
+                            raise_from(RequestSendFailed(e, can_retry=False), e)
+
+                        # Some other socket error, try retrying
+                        logger.info("Failed to send request due to socket error: %s", e)
+                        raise_from(RequestSendFailed(e, can_retry=True), e)
+
+                    except (RequestTransmissionFailed, ResponseNeverReceived) as e:
+                        for i in e.reasons:
+                            # If it's an OpenSSL error, they probably don't have
+                            # a valid certificate or something else very bad went on.
+                            if i.trap(SSL.Error):
+                                raise_from(RequestSendFailed(e, can_retry=False), e)
+
+                        # If it's not that, raise it normally.
+                        logger.info("Failed to send request: %s", e)
+                        raise_from(RequestSendFailed(e, can_retry=True), e)
+
                     except Exception as e:
                         logger.info("Failed to send request: %s", e)
                         raise_from(RequestSendFailed(e, can_retry=True), e)
@@ -557,6 +589,7 @@ def put_json(
         ignore_backoff=False,
         backoff_on_404=False,
         try_trailing_slash_on_400=False,
+        retry_on_dns_fail=True,
     ):
         """ Sends the specifed json data using PUT
 
@@ -618,6 +651,7 @@ def put_json(
             ignore_backoff=ignore_backoff,
             long_retries=long_retries,
             timeout=timeout,
+            retry_on_dns_fail=retry_on_dns_fail,
         )
 
         body = yield _handle_json_response(

From bdad7ffaff1d876fa6a98369d30a7a4ad4c88781 Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Wed, 26 Jun 2019 15:25:20 +1000
Subject: [PATCH 02/19] fixes

---
 synapse/http/matrixfederationclient.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 1458e9d2079f..892d971d8ff6 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -425,7 +425,7 @@ def _send_request(
                             raise_from(RequestSendFailed(e, can_retry=False), e)
 
                         # Some other socket error, try retrying
-                        logger.info("Failed to send request due to socket error: %s", e)
+                        logger.info("Failed to send request: %s", e)
                         raise_from(RequestSendFailed(e, can_retry=True), e)
 
                     except (RequestTransmissionFailed, ResponseNeverReceived) as e:

From ad0d7d4c9ffe351e03e28dd9ad3c765bd7802c50 Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Wed, 26 Jun 2019 15:25:27 +1000
Subject: [PATCH 03/19] changelog

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

diff --git a/changelog.d/5556.misc b/changelog.d/5556.misc
new file mode 100644
index 000000000000..185603a37ff9
--- /dev/null
+++ b/changelog.d/5556.misc
@@ -0,0 +1 @@
+Treat more outgoing federation connection failures (like refused connection, dead domains, and no route to host) as fatal and not able to be retried immediately.

From 69490ad447d9ab91c57fd419909011ecc9820e44 Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Wed, 26 Jun 2019 15:27:28 +1000
Subject: [PATCH 04/19] lint

---
 synapse/federation/transport/client.py |  2 +-
 synapse/http/matrixfederationclient.py | 13 ++++++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index 1608cd4f7358..66cd8e0a844f 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.py
@@ -181,7 +181,7 @@ def send_transaction(self, transaction, json_data_callback=None):
             long_retries=True,
             backoff_on_404=True,  # If we get a 404 the other side has gone
             try_trailing_slash_on_400=True,
-            retry_on_dns_fail=False, # If we get DNS errors, the other side has gone
+            retry_on_dns_fail=False,  # If we get DNS errors, the other side has gone
         )
 
         defer.returnValue(response)
diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 892d971d8ff6..1708997a7fa2 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -19,8 +19,6 @@
 import sys
 from io import BytesIO
 
-from OpenSSL import SSL
-
 from six import PY3, raise_from, string_types
 from six.moves import urllib
 
@@ -31,12 +29,17 @@
 from signedjson.sign import sign_json
 from zope.interface import implementer
 
-from twisted.names.error import DNSServerError
+from OpenSSL import SSL
 from twisted.internet import defer, protocol
-from twisted.internet.error import DNSLookupError, ConnectError, ConnectionRefusedError
+from twisted.internet.error import ConnectError, ConnectionRefusedError, DNSLookupError
 from twisted.internet.interfaces import IReactorPluggableNameResolver
 from twisted.internet.task import _EPSILON, Cooperator
-from twisted.web._newclient import ResponseDone, RequestTransmissionFailed, ResponseNeverReceived
+from twisted.names.error import DNSServerError
+from twisted.web._newclient import (
+    RequestTransmissionFailed,
+    ResponseDone,
+    ResponseNeverReceived,
+)
 from twisted.web.http_headers import Headers
 
 import synapse.metrics

From 535e8ea79f10030bbe739ce177b1a9f80a1dc2a7 Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Thu, 27 Jun 2019 19:09:59 +1000
Subject: [PATCH 05/19] fix some logs

---
 tests/handlers/test_typing.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py
index 5d5e324df262..c8b17cf6ff69 100644
--- a/tests/handlers/test_typing.py
+++ b/tests/handlers/test_typing.py
@@ -190,6 +190,7 @@ def test_started_typing_remote_send(self):
             json_data_callback=ANY,
             long_retries=True,
             backoff_on_404=True,
+            retry_on_dns_fail=False,
             try_trailing_slash_on_400=True,
         )
 
@@ -263,6 +264,7 @@ def test_stopped_typing(self):
             ),
             json_data_callback=ANY,
             long_retries=True,
+            retry_on_dns_fail=False,
             backoff_on_404=True,
             try_trailing_slash_on_400=True,
         )

From 84cbc5019c59eb89915346a6dce98c8a179e58b1 Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Mon, 1 Jul 2019 20:57:27 +1000
Subject: [PATCH 06/19] make the aggressiveness more configurable

---
 synapse/config/server.py               | 18 +++++++
 synapse/federation/transport/client.py |  3 +-
 synapse/http/matrixfederationclient.py | 66 +++++++++++++++++++-------
 3 files changed, 69 insertions(+), 18 deletions(-)

diff --git a/synapse/config/server.py b/synapse/config/server.py
index 2a74dea2ea6b..d17e1aadc618 100644
--- a/synapse/config/server.py
+++ b/synapse/config/server.py
@@ -18,6 +18,7 @@
 import logging
 import os.path
 
+import attr
 from netaddr import IPSet
 
 from synapse.api.room_versions import KNOWN_ROOM_VERSIONS
@@ -324,6 +325,23 @@ def read_config(self, config, **kwargs):
             "cleanup_extremities_with_dummy_events", False
         )
 
+        @attr.s
+        class FederationBackoffSettings(object):
+            dns_resolution = attr.ib(default=False, type=bool)
+            dns_servfail = attr.ib(default=False, type=bool)
+            no_route_to_host = attr.ib(default=False, type=bool)
+            refused_connection = attr.ib(default=False, type=bool)
+            cannot_assign_address = attr.ib(default=False, type=bool)
+            timeout_amount = attr.ib(default=60, type=int)
+            on_timeout = attr.ib(default=False, type=bool)
+            invalid_tls = attr.ib(default=True, type=bool)
+
+        federation_backoff_settings = config.get("federation_backoff", {})
+
+        self.federation_backoff_settings = FederationBackoffSettings(
+            **federation_backoff_settings
+        )
+
     def has_tls_listener(self):
         return any(l["tls"] for l in self.listeners)
 
diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index 66cd8e0a844f..86af251d43bb 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.py
@@ -33,6 +33,7 @@ class TransportLayerClient(object):
     def __init__(self, hs):
         self.server_name = hs.hostname
         self.client = hs.get_http_client()
+        self.backoff_settings = hs.config.federation_backoff_settings
 
     @log_function
     def get_room_state(self, destination, room_id, event_id):
@@ -181,7 +182,7 @@ def send_transaction(self, transaction, json_data_callback=None):
             long_retries=True,
             backoff_on_404=True,  # If we get a 404 the other side has gone
             try_trailing_slash_on_400=True,
-            retry_on_dns_fail=False,  # If we get DNS errors, the other side has gone
+            retry_on_dns_fail=self.backoff_settings.dns_resolution,
         )
 
         defer.returnValue(response)
diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 1708997a7fa2..8613ac7fe16d 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -31,7 +31,12 @@
 
 from OpenSSL import SSL
 from twisted.internet import defer, protocol
-from twisted.internet.error import ConnectError, ConnectionRefusedError, DNSLookupError
+from twisted.internet.error import (
+    ConnectError,
+    ConnectionRefusedError,
+    DNSLookupError,
+    TimeoutError,
+)
 from twisted.internet.interfaces import IReactorPluggableNameResolver
 from twisted.internet.task import _EPSILON, Cooperator
 from twisted.names.error import DNSServerError
@@ -179,6 +184,7 @@ def __init__(self, hs, tls_client_options_factory):
         self.hs = hs
         self.signing_key = hs.config.signing_key[0]
         self.server_name = hs.hostname
+        self.backoff_settings = hs.config.federation_backoff_settings
 
         real_reactor = hs.get_reactor()
 
@@ -211,7 +217,7 @@ def __getattr__(_self, attr):
         self.clock = hs.get_clock()
         self._store = hs.get_datastore()
         self.version_string_bytes = hs.version_string.encode("ascii")
-        self.default_timeout = 60
+        self.default_timeout = self.backoff_settings.timeout_amount
 
         def schedule(x):
             self.reactor.callLater(_EPSILON, x)
@@ -415,15 +421,52 @@ def _send_request(
                         raise_from(RequestSendFailed(e, can_retry=retry_on_dns_fail), e)
                     except DNSServerError as e:
                         # Their domain's nameserver is busted and can't give us a result
-                        raise_from(RequestSendFailed(e, can_retry=retry_on_dns_fail), e)
+                        raise_from(
+                            RequestSendFailed(
+                                e, can_retry=not self.backoff_settings.dns_servfail
+                            ),
+                            e,
+                        )
+                    except (RequestTransmissionFailed, ResponseNeverReceived) as e:
+                        for i in e.reasons:
+                            # If it's an OpenSSL error, they probably don't have
+                            # a valid certificate or something else very bad went on.
+                            if i.check(SSL.Error):
+                                if self.backoff_settings.invalid_tls:
+                                    raise_from(RequestSendFailed(e, can_retry=False), e)
+
+                            elif i.check(TimeoutError, defer.CancelledError):
+                                # If we backoff hard on timeout, raise it here.
+                                if self.backoff_settings.on_timeout:
+                                    raise_from(RequestSendFailed(e, can_retry=False), e)
+
+                        # If it's not that, raise it normally.
+                        logger.info("Failed to send request: %s", e)
+                        raise_from(RequestSendFailed(e, can_retry=True), e)
+
+                    except TimeoutError as e:
+                        # Handle timeouts
+                        if self.backoff_settings.on_timeout:
+                            raise_from(RequestSendFailed(e, can_retry=False), e)
+
+                        # If it's not that, raise it normally.
+                        logger.info("Failed to send request: %s", e)
+                        raise_from(RequestSendFailed(e, can_retry=True), e)
+
                     except (ConnectError, ConnectionRefusedError) as e:
-                        if e.osError == 113:
+                        if e.osError == 113 and self.backoff_settings.no_route_to_host:
                             # No route to host -- they're gone
                             raise_from(RequestSendFailed(e, can_retry=False), e)
-                        elif e.osError == 111:
+                        elif (
+                            e.osError == 111
+                            and self.backoff_settings.refused_connection
+                        ):
                             # Refused connection -- they're gone
                             raise_from(RequestSendFailed(e, can_retry=False), e)
-                        elif e.osError == 99:
+                        elif (
+                            e.osError == 99
+                            and self.backoff_settings.cannot_assign_address
+                        ):
                             # Cannot assign address -- don't try?
                             raise_from(RequestSendFailed(e, can_retry=False), e)
 
@@ -431,17 +474,6 @@ def _send_request(
                         logger.info("Failed to send request: %s", e)
                         raise_from(RequestSendFailed(e, can_retry=True), e)
 
-                    except (RequestTransmissionFailed, ResponseNeverReceived) as e:
-                        for i in e.reasons:
-                            # If it's an OpenSSL error, they probably don't have
-                            # a valid certificate or something else very bad went on.
-                            if i.trap(SSL.Error):
-                                raise_from(RequestSendFailed(e, can_retry=False), e)
-
-                        # If it's not that, raise it normally.
-                        logger.info("Failed to send request: %s", e)
-                        raise_from(RequestSendFailed(e, can_retry=True), e)
-
                     except Exception as e:
                         logger.info("Failed to send request: %s", e)
                         raise_from(RequestSendFailed(e, can_retry=True), e)

From d08ce1b4bfa89563e434c9249f1ad9227e52650b Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Mon, 1 Jul 2019 21:35:25 +1000
Subject: [PATCH 07/19] make logging a little better

---
 synapse/http/matrixfederationclient.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 8613ac7fe16d..2ba7738e219a 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -523,9 +523,10 @@ def _send_request(
                     break
                 except RequestSendFailed as e:
                     logger.warn(
-                        "{%s} [%s] Request failed: %s %s: %s",
+                        "{%s} [%s] Request failed, %s - %s: %s %s: %s",
                         request.txn_id,
                         request.destination,
+                        "retrying" if e.can_retry else "not retrying",
                         request.method,
                         url_str,
                         _flatten_response_never_received(e.inner_exception),

From a82457c5a3e430cea16802d660368b7a44501230 Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Mon, 1 Jul 2019 21:36:38 +1000
Subject: [PATCH 08/19] make logging a little better

---
 synapse/http/matrixfederationclient.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 2ba7738e219a..1c50673163d0 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -475,7 +475,7 @@ def _send_request(
                         raise_from(RequestSendFailed(e, can_retry=True), e)
 
                     except Exception as e:
-                        logger.info("Failed to send request: %s", e)
+                        logger.info("Failed to send request for unhandled reason: %s", e)
                         raise_from(RequestSendFailed(e, can_retry=True), e)
 
                     logger.info(

From 30ed28417eee255e4f7b18166a8407cbced9cfca Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Mon, 1 Jul 2019 21:47:17 +1000
Subject: [PATCH 09/19] make logging a little better

---
 synapse/http/matrixfederationclient.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 1c50673163d0..fcb5254f9310 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -523,7 +523,7 @@ def _send_request(
                     break
                 except RequestSendFailed as e:
                     logger.warn(
-                        "{%s} [%s] Request failed, %s - %s: %s %s: %s",
+                        "{%s} [%s] Request failed, %s: %s %s: %s",
                         request.txn_id,
                         request.destination,
                         "retrying" if e.can_retry else "not retrying",

From 45c0117abf4e9c3fa13943498d6e7426d63d5ba3 Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Mon, 1 Jul 2019 22:07:19 +1000
Subject: [PATCH 10/19] cancel in connect

---
 synapse/http/matrixfederationclient.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index fcb5254f9310..53f45824ebba 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -36,6 +36,7 @@
     ConnectionRefusedError,
     DNSLookupError,
     TimeoutError,
+    ConnectingCancelledError,
 )
 from twisted.internet.interfaces import IReactorPluggableNameResolver
 from twisted.internet.task import _EPSILON, Cooperator
@@ -444,7 +445,7 @@ def _send_request(
                         logger.info("Failed to send request: %s", e)
                         raise_from(RequestSendFailed(e, can_retry=True), e)
 
-                    except TimeoutError as e:
+                    except (TimeoutError, ConnectingCancelledError) as e:
                         # Handle timeouts
                         if self.backoff_settings.on_timeout:
                             raise_from(RequestSendFailed(e, can_retry=False), e)

From f1467e59726948ecc76cd6be863ed8d0dad50027 Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Mon, 1 Jul 2019 22:11:15 +1000
Subject: [PATCH 11/19] handle more things

---
 synapse/federation/transport/client.py | 2 +-
 synapse/http/matrixfederationclient.py | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index 86af251d43bb..29273069baa7 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.py
@@ -182,7 +182,7 @@ def send_transaction(self, transaction, json_data_callback=None):
             long_retries=True,
             backoff_on_404=True,  # If we get a 404 the other side has gone
             try_trailing_slash_on_400=True,
-            retry_on_dns_fail=self.backoff_settings.dns_resolution,
+            retry_on_dns_fail=not self.backoff_settings.dns_resolution,
         )
 
         defer.returnValue(response)
diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 53f45824ebba..1ea955fb42ba 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -20,6 +20,7 @@
 from io import BytesIO
 
 from six import PY3, raise_from, string_types
+from service_identity.exceptions import VerificationError
 from six.moves import urllib
 
 import attr
@@ -432,7 +433,7 @@ def _send_request(
                         for i in e.reasons:
                             # If it's an OpenSSL error, they probably don't have
                             # a valid certificate or something else very bad went on.
-                            if i.check(SSL.Error):
+                            if i.check(SSL.Error) or i.check(VerificationError):
                                 if self.backoff_settings.invalid_tls:
                                     raise_from(RequestSendFailed(e, can_retry=False), e)
 

From 61f75d53ccaa9bf276f0bb1984af9267db358b07 Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Mon, 1 Jul 2019 23:06:20 +1000
Subject: [PATCH 12/19] fix

---
 tests/handlers/test_typing.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py
index c8b17cf6ff69..61ef04203324 100644
--- a/tests/handlers/test_typing.py
+++ b/tests/handlers/test_typing.py
@@ -190,7 +190,7 @@ def test_started_typing_remote_send(self):
             json_data_callback=ANY,
             long_retries=True,
             backoff_on_404=True,
-            retry_on_dns_fail=False,
+            retry_on_dns_fail=True,
             try_trailing_slash_on_400=True,
         )
 
@@ -264,7 +264,7 @@ def test_stopped_typing(self):
             ),
             json_data_callback=ANY,
             long_retries=True,
-            retry_on_dns_fail=False,
+            retry_on_dns_fail=True,
             backoff_on_404=True,
             try_trailing_slash_on_400=True,
         )

From 02f771a203ab30a7262361ad485eb423e631aea1 Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Mon, 8 Jul 2019 18:06:56 +1000
Subject: [PATCH 13/19] fix lint

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

diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 5302f903c7d5..df08abb2bce8 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -20,13 +20,13 @@
 from io import BytesIO
 
 from six import PY3, raise_from, string_types
-from service_identity.exceptions import VerificationError
 from six.moves import urllib
 
 import attr
 import treq
 from canonicaljson import encode_canonical_json
 from prometheus_client import Counter
+from service_identity.exceptions import VerificationError
 from signedjson.sign import sign_json
 from zope.interface import implementer
 
@@ -34,10 +34,10 @@
 from twisted.internet import defer, protocol
 from twisted.internet.error import (
     ConnectError,
+    ConnectingCancelledError,
     ConnectionRefusedError,
     DNSLookupError,
     TimeoutError,
-    ConnectingCancelledError,
 )
 from twisted.internet.interfaces import IReactorPluggableNameResolver
 from twisted.internet.task import _EPSILON, Cooperator
@@ -477,7 +477,9 @@ def _send_request(
                         raise_from(RequestSendFailed(e, can_retry=True), e)
 
                     except Exception as e:
-                        logger.info("Failed to send request for unhandled reason: %s", e)
+                        logger.info(
+                            "Failed to send request for unhandled reason: %s", e
+                        )
                         raise_from(RequestSendFailed(e, can_retry=True), e)
 
                     logger.info(

From 7f1eada1925fc3e091cf02bf07401ef8cf19a434 Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Mon, 8 Jul 2019 20:21:08 +1000
Subject: [PATCH 14/19] backoff settings

---
 changelog.d/5556.feature |  1 +
 changelog.d/5556.misc    |  1 -
 docs/sample_config.yaml  | 65 +++++++++++++++++++++++++++++++++++++
 synapse/config/server.py | 69 ++++++++++++++++++++++++++++++++++++++--
 4 files changed, 133 insertions(+), 3 deletions(-)
 create mode 100644 changelog.d/5556.feature
 delete mode 100644 changelog.d/5556.misc

diff --git a/changelog.d/5556.feature b/changelog.d/5556.feature
new file mode 100644
index 000000000000..1dc29214ce6c
--- /dev/null
+++ b/changelog.d/5556.feature
@@ -0,0 +1 @@
+Synapse's federation backoff behaviour can now be tuned using the new `federation_backoff` settings.
\ No newline at end of file
diff --git a/changelog.d/5556.misc b/changelog.d/5556.misc
deleted file mode 100644
index 185603a37ff9..000000000000
--- a/changelog.d/5556.misc
+++ /dev/null
@@ -1 +0,0 @@
-Treat more outgoing federation connection failures (like refused connection, dead domains, and no route to host) as fatal and not able to be retried immediately.
diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml
index 7fe7c94ac4cb..cf41db42b7aa 100644
--- a/docs/sample_config.yaml
+++ b/docs/sample_config.yaml
@@ -124,6 +124,71 @@ federation_ip_range_blacklist:
   - 'fe80::/64'
   - 'fc00::/7'
 
+# Federation Backoff Tuning
+#
+# These options control what Synapse will consider an unrecoverable
+# network error as a federation client. Unrecoverable network errors
+# trigger immediate backoff. Setting these options too aggressively may
+# cause your Synapse to consider other servers down due to temporary
+# networking hiccups, causing outward federation delays for the duration
+# of the backoff period.
+#
+# These options may be useful on more constrained instances that are in
+# rooms with lots of servers.
+#
+# The options are:
+#
+#   dns_resolution: DNS lookup failure (NXDOMAIN and others) will cause
+#       immediate backoff. Default: False.
+#
+#   dns_servfail: DNS lookup failures where the server cannot process
+#       the request (SERVFAIL) will cause immediate backoff. This can be
+#       triggered either by temporary outages on your DNS server, or
+#       failure of the DNS server responsible for the target domain.
+#       Only enable this if you have reliable DNS servers.
+#       Default: False.
+#
+#   no_route_to_host: EHOSTUNREACH errors will cause immediate backoff.
+#       This can be caused by DNS resolving to unroutable IP addresses.
+#       Default: False.
+#
+#   refused_connection: ECONNREFUSED errors will cause immediate backoff.
+#       This can result in temporary outages (e.g. restarts of other
+#       servers) triggering backoff and subsequent federation delays.
+#       Default: False.
+#
+#   cannot_assign_address: Being unable to assign an address when binding
+#       a socket will cause immediate backoff. Can be caused by IPv4/v6
+#       misconfiguration.
+#       Default: False.
+#
+#   invalid_tls: Invalid TLS negotiation or a TLS certificate failure
+#       will cause immediate backoff. This is generally safe to enable,
+#       but it will cause Synapse to not check for a new valid TLS
+#       setup until the end of the backoff, which may cause a delay in
+#       re-establishing federation with servers that fix their TLS.
+#       Default: False.
+#
+#   on_timeout: A timeout will cause immediate backoff. Timeouts can be
+#       caused by various reasons (overloading of the target server,
+#       network degradation) and may cause Synapse to consider a
+#       temporarily overloaded server as down, and cause subsequent
+#       federation delays. Default: False.
+#
+#   timeout_amount: How long Synapse will wait for before timing out
+#       federation client requests. This accepts a human-readable time
+#       (e.g. "60s") or a flat number of milliseconds. Default: 60s.
+#
+#federation_backoff:
+#  dns_resolution: False
+#  dns_servfail: False
+#  no_route_to_host: False
+#  refused_connection: False
+#  cannot_assign_address: False
+#  invalid_tls: False
+#  on_timeout: False
+#  timeout_amount: "60s"
+
 # List of ports that Synapse should listen on, their purpose and their
 # configuration.
 #
diff --git a/synapse/config/server.py b/synapse/config/server.py
index d17e1aadc618..31cbff2ac473 100644
--- a/synapse/config/server.py
+++ b/synapse/config/server.py
@@ -332,9 +332,9 @@ class FederationBackoffSettings(object):
             no_route_to_host = attr.ib(default=False, type=bool)
             refused_connection = attr.ib(default=False, type=bool)
             cannot_assign_address = attr.ib(default=False, type=bool)
-            timeout_amount = attr.ib(default=60, type=int)
-            on_timeout = attr.ib(default=False, type=bool)
             invalid_tls = attr.ib(default=True, type=bool)
+            timeout_amount = attr.ib(default="60s", converter=self.parse_duration, type=int)
+            on_timeout = attr.ib(default=False, type=bool)
 
         federation_backoff_settings = config.get("federation_backoff", {})
 
@@ -483,6 +483,71 @@ def generate_config_section(
           - 'fe80::/64'
           - 'fc00::/7'
 
+        # Federation Backoff Tuning
+        #
+        # These options control what Synapse will consider an unrecoverable
+        # network error as a federation client. Unrecoverable network errors
+        # trigger immediate backoff. Setting these options too aggressively may
+        # cause your Synapse to consider other servers down due to temporary
+        # networking hiccups, causing outward federation delays for the duration
+        # of the backoff period.
+        #
+        # These options may be useful on more constrained instances that are in
+        # rooms with lots of servers.
+        #
+        # The options are:
+        #
+        #   dns_resolution: DNS lookup failure (NXDOMAIN and others) will cause
+        #       immediate backoff. Default: False.
+        #
+        #   dns_servfail: DNS lookup failures where the server cannot process
+        #       the request (SERVFAIL) will cause immediate backoff. This can be
+        #       triggered either by temporary outages on your DNS server, or
+        #       failure of the DNS server responsible for the target domain.
+        #       Only enable this if you have reliable DNS servers.
+        #       Default: False.
+        #
+        #   no_route_to_host: EHOSTUNREACH errors will cause immediate backoff.
+        #       This can be caused by DNS resolving to unroutable IP addresses.
+        #       Default: False.
+        #
+        #   refused_connection: ECONNREFUSED errors will cause immediate backoff.
+        #       This can result in temporary outages (e.g. restarts of other
+        #       servers) triggering backoff and subsequent federation delays.
+        #       Default: False.
+        #
+        #   cannot_assign_address: Being unable to assign an address when binding
+        #       a socket will cause immediate backoff. Can be caused by IPv4/v6
+        #       misconfiguration.
+        #       Default: False.
+        #
+        #   invalid_tls: Invalid TLS negotiation or a TLS certificate failure
+        #       will cause immediate backoff. This is generally safe to enable,
+        #       but it will cause Synapse to not check for a new valid TLS
+        #       setup until the end of the backoff, which may cause a delay in
+        #       re-establishing federation with servers that fix their TLS.
+        #       Default: False.
+        #
+        #   on_timeout: A timeout will cause immediate backoff. Timeouts can be
+        #       caused by various reasons (overloading of the target server,
+        #       network degradation) and may cause Synapse to consider a
+        #       temporarily overloaded server as down, and cause subsequent
+        #       federation delays. Default: False.
+        #
+        #   timeout_amount: How long Synapse will wait for before timing out
+        #       federation client requests. This accepts a human-readable time
+        #       (e.g. "60s") or a flat number of milliseconds. Default: 60s.
+        #
+        #federation_backoff:
+        #  dns_resolution: False
+        #  dns_servfail: False
+        #  no_route_to_host: False
+        #  refused_connection: False
+        #  cannot_assign_address: False
+        #  invalid_tls: False
+        #  on_timeout: False
+        #  timeout_amount: "60s"
+
         # List of ports that Synapse should listen on, their purpose and their
         # configuration.
         #

From b5574c5d63abf4a80873b80af3b5b4ab8e88a5b9 Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Mon, 8 Jul 2019 20:32:41 +1000
Subject: [PATCH 15/19] add some tests

---
 synapse/config/server.py    |  6 ++--
 tests/config/test_server.py | 72 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/synapse/config/server.py b/synapse/config/server.py
index 31cbff2ac473..6cbcc252f525 100644
--- a/synapse/config/server.py
+++ b/synapse/config/server.py
@@ -332,8 +332,10 @@ class FederationBackoffSettings(object):
             no_route_to_host = attr.ib(default=False, type=bool)
             refused_connection = attr.ib(default=False, type=bool)
             cannot_assign_address = attr.ib(default=False, type=bool)
-            invalid_tls = attr.ib(default=True, type=bool)
-            timeout_amount = attr.ib(default="60s", converter=self.parse_duration, type=int)
+            invalid_tls = attr.ib(default=False, type=bool)
+            timeout_amount = attr.ib(
+                default="60s", converter=self.parse_duration, type=int
+            )
             on_timeout = attr.ib(default=False, type=bool)
 
         federation_backoff_settings = config.get("federation_backoff", {})
diff --git a/tests/config/test_server.py b/tests/config/test_server.py
index 1ca5ea54ca6e..3cfead7970e8 100644
--- a/tests/config/test_server.py
+++ b/tests/config/test_server.py
@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 # Copyright 2019 New Vector Ltd
+# Copyright 2019 Matrix.org Foundation C.I.C.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -13,7 +14,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from synapse.config.server import is_threepid_reserved
+from synapse.config.server import ConfigError, ServerConfig, is_threepid_reserved
 
 from tests import unittest
 
@@ -29,3 +30,72 @@ def test_is_threepid_reserved(self):
         self.assertTrue(is_threepid_reserved(config, user1))
         self.assertFalse(is_threepid_reserved(config, user3))
         self.assertFalse(is_threepid_reserved(config, user1_msisdn))
+
+
+class FederationBackoffTestCase(unittest.TestCase):
+    """
+    Tests for the "federation_backoff" settings dict.
+    """
+
+    def test_all_defaults(self):
+        """
+        When not defined, the defaults are all the Synapse standard ones.
+        """
+        config = {"server_name": "example.com"}
+        t = ServerConfig()
+        t.read_config(config, config_dir_path="", data_dir_path="")
+
+        fb = t.federation_backoff_settings
+        self.assertEqual(fb.dns_resolution, False)
+        self.assertEqual(fb.dns_servfail, False)
+        self.assertEqual(fb.no_route_to_host, False)
+        self.assertEqual(fb.refused_connection, False)
+        self.assertEqual(fb.cannot_assign_address, False)
+        self.assertEqual(fb.invalid_tls, False)
+        self.assertEqual(fb.on_timeout, False)
+        self.assertEqual(fb.timeout_amount, 60000)
+
+    def test_all_changed(self):
+        """
+        When defined, Synapse will take up the new settings.
+        """
+        config = {
+            "server_name": "example.com",
+            "federation_backoff": {
+                "dns_resolution": True,
+                "dns_servfail": True,
+                "no_route_to_host": True,
+                "refused_connection": True,
+                "cannot_assign_address": True,
+                "invalid_tls": True,
+                "on_timeout": True,
+                "timeout_amount": "30s",
+            },
+        }
+        t = ServerConfig()
+        t.read_config(config, config_dir_path="", data_dir_path="")
+
+        fb = t.federation_backoff_settings
+        self.assertEqual(fb.dns_resolution, True)
+        self.assertEqual(fb.dns_servfail, True)
+        self.assertEqual(fb.no_route_to_host, True)
+        self.assertEqual(fb.refused_connection, True)
+        self.assertEqual(fb.cannot_assign_address, True)
+        self.assertEqual(fb.invalid_tls, True)
+        self.assertEqual(fb.on_timeout, True)
+        self.assertEqual(fb.timeout_amount, 30000)
+
+    def test_timeout_amount_allowed_integer(self):
+        """
+        When federation_backoff.timeout_amount is given as an integer, it will
+        be parsed as ms.
+        """
+        config = {
+            "server_name": "example.com",
+            "federation_backoff": {"timeout_amount": 30001},
+        }
+        t = ServerConfig()
+        t.read_config(config, config_dir_path="", data_dir_path="")
+
+        fb = t.federation_backoff_settings
+        self.assertEqual(fb.timeout_amount, 30001)

From fcea504048ca0676d1dafab58819dcbde5183f35 Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Mon, 8 Jul 2019 21:12:51 +1000
Subject: [PATCH 16/19] reload config and more tests

---
 synapse/http/matrixfederationclient.py | 67 +++++++++++++++-----------
 synapse/server.py                      |  8 +--
 tests/http/test_fedclient.py           | 46 ++++++++++++++++--
 tests/unittest.py                      | 15 ++++++
 tests/utils.py                         |  7 +++
 5 files changed, 106 insertions(+), 37 deletions(-)

diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index df08abb2bce8..2cda31a88951 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -173,6 +173,19 @@ def _handle_json_response(reactor, timeout_sec, request, response):
     defer.returnValue(body)
 
 
+@implementer(IReactorPluggableNameResolver)
+@attr.s
+class _Reactor(object):
+    real_reactor = attr.ib()
+    nameResolver = attr.ib()
+
+    def __getattr__(self, attr):
+        if attr == "nameResolver":
+            return nameResolver
+        else:
+            return getattr(self.real_reactor, attr)
+
+
 class MatrixFederationHttpClient(object):
     """HTTP client used to talk to other homeservers over the federation
     protocol. Send client certificates and signs requests.
@@ -182,49 +195,43 @@ class MatrixFederationHttpClient(object):
             requests.
     """
 
-    def __init__(self, hs, tls_client_options_factory):
+    def __init__(self, hs):
         self.hs = hs
-        self.signing_key = hs.config.signing_key[0]
-        self.server_name = hs.hostname
-        self.backoff_settings = hs.config.federation_backoff_settings
+        self.server_name = self.hs.hostname
+        self.version_string_bytes = hs.version_string.encode("ascii")
+
+        self._real_reactor = hs.get_reactor()
+        self.clock = hs.get_clock()
+        self._store = hs.get_datastore()
+        self._cooperator = Cooperator(
+            scheduler=lambda x: self.reactor.callLater(_EPSILON, x)
+        )
 
-        real_reactor = hs.get_reactor()
+        self.refresh_config()
+
+    def refresh_config(self):
+        self.signing_key = self.hs.config.signing_key[0]
 
         # We need to use a DNS resolver which filters out blacklisted IP
         # addresses, to prevent DNS rebinding.
         nameResolver = IPBlacklistingResolver(
-            real_reactor, None, hs.config.federation_ip_range_blacklist
+            self._real_reactor, None, self.hs.config.federation_ip_range_blacklist
         )
+        self.reactor = _Reactor(self._real_reactor, nameResolver)
 
-        @implementer(IReactorPluggableNameResolver)
-        class Reactor(object):
-            def __getattr__(_self, attr):
-                if attr == "nameResolver":
-                    return nameResolver
-                else:
-                    return getattr(real_reactor, attr)
-
-        self.reactor = Reactor()
-
-        self.agent = MatrixFederationAgent(self.reactor, tls_client_options_factory)
+        self.agent = MatrixFederationAgent(
+            self.reactor, self.hs.get_client_tls_options()
+        )
 
         # Use a BlacklistingAgentWrapper to prevent circumventing the IP
         # blacklist via IP literals in server names
         self.agent = BlacklistingAgentWrapper(
             self.agent,
             self.reactor,
-            ip_blacklist=hs.config.federation_ip_range_blacklist,
+            ip_blacklist=self.hs.config.federation_ip_range_blacklist,
         )
 
-        self.clock = hs.get_clock()
-        self._store = hs.get_datastore()
-        self.version_string_bytes = hs.version_string.encode("ascii")
-        self.default_timeout = self.backoff_settings.timeout_amount
-
-        def schedule(x):
-            self.reactor.callLater(_EPSILON, x)
-
-        self._cooperator = Cooperator(scheduler=schedule)
+        self.backoff_settings = self.hs.config.federation_backoff_settings
 
     @defer.inlineCallbacks
     def _send_request_with_optional_trailing_slash(
@@ -329,7 +336,7 @@ def _send_request(
         if timeout:
             _sec_timeout = timeout / 1000
         else:
-            _sec_timeout = self.default_timeout
+            _sec_timeout = self.backoff_settings.timeout_amount / 1000
 
         if (
             self.hs.config.federation_domain_whitelist is not None
@@ -480,6 +487,8 @@ def _send_request(
                         logger.info(
                             "Failed to send request for unhandled reason: %s", e
                         )
+                        import traceback
+                        logger.debug(traceback.format_exc())
                         raise_from(RequestSendFailed(e, can_retry=True), e)
 
                     logger.info(
@@ -762,7 +771,7 @@ def post_json(
         if timeout:
             _sec_timeout = timeout / 1000
         else:
-            _sec_timeout = self.default_timeout
+            _sec_timeout = self.backoff_settings.timeout_amount / 1000
 
         body = yield _handle_json_response(
             self.reactor, _sec_timeout, request, response
diff --git a/synapse/server.py b/synapse/server.py
index 9e28dba2b1c6..952f94c364d2 100644
--- a/synapse/server.py
+++ b/synapse/server.py
@@ -401,10 +401,7 @@ def build_pusherpool(self):
         return PusherPool(self)
 
     def build_http_client(self):
-        tls_client_options_factory = context_factory.ClientTLSOptionsFactory(
-            self.config
-        )
-        return MatrixFederationHttpClient(self, tls_client_options_factory)
+        return MatrixFederationHttpClient(self)
 
     def build_db_pool(self):
         name = self.db_config["name"]
@@ -413,6 +410,9 @@ def build_db_pool(self):
             name, cp_reactor=self.get_reactor(), **self.db_config.get("args", {})
         )
 
+    def get_client_tls_options(self):
+        return context_factory.ClientTLSOptionsFactory(self.config)
+
     def get_db_conn(self, run_new_connection=True):
         """Makes a new connection to the database, skipping the db pool
 
diff --git a/tests/http/test_fedclient.py b/tests/http/test_fedclient.py
index b9d6d7ad1c1f..f5c64d8731a7 100644
--- a/tests/http/test_fedclient.py
+++ b/tests/http/test_fedclient.py
@@ -32,7 +32,7 @@
 from synapse.logging.context import LoggingContext
 
 from tests.server import FakeTransport
-from tests.unittest import HomeserverTestCase
+from tests import unittest
 
 
 def check_logcontext(context):
@@ -41,13 +41,13 @@ def check_logcontext(context):
         raise AssertionError("Expected logcontext %s but was %s" % (context, current))
 
 
-class FederationClientTests(HomeserverTestCase):
+class FederationClientTests(unittest.HomeserverTestCase):
     def make_homeserver(self, reactor, clock):
         hs = self.setup_test_homeserver(reactor=reactor, clock=clock)
         return hs
 
     def prepare(self, reactor, clock, homeserver):
-        self.cl = MatrixFederationHttpClient(self.hs, None)
+        self.cl = MatrixFederationHttpClient(self.hs)
         self.reactor.lookups["testserv"] = "1.2.3.4"
 
     def test_client_get(self):
@@ -221,7 +221,7 @@ def test_client_ip_range_blacklist(self):
         self.reactor.lookups["internal"] = "127.0.0.1"
         self.reactor.lookups["internalv6"] = "fe80:0:0:0:0:8a2e:370:7337"
         self.reactor.lookups["fine"] = "10.20.30.40"
-        cl = MatrixFederationHttpClient(self.hs, None)
+        cl = MatrixFederationHttpClient(self.hs)
 
         # Try making a GET request to a blacklisted IPv4 address
         # ------------------------------------------------------
@@ -489,3 +489,41 @@ def test_closes_connection(self):
         self.pump(120)
 
         self.assertTrue(conn.disconnecting)
+
+    @unittest.INFO
+    def test_client_respects_timeout_backoff(self):
+        """
+        If federation_backoff.on_timeout is set, Synapse will immediately
+        backoff on a timeout.
+        """
+
+        self.amend_config({"federation_backoff": {"on_timeout": True}})
+        self.cl.refresh_config()
+
+        d = self.cl.get_json("testserv:8008", "foo/bar")
+
+        # Send the request
+        self.pump()
+
+        # there should have been a call to connectTCP
+        clients = self.reactor.tcpClients
+        self.assertEqual(len(clients), 1)
+        (_host, _port, factory, _timeout, _bindAddress) = clients[0]
+
+        # complete the connection and wire it up to a fake transport
+        client = factory.buildProtocol(None)
+        conn = StringTransport()
+        client.makeConnection(conn)
+
+        # that should have made it send the request to the connection
+        self.assertRegex(conn.value(), b"^GET /foo/bar")
+
+        # Clear the original request data before sending a response
+        conn.clear()
+
+        # Timeout.
+        self.pump(120)
+
+        # We should get a 404 failure response
+        f = self.failureResultOf(d)
+        print(f)
diff --git a/tests/unittest.py b/tests/unittest.py
index a09e76c7c21d..f7e3765a2548 100644
--- a/tests/unittest.py
+++ b/tests/unittest.py
@@ -379,6 +379,21 @@ def setup_test_homeserver(self, *args, **kwargs):
 
         return hs
 
+    def amend_config(self, config, with_default=True):
+        """
+        Amend the homeserver's config during a test.
+        """
+        conf = dict(self._hs_args)
+
+        if with_default:
+            conf.update(self.default_config())
+
+        conf.update(config)
+        config_obj = HomeServerConfig()
+        config_obj.parse_config_dict(conf, "", "")
+
+        self.hs.config = config_obj
+
     def pump(self, by=0.0):
         """
         Pump the reactor enough that Deferreds will fire.
diff --git a/tests/utils.py b/tests/utils.py
index 8a94ce0b475d..c4d6243b8bbd 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -184,6 +184,13 @@ def default_config(name, parse=False):
 
 class TestHomeServer(HomeServer):
     DATASTORE_CLASS = DataStore
+    _client_tls_options = None
+
+    def get_client_tls_options(self):
+        """
+        No client TLS here.
+        """
+        return self._client_tls_options
 
 
 @defer.inlineCallbacks

From c579143d9017e9513d24dcb69d1a7161428615af Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Mon, 8 Jul 2019 21:39:10 +1000
Subject: [PATCH 17/19] proper config reloading & simplification

---
 synapse/http/matrixfederationclient.py | 21 ++++++++-------
 synapse/server.py                      | 20 ++++++++++++++
 tests/http/test_fedclient.py           | 36 ++++++++++++++++----------
 tests/unittest.py                      |  1 +
 4 files changed, 55 insertions(+), 23 deletions(-)

diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 2cda31a88951..a69843e75140 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -207,9 +207,9 @@ def __init__(self, hs):
             scheduler=lambda x: self.reactor.callLater(_EPSILON, x)
         )
 
-        self.refresh_config()
+        self.reload_config()
 
-    def refresh_config(self):
+    def reload_config(self):
         self.signing_key = self.hs.config.signing_key[0]
 
         # We need to use a DNS resolver which filters out blacklisted IP
@@ -488,6 +488,7 @@ def _send_request(
                             "Failed to send request for unhandled reason: %s", e
                         )
                         import traceback
+
                         logger.debug(traceback.format_exc())
                         raise_from(RequestSendFailed(e, can_retry=True), e)
 
@@ -657,7 +658,7 @@ def put_json(
 
             timeout (int|None): number of milliseconds to wait for the response headers
                 (including connecting to the server), *for each attempt*.
-                self._default_timeout (60s) by default.
+                self.backoff_settings.timeout_amount (60s) by default.
 
             ignore_backoff (bool): true to ignore the historical backoff data
                 and try the request anyway.
@@ -704,7 +705,7 @@ def put_json(
         )
 
         body = yield _handle_json_response(
-            self.reactor, self.default_timeout, request, response
+            self.reactor, self.backoff_settings.timeout_amount, request, response
         )
 
         defer.returnValue(body)
@@ -736,7 +737,7 @@ def post_json(
 
             timeout (int|None): number of milliseconds to wait for the response headers
                 (including connecting to the server), *for each attempt*.
-                self._default_timeout (60s) by default.
+                self.backoff_settings.timeout_amount (60s) by default.
 
             ignore_backoff (bool): true to ignore the historical backoff data and
                 try the request anyway.
@@ -802,7 +803,7 @@ def get_json(
 
             timeout (int|None): number of milliseconds to wait for the response headers
                 (including connecting to the server), *for each attempt*.
-                self._default_timeout (60s) by default.
+                self.backoff_settings.timeout_amount (60s) by default.
 
             ignore_backoff (bool): true to ignore the historical backoff data
                 and try the request anyway.
@@ -838,7 +839,7 @@ def get_json(
         )
 
         body = yield _handle_json_response(
-            self.reactor, self.default_timeout, request, response
+            self.reactor, self.backoff_settings.timeout_amount, request, response
         )
 
         defer.returnValue(body)
@@ -865,7 +866,7 @@ def delete_json(
 
             timeout (int|None): number of milliseconds to wait for the response headers
                 (including connecting to the server), *for each attempt*.
-                self._default_timeout (60s) by default.
+                self.backoff_settings.timeout_amount (60s) by default.
 
             ignore_backoff (bool): true to ignore the historical backoff data and
                 try the request anyway.
@@ -897,7 +898,7 @@ def delete_json(
         )
 
         body = yield _handle_json_response(
-            self.reactor, self.default_timeout, request, response
+            self.reactor, self.backoff_settings.timeout_amount, request, response
         )
         defer.returnValue(body)
 
@@ -947,7 +948,7 @@ def get_file(
 
         try:
             d = _readBodyToFile(response, output_stream, max_size)
-            d.addTimeout(self.default_timeout, self.reactor)
+            d.addTimeout(self.backoff_settings.timeout_amount, self.reactor)
             length = yield make_deferred_yieldable(d)
         except Exception as e:
             logger.warn(
diff --git a/synapse/server.py b/synapse/server.py
index 952f94c364d2..2d4e1c334c2b 100644
--- a/synapse/server.py
+++ b/synapse/server.py
@@ -543,6 +543,26 @@ def should_send_federation(self):
             or self.config.worker_app == "synapse.app.federation_sender"
         )
 
+    def reload_config(self):
+        """
+        Reload the config of every loaded module.
+        """
+        for depname in self.DEPENDENCIES:
+            try:
+                dep = getattr(self, depname)
+                try:
+                    reloader = getattr(dep, "reload_config")
+                    reloader()
+                    logger.info("Reloaded the config of %s", depname)
+                except AttributeError:
+                    logger.info("Skipping reloading the config of %s", depname)
+                except Exception:
+                    logger.error("Failed reloading the config of %s!", depname)
+                    raise ValueError("Failed reloading the config of %s!" % (depname,))
+            except AttributeError:
+                # Dependency is not loaded yet.
+                pass
+
 
 def _make_dependency_method(depname):
     def _get(hs):
diff --git a/tests/http/test_fedclient.py b/tests/http/test_fedclient.py
index f5c64d8731a7..35d36c68d29b 100644
--- a/tests/http/test_fedclient.py
+++ b/tests/http/test_fedclient.py
@@ -47,7 +47,7 @@ def make_homeserver(self, reactor, clock):
         return hs
 
     def prepare(self, reactor, clock, homeserver):
-        self.cl = MatrixFederationHttpClient(self.hs)
+        self.cl = self.hs.get_http_client()
         self.reactor.lookups["testserv"] = "1.2.3.4"
 
     def test_client_get(self):
@@ -215,18 +215,18 @@ def test_client_ip_range_blacklist(self):
         """Ensure that Synapse does not try to connect to blacklisted IPs"""
 
         # Set up the ip_range blacklist
-        self.hs.config.federation_ip_range_blacklist = IPSet(
-            ["127.0.0.0/8", "fe80::/64"]
-        )
+
+        self.amend_config({
+            "federation_ip_range_blacklist": ["127.0.0.0/8", "fe80::/64"]
+        })
         self.reactor.lookups["internal"] = "127.0.0.1"
         self.reactor.lookups["internalv6"] = "fe80:0:0:0:0:8a2e:370:7337"
         self.reactor.lookups["fine"] = "10.20.30.40"
-        cl = MatrixFederationHttpClient(self.hs)
 
         # Try making a GET request to a blacklisted IPv4 address
         # ------------------------------------------------------
         # Make the request
-        d = cl.get_json("internal:8008", "foo/bar", timeout=10000)
+        d = self.cl.get_json("internal:8008", "foo/bar", timeout=10000)
 
         # Nothing happened yet
         self.assertNoResult(d)
@@ -244,7 +244,7 @@ def test_client_ip_range_blacklist(self):
         # Try making a POST request to a blacklisted IPv6 address
         # -------------------------------------------------------
         # Make the request
-        d = cl.post_json("internalv6:8008", "foo/bar", timeout=10000)
+        d = self.cl.post_json("internalv6:8008", "foo/bar", timeout=10000)
 
         # Nothing has happened yet
         self.assertNoResult(d)
@@ -263,7 +263,7 @@ def test_client_ip_range_blacklist(self):
         # Try making a GET request to a non-blacklisted IPv4 address
         # ----------------------------------------------------------
         # Make the request
-        d = cl.post_json("fine:8008", "foo/bar", timeout=10000)
+        d = self.cl.post_json("fine:8008", "foo/bar", timeout=10000)
 
         # Nothing has happened yet
         self.assertNoResult(d)
@@ -496,9 +496,7 @@ def test_client_respects_timeout_backoff(self):
         If federation_backoff.on_timeout is set, Synapse will immediately
         backoff on a timeout.
         """
-
         self.amend_config({"federation_backoff": {"on_timeout": True}})
-        self.cl.refresh_config()
 
         d = self.cl.get_json("testserv:8008", "foo/bar")
 
@@ -521,9 +519,21 @@ def test_client_respects_timeout_backoff(self):
         # Clear the original request data before sending a response
         conn.clear()
 
-        # Timeout.
+        # No retry interval for this destination.
+        retry_timings = self.get_success(
+            self.hs.get_datastore().get_destination_retry_timings("testserv:8008")
+        )
+        self.assertIsNone(retry_timings)
+
+        # Timeout the connection.
         self.pump(120)
 
-        # We should get a 404 failure response
+        # We should get the response failure bubbled up.
         f = self.failureResultOf(d)
-        print(f)
+        self.assertIsNotNone(f.check(RequestSendFailed))
+
+        # Retry in 600s
+        retry_timings = self.get_success(
+            self.hs.get_datastore().get_destination_retry_timings("testserv:8008")
+        )
+        self.assertEqual(retry_timings["retry_interval"], 600000)
diff --git a/tests/unittest.py b/tests/unittest.py
index f7e3765a2548..bae963a9454a 100644
--- a/tests/unittest.py
+++ b/tests/unittest.py
@@ -393,6 +393,7 @@ def amend_config(self, config, with_default=True):
         config_obj.parse_config_dict(conf, "", "")
 
         self.hs.config = config_obj
+        self.hs.reload_config()
 
     def pump(self, by=0.0):
         """

From 294c3f0eb9ede62af63f98e4e006b5af70755c5c Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Mon, 8 Jul 2019 21:40:06 +1000
Subject: [PATCH 18/19] flake

---
 synapse/http/matrixfederationclient.py |  2 +-
 tests/config/test_server.py            |  2 +-
 tests/http/test_fedclient.py           | 15 +++++----------
 3 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index a69843e75140..74b0a15ad922 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -181,7 +181,7 @@ class _Reactor(object):
 
     def __getattr__(self, attr):
         if attr == "nameResolver":
-            return nameResolver
+            return self.nameResolver
         else:
             return getattr(self.real_reactor, attr)
 
diff --git a/tests/config/test_server.py b/tests/config/test_server.py
index 3cfead7970e8..34cc4e225896 100644
--- a/tests/config/test_server.py
+++ b/tests/config/test_server.py
@@ -14,7 +14,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from synapse.config.server import ConfigError, ServerConfig, is_threepid_reserved
+from synapse.config.server import ServerConfig, is_threepid_reserved
 
 from tests import unittest
 
diff --git a/tests/http/test_fedclient.py b/tests/http/test_fedclient.py
index 35d36c68d29b..138a32b50ae4 100644
--- a/tests/http/test_fedclient.py
+++ b/tests/http/test_fedclient.py
@@ -15,8 +15,6 @@
 
 from mock import Mock
 
-from netaddr import IPSet
-
 from twisted.internet import defer
 from twisted.internet.defer import TimeoutError
 from twisted.internet.error import ConnectingCancelledError, DNSLookupError
@@ -25,14 +23,11 @@
 from twisted.web.http import HTTPChannel
 
 from synapse.api.errors import RequestSendFailed
-from synapse.http.matrixfederationclient import (
-    MatrixFederationHttpClient,
-    MatrixFederationRequest,
-)
+from synapse.http.matrixfederationclient import MatrixFederationRequest
 from synapse.logging.context import LoggingContext
 
-from tests.server import FakeTransport
 from tests import unittest
+from tests.server import FakeTransport
 
 
 def check_logcontext(context):
@@ -216,9 +211,9 @@ def test_client_ip_range_blacklist(self):
 
         # Set up the ip_range blacklist
 
-        self.amend_config({
-            "federation_ip_range_blacklist": ["127.0.0.0/8", "fe80::/64"]
-        })
+        self.amend_config(
+            {"federation_ip_range_blacklist": ["127.0.0.0/8", "fe80::/64"]}
+        )
         self.reactor.lookups["internal"] = "127.0.0.1"
         self.reactor.lookups["internalv6"] = "fe80:0:0:0:0:8a2e:370:7337"
         self.reactor.lookups["fine"] = "10.20.30.40"

From 1391f612b3c576a20bc55908625db980c53d56ab Mon Sep 17 00:00:00 2001
From: "Amber H. Brown" <hawkowl@atleastfornow.net>
Date: Mon, 8 Jul 2019 21:46:11 +1000
Subject: [PATCH 19/19] clean up some things

---
 synapse/http/matrixfederationclient.py | 5 +++--
 tests/http/test_fedclient.py           | 1 -
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 74b0a15ad922..a1f09653de4f 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 # Copyright 2014-2016 OpenMarket Ltd
 # Copyright 2018 New Vector Ltd
+# Copyright 2019 Matrix.org Foundation C.I.C.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -13,10 +14,12 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
+
 import cgi
 import logging
 import random
 import sys
+import traceback
 from io import BytesIO
 
 from six import PY3, raise_from, string_types
@@ -487,8 +490,6 @@ def _send_request(
                         logger.info(
                             "Failed to send request for unhandled reason: %s", e
                         )
-                        import traceback
-
                         logger.debug(traceback.format_exc())
                         raise_from(RequestSendFailed(e, can_retry=True), e)
 
diff --git a/tests/http/test_fedclient.py b/tests/http/test_fedclient.py
index 138a32b50ae4..1c1b38ab9429 100644
--- a/tests/http/test_fedclient.py
+++ b/tests/http/test_fedclient.py
@@ -485,7 +485,6 @@ def test_closes_connection(self):
 
         self.assertTrue(conn.disconnecting)
 
-    @unittest.INFO
     def test_client_respects_timeout_backoff(self):
         """
         If federation_backoff.on_timeout is set, Synapse will immediately