From cf94d173dd678a89d5802e40110fddd23dd8e55b Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Thu, 16 Dec 2021 16:24:44 +0100
Subject: [PATCH 01/14] Fix get federation status of destination if no error
 occured

---
 synapse/rest/admin/federation.py              | 14 +++-
 .../storage/databases/main/transactions.py    | 10 +++
 tests/rest/admin/test_federation.py           | 76 ++++++++++++++-----
 3 files changed, 78 insertions(+), 22 deletions(-)

diff --git a/synapse/rest/admin/federation.py b/synapse/rest/admin/federation.py
index 50d88c91091b..1aa958a8b758 100644
--- a/synapse/rest/admin/federation.py
+++ b/synapse/rest/admin/federation.py
@@ -19,7 +19,10 @@
 from synapse.http.servlet import RestServlet, parse_integer, parse_string
 from synapse.http.site import SynapseRequest
 from synapse.rest.admin._base import admin_patterns, assert_requester_is_admin
-from synapse.storage.databases.main.transactions import DestinationSortOrder
+from synapse.storage.databases.main.transactions import (
+    DestinationRetryTimings,
+    DestinationSortOrder,
+)
 from synapse.types import JsonDict
 
 if TYPE_CHECKING:
@@ -111,12 +114,17 @@ async def on_GET(
     ) -> Tuple[int, JsonDict]:
         await assert_requester_is_admin(self._auth, request)
 
+        if not await self._store.is_destination(destination):
+            raise NotFoundError("Unknown destination")
+
         destination_retry_timings = await self._store.get_destination_retry_timings(
             destination
         )
 
-        if not destination_retry_timings:
-            raise NotFoundError("Unknown destination")
+        if destination_retry_timings is None:
+            destination_retry_timings = DestinationRetryTimings(
+                failure_ts=None, retry_last_ts=0, retry_interval=0
+            )
 
         last_successful_stream_ordering = (
             await self._store.get_destination_last_successful_stream_ordering(
diff --git a/synapse/storage/databases/main/transactions.py b/synapse/storage/databases/main/transactions.py
index 54b41513ee13..664bd186ad72 100644
--- a/synapse/storage/databases/main/transactions.py
+++ b/synapse/storage/databases/main/transactions.py
@@ -559,3 +559,13 @@ def get_destinations_paginate_txn(
         return await self.db_pool.runInteraction(
             "get_destinations_paginate_txn", get_destinations_paginate_txn
         )
+
+    async def is_destination(self, destination: str) -> Optional[bool]:
+        """Function to check if a destination is a destination of this server."""
+        return await self.db_pool.simple_select_one_onecol(
+            table="destinations",
+            keyvalues={"destination": destination},
+            retcol="1",
+            allow_none=True,
+            desc="is_destination",
+        )
diff --git a/tests/rest/admin/test_federation.py b/tests/rest/admin/test_federation.py
index d1cd5b075157..65aaebc93fd1 100644
--- a/tests/rest/admin/test_federation.py
+++ b/tests/rest/admin/test_federation.py
@@ -311,15 +311,12 @@ def _order_test(
             retry_interval,
             last_successful_stream_ordering,
         ) in dest:
-            self.get_success(
-                self.store.set_destination_retry_timings(
-                    destination, failure_ts, retry_last_ts, retry_interval
-                )
-            )
-            self.get_success(
-                self.store.set_destination_last_successful_stream_ordering(
-                    destination, last_successful_stream_ordering
-                )
+            self._create_destination(
+                destination,
+                failure_ts,
+                retry_last_ts,
+                retry_interval,
+                last_successful_stream_ordering,
             )
 
         # order by default (destination)
@@ -410,11 +407,9 @@ def _search_test(
         _search_test(None, "foo")
         _search_test(None, "bar")
 
-    def test_get_single_destination(self):
-        """
-        Get one specific destinations.
-        """
-        self._create_destinations(5)
+    def test_get_single_destination_with_retry_timings(self) -> None:
+        """Get one specific destination which has retry timings."""
+        self._create_destinations(1)
 
         channel = self.make_request(
             "GET",
@@ -429,7 +424,53 @@ def test_get_single_destination(self):
         # convert channel.json_body into a List
         self._check_fields([channel.json_body])
 
-    def _create_destinations(self, number_destinations: int):
+    def test_get_single_destination_no_retry_timings(self) -> None:
+        """Get one specific destination which has no retry timings."""
+        self._create_destination("sub0.example.com")
+
+        channel = self.make_request(
+            "GET",
+            self.url + "/sub0.example.com",
+            access_token=self.admin_user_tok,
+        )
+
+        self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body)
+        self.assertEqual("sub0.example.com", channel.json_body["destination"])
+        self.assertEqual(0, channel.json_body["retry_last_ts"])
+        self.assertEqual(0, channel.json_body["retry_interval"])
+        self.assertIsNone(channel.json_body["failure_ts"])
+        self.assertIsNone(channel.json_body["last_successful_stream_ordering"])
+
+    def _create_destination(
+        self,
+        destination: str,
+        failure_ts: Optional[int] = None,
+        retry_last_ts: int = 0,
+        retry_interval: int = 0,
+        last_successful_stream_ordering: Optional[int] = None,
+    ) -> None:
+        """Create one specific destination
+
+        Args:
+            destination: the destination we have successfully sent to
+            failure_ts: when the server started failing (ms since epoch)
+            retry_last_ts: time of last retry attempt in unix epoch ms
+            retry_interval: how long until next retry in ms
+            last_successful_stream_ordering: the stream_ordering of the most
+                recent successfully-sent PDU
+        """
+        self.get_success(
+            self.store.set_destination_retry_timings(
+                destination, failure_ts, retry_last_ts, retry_interval
+            )
+        )
+        self.get_success(
+            self.store.set_destination_last_successful_stream_ordering(
+                destination, last_successful_stream_ordering
+            )
+        )
+
+    def _create_destinations(self, number_destinations: int) -> None:
         """Create a number of destinations
 
         Args:
@@ -437,10 +478,7 @@ def _create_destinations(self, number_destinations: int):
         """
         for i in range(0, number_destinations):
             dest = f"sub{i}.example.com"
-            self.get_success(self.store.set_destination_retry_timings(dest, 50, 50, 50))
-            self.get_success(
-                self.store.set_destination_last_successful_stream_ordering(dest, 100)
-            )
+            self._create_destination(dest, 50, 50, 50, 100)
 
     def _check_fields(self, content: List[JsonDict]):
         """Checks that the expected destination attributes are present in content

From c2cc7bc5ba05c35155b97c1e440e96005792b0e2 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Thu, 16 Dec 2021 16:34:59 +0100
Subject: [PATCH 02/14] newsfile

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

diff --git a/changelog.d/11593.bugfix b/changelog.d/11593.bugfix
new file mode 100644
index 000000000000..af67ec1a8998
--- /dev/null
+++ b/changelog.d/11593.bugfix
@@ -0,0 +1 @@
+Fix an error to get federation status of a destination server even if no error has occurred.
\ No newline at end of file

From d07bf85340c94827bebccee78f758c3a9813ef81 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Thu, 16 Dec 2021 17:54:39 +0100
Subject: [PATCH 03/14] fix mypy errors

---
 synapse/rest/admin/federation.py    | 26 +++++++++++++++-----------
 tests/rest/admin/test_federation.py |  9 +++++----
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/synapse/rest/admin/federation.py b/synapse/rest/admin/federation.py
index 1aa958a8b758..0cc150b88cd3 100644
--- a/synapse/rest/admin/federation.py
+++ b/synapse/rest/admin/federation.py
@@ -19,10 +19,7 @@
 from synapse.http.servlet import RestServlet, parse_integer, parse_string
 from synapse.http.site import SynapseRequest
 from synapse.rest.admin._base import admin_patterns, assert_requester_is_admin
-from synapse.storage.databases.main.transactions import (
-    DestinationRetryTimings,
-    DestinationSortOrder,
-)
+from synapse.storage.databases.main.transactions import DestinationSortOrder
 from synapse.types import JsonDict
 
 if TYPE_CHECKING:
@@ -121,10 +118,19 @@ async def on_GET(
             destination
         )
 
-        if destination_retry_timings is None:
-            destination_retry_timings = DestinationRetryTimings(
-                failure_ts=None, retry_last_ts=0, retry_interval=0
-            )
+        retry_timing_respone: JsonDict = {}
+        if destination_retry_timings:
+            retry_timing_respone = {
+                "failure_ts": destination_retry_timings.failure_ts,
+                "retry_last_ts": destination_retry_timings.retry_last_ts,
+                "retry_interval": destination_retry_timings.retry_interval,
+            }
+        else:
+            retry_timing_respone = {
+                "failure_ts": None,
+                "retry_last_ts": 0,
+                "retry_interval": 0,
+            }
 
         last_successful_stream_ordering = (
             await self._store.get_destination_last_successful_stream_ordering(
@@ -134,10 +140,8 @@ async def on_GET(
 
         response = {
             "destination": destination,
-            "failure_ts": destination_retry_timings.failure_ts,
-            "retry_last_ts": destination_retry_timings.retry_last_ts,
-            "retry_interval": destination_retry_timings.retry_interval,
             "last_successful_stream_ordering": last_successful_stream_ordering,
+            **retry_timing_respone,
         }
 
         return HTTPStatus.OK, response
diff --git a/tests/rest/admin/test_federation.py b/tests/rest/admin/test_federation.py
index 65aaebc93fd1..510ed8989d4c 100644
--- a/tests/rest/admin/test_federation.py
+++ b/tests/rest/admin/test_federation.py
@@ -464,11 +464,12 @@ def _create_destination(
                 destination, failure_ts, retry_last_ts, retry_interval
             )
         )
-        self.get_success(
-            self.store.set_destination_last_successful_stream_ordering(
-                destination, last_successful_stream_ordering
+        if last_successful_stream_ordering is not None:
+            self.get_success(
+                self.store.set_destination_last_successful_stream_ordering(
+                    destination, last_successful_stream_ordering
+                )
             )
-        )
 
     def _create_destinations(self, number_destinations: int) -> None:
         """Create a number of destinations

From 9cc1396acd3c09b731086affec60e952467e451a Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Thu, 23 Dec 2021 16:53:13 +0100
Subject: [PATCH 04/14] Add admin API to reset connection timeouts for remote
 server

---
 .../administration/admin_api/federation.md    | 29 +++++++++-
 synapse/federation/transport/server/_base.py  |  4 +-
 synapse/rest/admin/__init__.py                |  6 +-
 synapse/rest/admin/federation.py              | 43 ++++++++++++++-
 tests/rest/admin/test_federation.py           | 55 +++++++++++++++++--
 5 files changed, 127 insertions(+), 10 deletions(-)

diff --git a/docs/usage/administration/admin_api/federation.md b/docs/usage/administration/admin_api/federation.md
index 8f9535f57b09..94d766f57913 100644
--- a/docs/usage/administration/admin_api/federation.md
+++ b/docs/usage/administration/admin_api/federation.md
@@ -86,7 +86,7 @@ The following fields are returned in the JSON response body:
 - `next_token`: string representing a positive integer - Indication for pagination. See above.
 - `total` - integer - Total number of destinations.
 
-# Destination Details API
+## Destination Details API
 
 This API gets the retry timing info for a specific remote server.
 
@@ -108,7 +108,34 @@ A response body like the following is returned:
 }
 ```
 
+**Parameters**
+
+The following parameters should be set in the URL:
+
+- `destination` - Name of the remote server.
+
 **Response**
 
 The response fields are the same like in the `destinations` array in
 [List of destinations](#list-of-destinations) response.
+
+## Reset connection timeout
+
+This API resets the retry timing for a specific remote server and tries to connect
+the remote server again. It does not wait for the next `retry_interval`.
+The connection must have previously run into an error and `retry_last_ts`
+([Destination Details API](#destination-details-api)) must not be equal to `0`.
+
+The API is:
+
+```
+POST /_synapse/admin/v1/federation/destinations/<destination>/reset_connection
+
+{}
+```
+
+**Parameters**
+
+The following parameters should be set in the URL:
+
+- `destination` - Name of the remote server.
diff --git a/synapse/federation/transport/server/_base.py b/synapse/federation/transport/server/_base.py
index da1fbf8b6361..15d7740c2e86 100644
--- a/synapse/federation/transport/server/_base.py
+++ b/synapse/federation/transport/server/_base.py
@@ -114,11 +114,11 @@ async def authenticate_request(
         # alive
         retry_timings = await self.store.get_destination_retry_timings(origin)
         if retry_timings and retry_timings.retry_last_ts:
-            run_in_background(self._reset_retry_timings, origin)
+            run_in_background(self.reset_retry_timings, origin)
 
         return origin
 
-    async def _reset_retry_timings(self, origin: str) -> None:
+    async def reset_retry_timings(self, origin: str) -> None:
         try:
             logger.info("Marking origin %r as up", origin)
             await self.store.set_destination_retry_timings(origin, None, 0, 0)
diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py
index 701c609c1208..d1df40c7e44d 100644
--- a/synapse/rest/admin/__init__.py
+++ b/synapse/rest/admin/__init__.py
@@ -41,7 +41,8 @@
     EventReportsRestServlet,
 )
 from synapse.rest.admin.federation import (
-    DestinationsRestServlet,
+    DestinationResetConnectionRestServlet,
+    DestinationRestServlet,
     ListDestinationsRestServlet,
 )
 from synapse.rest.admin.groups import DeleteGroupAdminRestServlet
@@ -265,7 +266,8 @@ def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None:
     ListRegistrationTokensRestServlet(hs).register(http_server)
     NewRegistrationTokenRestServlet(hs).register(http_server)
     RegistrationTokenRestServlet(hs).register(http_server)
-    DestinationsRestServlet(hs).register(http_server)
+    DestinationResetConnectionRestServlet(hs).register(http_server)
+    DestinationRestServlet(hs).register(http_server)
     ListDestinationsRestServlet(hs).register(http_server)
 
     # Some servlets only get registered for the main process.
diff --git a/synapse/rest/admin/federation.py b/synapse/rest/admin/federation.py
index 0cc150b88cd3..3a3a39244dc6 100644
--- a/synapse/rest/admin/federation.py
+++ b/synapse/rest/admin/federation.py
@@ -16,6 +16,7 @@
 from typing import TYPE_CHECKING, Tuple
 
 from synapse.api.errors import Codes, NotFoundError, SynapseError
+from synapse.federation.transport.server._base import Authenticator
 from synapse.http.servlet import RestServlet, parse_integer, parse_string
 from synapse.http.site import SynapseRequest
 from synapse.rest.admin._base import admin_patterns, assert_requester_is_admin
@@ -90,7 +91,7 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
         return HTTPStatus.OK, response
 
 
-class DestinationsRestServlet(RestServlet):
+class DestinationRestServlet(RestServlet):
     """Get details of a destination.
     This needs user to have administrator access in Synapse.
 
@@ -145,3 +146,43 @@ async def on_GET(
         }
 
         return HTTPStatus.OK, response
+
+
+class DestinationResetConnectionRestServlet(RestServlet):
+    """Reset destinations' connection timeouts and wake it up.
+    This needs user to have administrator access in Synapse.
+
+    POST /_synapse/admin/v1/federation/destinations/<destination>/reset_connection
+    {}
+
+    returns:
+        200 OK otherwise an error.
+    """
+
+    PATTERNS = admin_patterns(
+        "/federation/destinations/(?P<destination>[^/]+)/reset_connection$"
+    )
+
+    def __init__(self, hs: "HomeServer"):
+        self._auth = hs.get_auth()
+        self._store = hs.get_datastore()
+        self._authenticator = Authenticator(hs)
+
+    async def on_POST(
+        self, request: SynapseRequest, destination: str
+    ) -> Tuple[int, JsonDict]:
+        await assert_requester_is_admin(self._auth, request)
+
+        if not await self._store.is_destination(destination):
+            raise NotFoundError("Unknown destination")
+
+        retry_timings = await self._store.get_destination_retry_timings(destination)
+        if not (retry_timings and retry_timings.retry_last_ts):
+            raise SynapseError(
+                HTTPStatus.BAD_REQUEST,
+                "The retry timing does not need to be reset for this destination.",
+            )
+
+        await self._authenticator.reset_retry_timings(destination)
+
+        return HTTPStatus.OK, {}
diff --git a/tests/rest/admin/test_federation.py b/tests/rest/admin/test_federation.py
index b70350b6f1d7..e2d3cff2a3d8 100644
--- a/tests/rest/admin/test_federation.py
+++ b/tests/rest/admin/test_federation.py
@@ -43,11 +43,15 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
 
     @parameterized.expand(
         [
-            ("/_synapse/admin/v1/federation/destinations",),
-            ("/_synapse/admin/v1/federation/destinations/dummy",),
+            ("GET", "/_synapse/admin/v1/federation/destinations"),
+            ("GET", "/_synapse/admin/v1/federation/destinations/dummy"),
+            (
+                "POST",
+                "/_synapse/admin/v1/federation/destinations/dummy/reset_connection",
+            ),
         ]
     )
-    def test_requester_is_no_admin(self, url: str) -> None:
+    def test_requester_is_no_admin(self, method: str, url: str) -> None:
         """
         If the user is not a server admin, an error 403 is returned.
         """
@@ -56,7 +60,7 @@ def test_requester_is_no_admin(self, url: str) -> None:
         other_user_tok = self.login("user", "pass")
 
         channel = self.make_request(
-            "GET",
+            method,
             url,
             content={},
             access_token=other_user_tok,
@@ -120,6 +124,16 @@ def test_invalid_parameter(self) -> None:
         self.assertEqual(HTTPStatus.NOT_FOUND, channel.code, msg=channel.json_body)
         self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"])
 
+        # invalid destination
+        channel = self.make_request(
+            "POST",
+            self.url + "/dummy/reset_connection",
+            access_token=self.admin_user_tok,
+        )
+
+        self.assertEqual(HTTPStatus.NOT_FOUND, channel.code, msg=channel.json_body)
+        self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"])
+
     def test_limit(self) -> None:
         """
         Testing list of destinations with limit
@@ -444,6 +458,39 @@ def test_get_single_destination_no_retry_timings(self) -> None:
         self.assertIsNone(channel.json_body["failure_ts"])
         self.assertIsNone(channel.json_body["last_successful_stream_ordering"])
 
+    def test_destination_reset_connection(self) -> None:
+        """Reset timeouts and wake up destination."""
+        self._create_destination("sub0.example.com", 100, 100, 100)
+
+        channel = self.make_request(
+            "POST",
+            self.url + "/sub0.example.com/reset_connection",
+            access_token=self.admin_user_tok,
+        )
+
+        self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body)
+
+        retry_timings = self.get_success(
+            self.store.get_destination_retry_timings("sub0.example.com")
+        )
+        self.assertIsNone(retry_timings)
+
+    def test_destination_reset_connection_not_required(self) -> None:
+        """Try to reset timeouts of a destination with no timeouts and get an error."""
+        self._create_destination("sub0.example.com", None, 0, 0)
+
+        channel = self.make_request(
+            "POST",
+            self.url + "/sub0.example.com/reset_connection",
+            access_token=self.admin_user_tok,
+        )
+
+        self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body)
+        self.assertEqual(
+            "The retry timing does not need to be reset for this destination.",
+            channel.json_body["error"],
+        )
+
     def _create_destination(
         self,
         destination: str,

From dafbd9339f27498af93d66a33f053548b7eb0211 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Thu, 23 Dec 2021 17:06:23 +0100
Subject: [PATCH 05/14] newsfile

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

diff --git a/changelog.d/11639.feature b/changelog.d/11639.feature
new file mode 100644
index 000000000000..e9f6704f7a1f
--- /dev/null
+++ b/changelog.d/11639.feature
@@ -0,0 +1 @@
+Add admin API to reset connection timeouts for remote server.
\ No newline at end of file

From 954605657bebec1d6d6570509d01970d7515fb7c Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Tue, 28 Dec 2021 14:25:13 +0100
Subject: [PATCH 06/14] Replace uses of `Authenticator._reset_retry_timings`

---
 docs/usage/administration/admin_api/federation.md | 3 +++
 synapse/federation/transport/server/_base.py      | 4 ++--
 synapse/rest/admin/federation.py                  | 7 ++++---
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/docs/usage/administration/admin_api/federation.md b/docs/usage/administration/admin_api/federation.md
index 94d766f57913..66d6f3c254a3 100644
--- a/docs/usage/administration/admin_api/federation.md
+++ b/docs/usage/administration/admin_api/federation.md
@@ -126,6 +126,9 @@ the remote server again. It does not wait for the next `retry_interval`.
 The connection must have previously run into an error and `retry_last_ts`
 ([Destination Details API](#destination-details-api)) must not be equal to `0`.
 
+The connection attempt is carried out in the background and can take a while
+even if the API already returns the http status 200.
+
 The API is:
 
 ```
diff --git a/synapse/federation/transport/server/_base.py b/synapse/federation/transport/server/_base.py
index 15d7740c2e86..da1fbf8b6361 100644
--- a/synapse/federation/transport/server/_base.py
+++ b/synapse/federation/transport/server/_base.py
@@ -114,11 +114,11 @@ async def authenticate_request(
         # alive
         retry_timings = await self.store.get_destination_retry_timings(origin)
         if retry_timings and retry_timings.retry_last_ts:
-            run_in_background(self.reset_retry_timings, origin)
+            run_in_background(self._reset_retry_timings, origin)
 
         return origin
 
-    async def reset_retry_timings(self, origin: str) -> None:
+    async def _reset_retry_timings(self, origin: str) -> None:
         try:
             logger.info("Marking origin %r as up", origin)
             await self.store.set_destination_retry_timings(origin, None, 0, 0)
diff --git a/synapse/rest/admin/federation.py b/synapse/rest/admin/federation.py
index 3a3a39244dc6..ddb4f53441cc 100644
--- a/synapse/rest/admin/federation.py
+++ b/synapse/rest/admin/federation.py
@@ -16,7 +16,6 @@
 from typing import TYPE_CHECKING, Tuple
 
 from synapse.api.errors import Codes, NotFoundError, SynapseError
-from synapse.federation.transport.server._base import Authenticator
 from synapse.http.servlet import RestServlet, parse_integer, parse_string
 from synapse.http.site import SynapseRequest
 from synapse.rest.admin._base import admin_patterns, assert_requester_is_admin
@@ -166,7 +165,7 @@ class DestinationResetConnectionRestServlet(RestServlet):
     def __init__(self, hs: "HomeServer"):
         self._auth = hs.get_auth()
         self._store = hs.get_datastore()
-        self._authenticator = Authenticator(hs)
+        self._notifier = hs.get_notifier()
 
     async def on_POST(
         self, request: SynapseRequest, destination: str
@@ -183,6 +182,8 @@ async def on_POST(
                 "The retry timing does not need to be reset for this destination.",
             )
 
-        await self._authenticator.reset_retry_timings(destination)
+        # reset timings and wake up, this is based on `Authenticator._reset_retry_timings`
+        await self._store.set_destination_retry_timings(destination, None, 0, 0)
+        self._notifier.notify_remote_server_up(destination)
 
         return HTTPStatus.OK, {}

From 25d16a1c214289857bf8d0af020ba86daf67e6a1 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Wed, 5 Jan 2022 19:19:56 +0100
Subject: [PATCH 07/14] uses `Authenticator._reset_retry_timings` and fix
 import loop

---
 synapse/federation/transport/server/_base.py | 13 ++++++++-----
 synapse/rest/admin/federation.py             |  8 ++++----
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/synapse/federation/transport/server/_base.py b/synapse/federation/transport/server/_base.py
index da1fbf8b6361..e9db4412d7c9 100644
--- a/synapse/federation/transport/server/_base.py
+++ b/synapse/federation/transport/server/_base.py
@@ -15,6 +15,7 @@
 import functools
 import logging
 import re
+import typing
 from typing import Any, Awaitable, Callable, Optional, Tuple, cast
 
 from synapse.api.errors import Codes, FederationDeniedError, SynapseError
@@ -29,11 +30,13 @@
     start_active_span_follows_from,
     whitelisted_homeserver,
 )
-from synapse.server import HomeServer
 from synapse.types import JsonDict
 from synapse.util.ratelimitutils import FederationRateLimiter
 from synapse.util.stringutils import parse_and_validate_server_name
 
+if typing.TYPE_CHECKING:
+    from synapse.server import HomeServer
+
 logger = logging.getLogger(__name__)
 
 
@@ -46,7 +49,7 @@ class NoAuthenticationError(AuthenticationError):
 
 
 class Authenticator:
-    def __init__(self, hs: HomeServer):
+    def __init__(self, hs: "HomeServer"):
         self._clock = hs.get_clock()
         self.keyring = hs.get_keyring()
         self.server_name = hs.hostname
@@ -114,11 +117,11 @@ async def authenticate_request(
         # alive
         retry_timings = await self.store.get_destination_retry_timings(origin)
         if retry_timings and retry_timings.retry_last_ts:
-            run_in_background(self._reset_retry_timings, origin)
+            run_in_background(self.reset_retry_timings, origin)
 
         return origin
 
-    async def _reset_retry_timings(self, origin: str) -> None:
+    async def reset_retry_timings(self, origin: str) -> None:
         try:
             logger.info("Marking origin %r as up", origin)
             await self.store.set_destination_retry_timings(origin, None, 0, 0)
@@ -227,7 +230,7 @@ class BaseFederationServlet:
 
     def __init__(
         self,
-        hs: HomeServer,
+        hs: "HomeServer",
         authenticator: Authenticator,
         ratelimiter: FederationRateLimiter,
         server_name: str,
diff --git a/synapse/rest/admin/federation.py b/synapse/rest/admin/federation.py
index ddb4f53441cc..a237e6c12a7a 100644
--- a/synapse/rest/admin/federation.py
+++ b/synapse/rest/admin/federation.py
@@ -16,6 +16,7 @@
 from typing import TYPE_CHECKING, Tuple
 
 from synapse.api.errors import Codes, NotFoundError, SynapseError
+from synapse.federation.transport.server._base import Authenticator
 from synapse.http.servlet import RestServlet, parse_integer, parse_string
 from synapse.http.site import SynapseRequest
 from synapse.rest.admin._base import admin_patterns, assert_requester_is_admin
@@ -165,7 +166,7 @@ class DestinationResetConnectionRestServlet(RestServlet):
     def __init__(self, hs: "HomeServer"):
         self._auth = hs.get_auth()
         self._store = hs.get_datastore()
-        self._notifier = hs.get_notifier()
+        self._authenticator = Authenticator(hs)
 
     async def on_POST(
         self, request: SynapseRequest, destination: str
@@ -182,8 +183,7 @@ async def on_POST(
                 "The retry timing does not need to be reset for this destination.",
             )
 
-        # reset timings and wake up, this is based on `Authenticator._reset_retry_timings`
-        await self._store.set_destination_retry_timings(destination, None, 0, 0)
-        self._notifier.notify_remote_server_up(destination)
+        # reset timings and wake up
+        await self._authenticator.reset_retry_timings(destination)
 
         return HTTPStatus.OK, {}

From 76858e18b12e147e0fd133844664584842b42125 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Wed, 5 Jan 2022 20:41:16 +0100
Subject: [PATCH 08/14] try to fix more imports

---
 synapse/federation/transport/server/__init__.py   | 15 +++++++++------
 synapse/federation/transport/server/federation.py | 13 ++++++++-----
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/synapse/federation/transport/server/__init__.py b/synapse/federation/transport/server/__init__.py
index 77b936361a4a..eb29eee1ce40 100644
--- a/synapse/federation/transport/server/__init__.py
+++ b/synapse/federation/transport/server/__init__.py
@@ -13,6 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import logging
+import typing
 from typing import Dict, Iterable, List, Optional, Tuple, Type
 
 from typing_extensions import Literal
@@ -36,17 +37,19 @@
     parse_integer_from_args,
     parse_string_from_args,
 )
-from synapse.server import HomeServer
 from synapse.types import JsonDict, ThirdPartyInstanceID
 from synapse.util.ratelimitutils import FederationRateLimiter
 
+if typing.TYPE_CHECKING:
+    from synapse.server import HomeServer
+
 logger = logging.getLogger(__name__)
 
 
 class TransportLayerServer(JsonResource):
     """Handles incoming federation HTTP requests"""
 
-    def __init__(self, hs: HomeServer, servlet_groups: Optional[List[str]] = None):
+    def __init__(self, hs: "HomeServer", servlet_groups: Optional[List[str]] = None):
         """Initialize the TransportLayerServer
 
         Will by default register all servlets. For custom behaviour, pass in
@@ -113,7 +116,7 @@ class PublicRoomList(BaseFederationServlet):
 
     def __init__(
         self,
-        hs: HomeServer,
+        hs: "HomeServer",
         authenticator: Authenticator,
         ratelimiter: FederationRateLimiter,
         server_name: str,
@@ -203,7 +206,7 @@ class FederationGroupsRenewAttestaionServlet(BaseFederationServlet):
 
     def __init__(
         self,
-        hs: HomeServer,
+        hs: "HomeServer",
         authenticator: Authenticator,
         ratelimiter: FederationRateLimiter,
         server_name: str,
@@ -251,7 +254,7 @@ class OpenIdUserInfo(BaseFederationServlet):
 
     def __init__(
         self,
-        hs: HomeServer,
+        hs: "HomeServer",
         authenticator: Authenticator,
         ratelimiter: FederationRateLimiter,
         server_name: str,
@@ -297,7 +300,7 @@ async def on_GET(
 
 
 def register_servlets(
-    hs: HomeServer,
+    hs: "HomeServer",
     resource: HttpServer,
     authenticator: Authenticator,
     ratelimiter: FederationRateLimiter,
diff --git a/synapse/federation/transport/server/federation.py b/synapse/federation/transport/server/federation.py
index 77bfd88ad052..0327a1dda8ab 100644
--- a/synapse/federation/transport/server/federation.py
+++ b/synapse/federation/transport/server/federation.py
@@ -12,6 +12,7 @@
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 import logging
+import typing
 from typing import Dict, List, Mapping, Optional, Sequence, Tuple, Type, Union
 
 from typing_extensions import Literal
@@ -30,11 +31,13 @@
     parse_string_from_args,
     parse_strings_from_args,
 )
-from synapse.server import HomeServer
 from synapse.types import JsonDict
 from synapse.util.ratelimitutils import FederationRateLimiter
 from synapse.util.versionstring import get_version_string
 
+if typing.TYPE_CHECKING:
+    from synapse.server import HomeServer
+
 logger = logging.getLogger(__name__)
 
 
@@ -46,7 +49,7 @@ class BaseFederationServerServlet(BaseFederationServlet):
 
     def __init__(
         self,
-        hs: HomeServer,
+        hs: "HomeServer",
         authenticator: Authenticator,
         ratelimiter: FederationRateLimiter,
         server_name: str,
@@ -581,7 +584,7 @@ class FederationSpaceSummaryServlet(BaseFederationServlet):
 
     def __init__(
         self,
-        hs: HomeServer,
+        hs: "HomeServer",
         authenticator: Authenticator,
         ratelimiter: FederationRateLimiter,
         server_name: str,
@@ -655,7 +658,7 @@ class FederationRoomHierarchyServlet(BaseFederationServlet):
 
     def __init__(
         self,
-        hs: HomeServer,
+        hs: "HomeServer",
         authenticator: Authenticator,
         ratelimiter: FederationRateLimiter,
         server_name: str,
@@ -691,7 +694,7 @@ class RoomComplexityServlet(BaseFederationServlet):
 
     def __init__(
         self,
-        hs: HomeServer,
+        hs: "HomeServer",
         authenticator: Authenticator,
         ratelimiter: FederationRateLimiter,
         server_name: str,

From 58998c4505d94fee7e76f561b1b04967013b6657 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Wed, 5 Jan 2022 21:16:04 +0100
Subject: [PATCH 09/14] try to fix more imports - part 2

---
 synapse/federation/transport/server/groups_local.py  | 7 +++++--
 synapse/federation/transport/server/groups_server.py | 7 +++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/synapse/federation/transport/server/groups_local.py b/synapse/federation/transport/server/groups_local.py
index a12cd18d5806..1b8d9ebbcf02 100644
--- a/synapse/federation/transport/server/groups_local.py
+++ b/synapse/federation/transport/server/groups_local.py
@@ -11,6 +11,7 @@
 #  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 typing
 from typing import Dict, List, Tuple, Type
 
 from synapse.api.errors import SynapseError
@@ -19,10 +20,12 @@
     BaseFederationServlet,
 )
 from synapse.handlers.groups_local import GroupsLocalHandler
-from synapse.server import HomeServer
 from synapse.types import JsonDict, get_domain_from_id
 from synapse.util.ratelimitutils import FederationRateLimiter
 
+if typing.TYPE_CHECKING:
+    from synapse.server import HomeServer
+
 
 class BaseGroupsLocalServlet(BaseFederationServlet):
     """Abstract base class for federation servlet classes which provides a groups local handler.
@@ -32,7 +35,7 @@ class BaseGroupsLocalServlet(BaseFederationServlet):
 
     def __init__(
         self,
-        hs: HomeServer,
+        hs: "HomeServer",
         authenticator: Authenticator,
         ratelimiter: FederationRateLimiter,
         server_name: str,
diff --git a/synapse/federation/transport/server/groups_server.py b/synapse/federation/transport/server/groups_server.py
index b30e92a5eb74..a1fba0587bd1 100644
--- a/synapse/federation/transport/server/groups_server.py
+++ b/synapse/federation/transport/server/groups_server.py
@@ -11,6 +11,7 @@
 #  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 typing
 from typing import Dict, List, Tuple, Type
 
 from typing_extensions import Literal
@@ -22,10 +23,12 @@
     BaseFederationServlet,
 )
 from synapse.http.servlet import parse_string_from_args
-from synapse.server import HomeServer
 from synapse.types import JsonDict, get_domain_from_id
 from synapse.util.ratelimitutils import FederationRateLimiter
 
+if typing.TYPE_CHECKING:
+    from synapse.server import HomeServer
+
 
 class BaseGroupsServerServlet(BaseFederationServlet):
     """Abstract base class for federation servlet classes which provides a groups server handler.
@@ -35,7 +38,7 @@ class BaseGroupsServerServlet(BaseFederationServlet):
 
     def __init__(
         self,
-        hs: HomeServer,
+        hs: "HomeServer",
         authenticator: Authenticator,
         ratelimiter: FederationRateLimiter,
         server_name: str,

From 3a4eae949aea068d09ecfcc9d0a35dff41ac14e2 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Wed, 5 Jan 2022 21:30:00 +0100
Subject: [PATCH 10/14] fix mistake in merge

---
 synapse/rest/admin/federation.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/synapse/rest/admin/federation.py b/synapse/rest/admin/federation.py
index 14b8b00ae7b6..484420553a64 100644
--- a/synapse/rest/admin/federation.py
+++ b/synapse/rest/admin/federation.py
@@ -173,7 +173,7 @@ async def on_POST(
     ) -> Tuple[int, JsonDict]:
         await assert_requester_is_admin(self._auth, request)
 
-        if not await self._store.is_destination(destination):
+        if not await self._store.is_destination_known(destination):
             raise NotFoundError("Unknown destination")
 
         retry_timings = await self._store.get_destination_retry_timings(destination)

From e081945091d2773ea62e86b7c4f122bcbaa90ad4 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Mon, 24 Jan 2022 11:55:37 +0100
Subject: [PATCH 11/14] update import `TYPE_CHECKING`

---
 synapse/federation/transport/server/__init__.py     |  3 +--
 synapse/federation/transport/server/_base.py        |  3 +--
 synapse/federation/transport/server/federation.py   | 13 +++++++++++--
 synapse/federation/transport/server/groups_local.py |  3 +--
 .../federation/transport/server/groups_server.py    |  3 +--
 5 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/synapse/federation/transport/server/__init__.py b/synapse/federation/transport/server/__init__.py
index eb29eee1ce40..477dc2ce3b64 100644
--- a/synapse/federation/transport/server/__init__.py
+++ b/synapse/federation/transport/server/__init__.py
@@ -13,8 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import logging
-import typing
-from typing import Dict, Iterable, List, Optional, Tuple, Type
+from typing import TYPE_CHECKING, Dict, Iterable, List, Optional, Tuple, Type
 
 from typing_extensions import Literal
 
diff --git a/synapse/federation/transport/server/_base.py b/synapse/federation/transport/server/_base.py
index e9db4412d7c9..a86afa76841a 100644
--- a/synapse/federation/transport/server/_base.py
+++ b/synapse/federation/transport/server/_base.py
@@ -15,8 +15,7 @@
 import functools
 import logging
 import re
-import typing
-from typing import Any, Awaitable, Callable, Optional, Tuple, cast
+from typing import TYPE_CHECKING, Any, Awaitable, Callable, Optional, Tuple, cast
 
 from synapse.api.errors import Codes, FederationDeniedError, SynapseError
 from synapse.api.urls import FEDERATION_V1_PREFIX
diff --git a/synapse/federation/transport/server/federation.py b/synapse/federation/transport/server/federation.py
index 08b819f228ac..52967cd3edcd 100644
--- a/synapse/federation/transport/server/federation.py
+++ b/synapse/federation/transport/server/federation.py
@@ -12,8 +12,17 @@
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 import logging
-import typing
-from typing import Dict, List, Mapping, Optional, Sequence, Tuple, Type, Union
+from typing import (
+    TYPE_CHECKING,
+    Dict,
+    List,
+    Mapping,
+    Optional,
+    Sequence,
+    Tuple,
+    Type,
+    Union,
+)
 
 from typing_extensions import Literal
 
diff --git a/synapse/federation/transport/server/groups_local.py b/synapse/federation/transport/server/groups_local.py
index 1b8d9ebbcf02..aa88c9853eb3 100644
--- a/synapse/federation/transport/server/groups_local.py
+++ b/synapse/federation/transport/server/groups_local.py
@@ -11,8 +11,7 @@
 #  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 typing
-from typing import Dict, List, Tuple, Type
+from typing import TYPE_CHECKING, Dict, List, Tuple, Type
 
 from synapse.api.errors import SynapseError
 from synapse.federation.transport.server._base import (
diff --git a/synapse/federation/transport/server/groups_server.py b/synapse/federation/transport/server/groups_server.py
index a1fba0587bd1..7de369654720 100644
--- a/synapse/federation/transport/server/groups_server.py
+++ b/synapse/federation/transport/server/groups_server.py
@@ -11,8 +11,7 @@
 #  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 typing
-from typing import Dict, List, Tuple, Type
+from typing import TYPE_CHECKING, Dict, List, Tuple, Type
 
 from typing_extensions import Literal
 

From f0cfa0d6f28f9ad5844c4e84af3388df4111f346 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Mon, 24 Jan 2022 12:06:07 +0100
Subject: [PATCH 12/14] update doc

---
 docs/usage/administration/admin_api/federation.md | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/docs/usage/administration/admin_api/federation.md b/docs/usage/administration/admin_api/federation.md
index 66d6f3c254a3..09ef5256cc68 100644
--- a/docs/usage/administration/admin_api/federation.md
+++ b/docs/usage/administration/admin_api/federation.md
@@ -121,6 +121,14 @@ The response fields are the same like in the `destinations` array in
 
 ## Reset connection timeout
 
+Synapse makes federation requests to other homeservers. If a federation request fails,
+Synapse will marks the destination homeserver as offline, preventing any future requests
+to that server for a "cooldown" period. This period grows over time if the server
+continues to fail its responses
+([exponential backoff](https://en.wikipedia.org/wiki/Exponential_backoff)).
+
+Admins can cancel the cooldown period with this API.
+
 This API resets the retry timing for a specific remote server and tries to connect
 the remote server again. It does not wait for the next `retry_interval`.
 The connection must have previously run into an error and `retry_last_ts`

From 3f842bf72d4a93480db91498fc2f57cb5a126443 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Mon, 24 Jan 2022 12:07:41 +0100
Subject: [PATCH 13/14] lint

---
 synapse/federation/transport/server/__init__.py      | 2 +-
 synapse/federation/transport/server/_base.py         | 2 +-
 synapse/federation/transport/server/federation.py    | 2 +-
 synapse/federation/transport/server/groups_local.py  | 2 +-
 synapse/federation/transport/server/groups_server.py | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/synapse/federation/transport/server/__init__.py b/synapse/federation/transport/server/__init__.py
index 477dc2ce3b64..db4fe2c79803 100644
--- a/synapse/federation/transport/server/__init__.py
+++ b/synapse/federation/transport/server/__init__.py
@@ -39,7 +39,7 @@
 from synapse.types import JsonDict, ThirdPartyInstanceID
 from synapse.util.ratelimitutils import FederationRateLimiter
 
-if typing.TYPE_CHECKING:
+if TYPE_CHECKING:
     from synapse.server import HomeServer
 
 logger = logging.getLogger(__name__)
diff --git a/synapse/federation/transport/server/_base.py b/synapse/federation/transport/server/_base.py
index a86afa76841a..2ca7c0583501 100644
--- a/synapse/federation/transport/server/_base.py
+++ b/synapse/federation/transport/server/_base.py
@@ -33,7 +33,7 @@
 from synapse.util.ratelimitutils import FederationRateLimiter
 from synapse.util.stringutils import parse_and_validate_server_name
 
-if typing.TYPE_CHECKING:
+if TYPE_CHECKING:
     from synapse.server import HomeServer
 
 logger = logging.getLogger(__name__)
diff --git a/synapse/federation/transport/server/federation.py b/synapse/federation/transport/server/federation.py
index 52967cd3edcd..9c1ad5851f69 100644
--- a/synapse/federation/transport/server/federation.py
+++ b/synapse/federation/transport/server/federation.py
@@ -44,7 +44,7 @@
 from synapse.util.ratelimitutils import FederationRateLimiter
 from synapse.util.versionstring import get_version_string
 
-if typing.TYPE_CHECKING:
+if TYPE_CHECKING:
     from synapse.server import HomeServer
 
 logger = logging.getLogger(__name__)
diff --git a/synapse/federation/transport/server/groups_local.py b/synapse/federation/transport/server/groups_local.py
index aa88c9853eb3..496472e1dcd8 100644
--- a/synapse/federation/transport/server/groups_local.py
+++ b/synapse/federation/transport/server/groups_local.py
@@ -22,7 +22,7 @@
 from synapse.types import JsonDict, get_domain_from_id
 from synapse.util.ratelimitutils import FederationRateLimiter
 
-if typing.TYPE_CHECKING:
+if TYPE_CHECKING:
     from synapse.server import HomeServer
 
 
diff --git a/synapse/federation/transport/server/groups_server.py b/synapse/federation/transport/server/groups_server.py
index 7de369654720..851b50152ec5 100644
--- a/synapse/federation/transport/server/groups_server.py
+++ b/synapse/federation/transport/server/groups_server.py
@@ -25,7 +25,7 @@
 from synapse.types import JsonDict, get_domain_from_id
 from synapse.util.ratelimitutils import FederationRateLimiter
 
-if typing.TYPE_CHECKING:
+if TYPE_CHECKING:
     from synapse.server import HomeServer
 
 

From 1ef60fce1c38852dad173b0f035f1ab02d69944a Mon Sep 17 00:00:00 2001
From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com>
Date: Mon, 24 Jan 2022 20:09:15 +0100
Subject: [PATCH 14/14] Apply suggestions from code review

Co-authored-by: David Robertson <david.m.robertson1@gmail.com>
---
 docs/usage/administration/admin_api/federation.md | 4 ++--
 synapse/rest/admin/federation.py                  | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/usage/administration/admin_api/federation.md b/docs/usage/administration/admin_api/federation.md
index 09ef5256cc68..5e609561a64d 100644
--- a/docs/usage/administration/admin_api/federation.md
+++ b/docs/usage/administration/admin_api/federation.md
@@ -122,14 +122,14 @@ The response fields are the same like in the `destinations` array in
 ## Reset connection timeout
 
 Synapse makes federation requests to other homeservers. If a federation request fails,
-Synapse will marks the destination homeserver as offline, preventing any future requests
+Synapse will mark the destination homeserver as offline, preventing any future requests
 to that server for a "cooldown" period. This period grows over time if the server
 continues to fail its responses
 ([exponential backoff](https://en.wikipedia.org/wiki/Exponential_backoff)).
 
 Admins can cancel the cooldown period with this API.
 
-This API resets the retry timing for a specific remote server and tries to connect
+This API resets the retry timing for a specific remote server and tries to connect to
 the remote server again. It does not wait for the next `retry_interval`.
 The connection must have previously run into an error and `retry_last_ts`
 ([Destination Details API](#destination-details-api)) must not be equal to `0`.
diff --git a/synapse/rest/admin/federation.py b/synapse/rest/admin/federation.py
index 484420553a64..0f33f9e4da88 100644
--- a/synapse/rest/admin/federation.py
+++ b/synapse/rest/admin/federation.py
@@ -16,7 +16,7 @@
 from typing import TYPE_CHECKING, Tuple
 
 from synapse.api.errors import Codes, NotFoundError, SynapseError
-from synapse.federation.transport.server._base import Authenticator
+from synapse.federation.transport.server import Authenticator
 from synapse.http.servlet import RestServlet, parse_integer, parse_string
 from synapse.http.site import SynapseRequest
 from synapse.rest.admin._base import admin_patterns, assert_requester_is_admin