From a0cd8ae8cbe14d2821cbe8fd6b011c4ddc729344 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 4 Jun 2021 10:47:58 +0100 Subject: [PATCH 1/6] Don't try and backfill the same room in parallel. (#10116) If backfilling is slow then the client may time out and retry, causing Synapse to start a new `/backfill` before the existing backfill has finished, duplicating work. --- changelog.d/10116.bugfix | 1 + synapse/handlers/federation.py | 8 ++++++++ 2 files changed, 9 insertions(+) create mode 100644 changelog.d/10116.bugfix diff --git a/changelog.d/10116.bugfix b/changelog.d/10116.bugfix new file mode 100644 index 0000000000..90ef707559 --- /dev/null +++ b/changelog.d/10116.bugfix @@ -0,0 +1 @@ +Fix bug where the server would attempt to fetch the same history in the room from a remote server multiple times in parallel. diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index f3f97db2fa..b802822baa 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -178,6 +178,8 @@ class FederationHandler(BaseHandler): self.room_queues = {} # type: Dict[str, List[Tuple[EventBase, str]]] self._room_pdu_linearizer = Linearizer("fed_room_pdu") + self._room_backfill = Linearizer("room_backfill") + self.third_party_event_rules = hs.get_third_party_event_rules() self._ephemeral_messages_enabled = hs.config.enable_ephemeral_messages @@ -1041,6 +1043,12 @@ class FederationHandler(BaseHandler): return. This is used as part of the heuristic to decide if we should back paginate. """ + with (await self._room_backfill.queue(room_id)): + return await self._maybe_backfill_inner(room_id, current_depth, limit) + + async def _maybe_backfill_inner( + self, room_id: str, current_depth: int, limit: int + ) -> bool: extremities = await self.store.get_oldest_events_with_depth_in_room(room_id) if not extremities: From fa1db8f1567471e6cb29c0d6c0b740fcb79ea202 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Mon, 7 Jun 2021 09:19:06 +0100 Subject: [PATCH 2/6] Delete completes to-device messages earlier in /sync (#10124) I hope this will improve https://github.com/matrix-org/synapse/issues/9564. --- changelog.d/10124.misc | 1 + synapse/handlers/sync.py | 21 +++++++++++---------- 2 files changed, 12 insertions(+), 10 deletions(-) create mode 100644 changelog.d/10124.misc diff --git a/changelog.d/10124.misc b/changelog.d/10124.misc new file mode 100644 index 0000000000..c06593238d --- /dev/null +++ b/changelog.d/10124.misc @@ -0,0 +1 @@ +Work to improve the responsiveness of `/sync` requests. diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 069ffc76f7..b1c58ffdc8 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -315,6 +315,17 @@ class SyncHandler: if context: context.tag = sync_type + # if we have a since token, delete any to-device messages before that token + # (since we now know that the device has received them) + if since_token is not None: + since_stream_id = since_token.to_device_key + deleted = await self.store.delete_messages_for_device( + sync_config.user.to_string(), sync_config.device_id, since_stream_id + ) + logger.debug( + "Deleted %d to-device messages up to %d", deleted, since_stream_id + ) + if timeout == 0 or since_token is None or full_state: # we are going to return immediately, so don't bother calling # notifier.wait_for_events. @@ -1230,16 +1241,6 @@ class SyncHandler: since_stream_id = int(sync_result_builder.since_token.to_device_key) if since_stream_id != int(now_token.to_device_key): - # We only delete messages when a new message comes in, but that's - # fine so long as we delete them at some point. - - deleted = await self.store.delete_messages_for_device( - user_id, device_id, since_stream_id - ) - logger.debug( - "Deleted %d to-device messages up to %d", deleted, since_stream_id - ) - messages, stream_id = await self.store.get_new_messages_for_device( user_id, device_id, since_stream_id, now_token.to_device_key ) From d558292548178dde785462bbca7f84c06c1e9eda Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Mon, 7 Jun 2021 16:12:34 +0200 Subject: [PATCH 3/6] Add missing type hints to the admin API servlets (#10105) --- changelog.d/10105.misc | 1 + synapse/rest/admin/__init__.py | 45 ++++++++++++++++++---------------- synapse/rest/admin/_base.py | 3 ++- synapse/rest/admin/groups.py | 12 +++++++-- synapse/rest/admin/media.py | 12 ++++----- synapse/rest/admin/users.py | 15 ++++-------- 6 files changed, 48 insertions(+), 40 deletions(-) create mode 100644 changelog.d/10105.misc diff --git a/changelog.d/10105.misc b/changelog.d/10105.misc new file mode 100644 index 0000000000..244a893d3e --- /dev/null +++ b/changelog.d/10105.misc @@ -0,0 +1 @@ +Add missing type hints to the admin API servlets. \ No newline at end of file diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 9cb9a9f6aa..abf749b001 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -17,11 +17,13 @@ import logging import platform +from typing import TYPE_CHECKING, Optional, Tuple import synapse from synapse.api.errors import Codes, NotFoundError, SynapseError -from synapse.http.server import JsonResource +from synapse.http.server import HttpServer, JsonResource from synapse.http.servlet import RestServlet, parse_json_object_from_request +from synapse.http.site import SynapseRequest from synapse.rest.admin._base import admin_patterns, assert_requester_is_admin from synapse.rest.admin.devices import ( DeleteDevicesRestServlet, @@ -66,22 +68,25 @@ from synapse.rest.admin.users import ( UserTokenRestServlet, WhoisRestServlet, ) -from synapse.types import RoomStreamToken +from synapse.types import JsonDict, RoomStreamToken from synapse.util.versionstring import get_version_string +if TYPE_CHECKING: + from synapse.server import HomeServer + logger = logging.getLogger(__name__) class VersionServlet(RestServlet): PATTERNS = admin_patterns("/server_version$") - def __init__(self, hs): + def __init__(self, hs: "HomeServer"): self.res = { "server_version": get_version_string(synapse), "python_version": platform.python_version(), } - def on_GET(self, request): + def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: return 200, self.res @@ -90,17 +95,14 @@ class PurgeHistoryRestServlet(RestServlet): "/purge_history/(?P[^/]*)(/(?P[^/]+))?" ) - def __init__(self, hs): - """ - - Args: - hs (synapse.server.HomeServer) - """ + def __init__(self, hs: "HomeServer"): self.pagination_handler = hs.get_pagination_handler() self.store = hs.get_datastore() self.auth = hs.get_auth() - async def on_POST(self, request, room_id, event_id): + async def on_POST( + self, request: SynapseRequest, room_id: str, event_id: Optional[str] + ) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) body = parse_json_object_from_request(request, allow_empty_body=True) @@ -119,6 +121,8 @@ class PurgeHistoryRestServlet(RestServlet): if event.room_id != room_id: raise SynapseError(400, "Event is for wrong room.") + # RoomStreamToken expects [int] not Optional[int] + assert event.internal_metadata.stream_ordering is not None room_token = RoomStreamToken( event.depth, event.internal_metadata.stream_ordering ) @@ -173,16 +177,13 @@ class PurgeHistoryRestServlet(RestServlet): class PurgeHistoryStatusRestServlet(RestServlet): PATTERNS = admin_patterns("/purge_history_status/(?P[^/]+)") - def __init__(self, hs): - """ - - Args: - hs (synapse.server.HomeServer) - """ + def __init__(self, hs: "HomeServer"): self.pagination_handler = hs.get_pagination_handler() self.auth = hs.get_auth() - async def on_GET(self, request, purge_id): + async def on_GET( + self, request: SynapseRequest, purge_id: str + ) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) purge_status = self.pagination_handler.get_purge_status(purge_id) @@ -203,12 +204,12 @@ class PurgeHistoryStatusRestServlet(RestServlet): class AdminRestResource(JsonResource): """The REST resource which gets mounted at /_synapse/admin""" - def __init__(self, hs): + def __init__(self, hs: "HomeServer"): JsonResource.__init__(self, hs, canonical_json=False) register_servlets(hs, self) -def register_servlets(hs, http_server): +def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: """ Register all the admin servlets. """ @@ -242,7 +243,9 @@ def register_servlets(hs, http_server): RateLimitRestServlet(hs).register(http_server) -def register_servlets_for_client_rest_resource(hs, http_server): +def register_servlets_for_client_rest_resource( + hs: "HomeServer", http_server: HttpServer +) -> None: """Register only the servlets which need to be exposed on /_matrix/client/xxx""" WhoisRestServlet(hs).register(http_server) PurgeHistoryStatusRestServlet(hs).register(http_server) diff --git a/synapse/rest/admin/_base.py b/synapse/rest/admin/_base.py index f203f6fdc6..d9a2f6ca15 100644 --- a/synapse/rest/admin/_base.py +++ b/synapse/rest/admin/_base.py @@ -13,6 +13,7 @@ # limitations under the License. import re +from typing import Iterable, Pattern from synapse.api.auth import Auth from synapse.api.errors import AuthError @@ -20,7 +21,7 @@ from synapse.http.site import SynapseRequest from synapse.types import UserID -def admin_patterns(path_regex: str, version: str = "v1"): +def admin_patterns(path_regex: str, version: str = "v1") -> Iterable[Pattern]: """Returns the list of patterns for an admin endpoint Args: diff --git a/synapse/rest/admin/groups.py b/synapse/rest/admin/groups.py index 3b3ffde0b6..68a3ba3cb7 100644 --- a/synapse/rest/admin/groups.py +++ b/synapse/rest/admin/groups.py @@ -12,10 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging +from typing import TYPE_CHECKING, Tuple from synapse.api.errors import SynapseError from synapse.http.servlet import RestServlet +from synapse.http.site import SynapseRequest from synapse.rest.admin._base import admin_patterns, assert_user_is_admin +from synapse.types import JsonDict + +if TYPE_CHECKING: + from synapse.server import HomeServer logger = logging.getLogger(__name__) @@ -25,12 +31,14 @@ class DeleteGroupAdminRestServlet(RestServlet): PATTERNS = admin_patterns("/delete_group/(?P[^/]*)") - def __init__(self, hs): + def __init__(self, hs: "HomeServer"): self.group_server = hs.get_groups_server_handler() self.is_mine_id = hs.is_mine_id self.auth = hs.get_auth() - async def on_POST(self, request, group_id): + async def on_POST( + self, request: SynapseRequest, group_id: str + ) -> Tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request) await assert_user_is_admin(self.auth, requester.user) diff --git a/synapse/rest/admin/media.py b/synapse/rest/admin/media.py index b68db2c57c..0a19a333d7 100644 --- a/synapse/rest/admin/media.py +++ b/synapse/rest/admin/media.py @@ -17,6 +17,7 @@ import logging from typing import TYPE_CHECKING, Tuple from synapse.api.errors import AuthError, Codes, NotFoundError, SynapseError +from synapse.http.server import HttpServer from synapse.http.servlet import RestServlet, parse_boolean, parse_integer from synapse.http.site import SynapseRequest from synapse.rest.admin._base import ( @@ -37,12 +38,11 @@ class QuarantineMediaInRoom(RestServlet): this server. """ - PATTERNS = ( - admin_patterns("/room/(?P[^/]+)/media/quarantine") - + + PATTERNS = [ + *admin_patterns("/room/(?P[^/]+)/media/quarantine"), # This path kept around for legacy reasons - admin_patterns("/quarantine_media/(?P[^/]+)") - ) + *admin_patterns("/quarantine_media/(?P[^/]+)"), + ] def __init__(self, hs: "HomeServer"): self.store = hs.get_datastore() @@ -312,7 +312,7 @@ class DeleteMediaByDateSize(RestServlet): return 200, {"deleted_media": deleted_media, "total": total} -def register_servlets_for_media_repo(hs: "HomeServer", http_server): +def register_servlets_for_media_repo(hs: "HomeServer", http_server: HttpServer) -> None: """ Media repo specific APIs. """ diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 8c9d21d3ea..7d75564758 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -478,13 +478,12 @@ class UserRegisterServlet(RestServlet): class WhoisRestServlet(RestServlet): path_regex = "/whois/(?P[^/]*)$" - PATTERNS = ( - admin_patterns(path_regex) - + + PATTERNS = [ + *admin_patterns(path_regex), # URL for spec reason # https://matrix.org/docs/spec/client_server/r0.6.1#get-matrix-client-r0-admin-whois-userid - client_patterns("/admin" + path_regex, v1=True) - ) + *client_patterns("/admin" + path_regex, v1=True), + ] def __init__(self, hs: "HomeServer"): self.hs = hs @@ -553,11 +552,7 @@ class DeactivateAccountRestServlet(RestServlet): class AccountValidityRenewServlet(RestServlet): PATTERNS = admin_patterns("/account_validity/validity$") - def __init__(self, hs): - """ - Args: - hs (synapse.server.HomeServer): server - """ + def __init__(self, hs: "HomeServer"): self.hs = hs self.account_activity_handler = hs.get_account_validity_handler() self.auth = hs.get_auth() From 8942e23a6941dc740f6f703f7e353f273874f104 Mon Sep 17 00:00:00 2001 From: 14mRh4X0r <14mRh4X0r@gmail.com> Date: Mon, 7 Jun 2021 16:42:05 +0200 Subject: [PATCH 4/6] Always update AS last_pos, even on no events (#10107) Fixes #1834. `get_new_events_for_appservice` internally calls `get_events_as_list`, which will filter out any rejected events. If all returned events are filtered out, `_notify_interested_services` will return without updating the last handled stream position. If there are 100 consecutive such events, processing will halt altogether. Breaking the loop is now done by checking whether we're up-to-date with `current_max` in the loop condition, instead of relying on an empty `events` list. Signed-off-by: Willem Mulder <14mRh4X0r@gmail.com> --- changelog.d/10107.bugfix | 1 + synapse/handlers/appservice.py | 25 ++++++++++++------------- tests/handlers/test_appservice.py | 6 ++---- 3 files changed, 15 insertions(+), 17 deletions(-) create mode 100644 changelog.d/10107.bugfix diff --git a/changelog.d/10107.bugfix b/changelog.d/10107.bugfix new file mode 100644 index 0000000000..80030efab2 --- /dev/null +++ b/changelog.d/10107.bugfix @@ -0,0 +1 @@ +Fixed a bug that could cause Synapse to stop notifying application services. Contributed by Willem Mulder. diff --git a/synapse/handlers/appservice.py b/synapse/handlers/appservice.py index 177310f0be..862638cc4f 100644 --- a/synapse/handlers/appservice.py +++ b/synapse/handlers/appservice.py @@ -87,7 +87,8 @@ class ApplicationServicesHandler: self.is_processing = True try: limit = 100 - while True: + upper_bound = -1 + while upper_bound < self.current_max: ( upper_bound, events, @@ -95,9 +96,6 @@ class ApplicationServicesHandler: self.current_max, limit ) - if not events: - break - events_by_room = {} # type: Dict[str, List[EventBase]] for event in events: events_by_room.setdefault(event.room_id, []).append(event) @@ -153,9 +151,6 @@ class ApplicationServicesHandler: await self.store.set_appservice_last_pos(upper_bound) - now = self.clock.time_msec() - ts = await self.store.get_received_ts(events[-1].event_id) - synapse.metrics.event_processing_positions.labels( "appservice_sender" ).set(upper_bound) @@ -168,12 +163,16 @@ class ApplicationServicesHandler: event_processing_loop_counter.labels("appservice_sender").inc() - synapse.metrics.event_processing_lag.labels( - "appservice_sender" - ).set(now - ts) - synapse.metrics.event_processing_last_ts.labels( - "appservice_sender" - ).set(ts) + if events: + now = self.clock.time_msec() + ts = await self.store.get_received_ts(events[-1].event_id) + + synapse.metrics.event_processing_lag.labels( + "appservice_sender" + ).set(now - ts) + synapse.metrics.event_processing_last_ts.labels( + "appservice_sender" + ).set(ts) finally: self.is_processing = False diff --git a/tests/handlers/test_appservice.py b/tests/handlers/test_appservice.py index b037b12a0f..5d6cc2885f 100644 --- a/tests/handlers/test_appservice.py +++ b/tests/handlers/test_appservice.py @@ -57,10 +57,10 @@ class AppServiceHandlerTestCase(unittest.TestCase): sender="@someone:anywhere", type="m.room.message", room_id="!foo:bar" ) self.mock_store.get_new_events_for_appservice.side_effect = [ - make_awaitable((0, [event])), make_awaitable((0, [])), + make_awaitable((1, [event])), ] - self.handler.notify_interested_services(RoomStreamToken(None, 0)) + self.handler.notify_interested_services(RoomStreamToken(None, 1)) self.mock_scheduler.submit_event_for_as.assert_called_once_with( interested_service, event @@ -77,7 +77,6 @@ class AppServiceHandlerTestCase(unittest.TestCase): self.mock_as_api.query_user.return_value = make_awaitable(True) self.mock_store.get_new_events_for_appservice.side_effect = [ make_awaitable((0, [event])), - make_awaitable((0, [])), ] self.handler.notify_interested_services(RoomStreamToken(None, 0)) @@ -95,7 +94,6 @@ class AppServiceHandlerTestCase(unittest.TestCase): self.mock_as_api.query_user.return_value = make_awaitable(True) self.mock_store.get_new_events_for_appservice.side_effect = [ make_awaitable((0, [event])), - make_awaitable((0, [])), ] self.handler.notify_interested_services(RoomStreamToken(None, 0)) From 543e423fce64c14dd136d4021b27a99a3e9fd08b Mon Sep 17 00:00:00 2001 From: Chris Castle Date: Mon, 7 Jun 2021 08:31:39 -0700 Subject: [PATCH 5/6] Fix broken link to README at root of repo (#10132) Signed-off-by: Chris Castle chris@crc.io --- changelog.d/10132.doc | 1 + docker/README.md | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/10132.doc diff --git a/changelog.d/10132.doc b/changelog.d/10132.doc new file mode 100644 index 0000000000..70f538f077 --- /dev/null +++ b/changelog.d/10132.doc @@ -0,0 +1 @@ +Fix broken link in Docker docs. diff --git a/docker/README.md b/docker/README.md index c8d3c4b3da..3f28cdada3 100644 --- a/docker/README.md +++ b/docker/README.md @@ -226,4 +226,4 @@ healthcheck: ## Using jemalloc Jemalloc is embedded in the image and will be used instead of the default allocator. -You can read about jemalloc by reading the Synapse [README](../README.md). +You can read about jemalloc by reading the Synapse [README](../README.rst). From beb251e3eed3f5b93fafea4650ba7146bb19bcf9 Mon Sep 17 00:00:00 2001 From: Rohan Sharma Date: Mon, 7 Jun 2021 21:05:02 +0530 Subject: [PATCH 6/6] Make link in docs use HTTPS (#10130) Fixes #10121 Signed-off-by: Rohan Sharma --- changelog.d/10130.doc | 1 + docs/turn-howto.md | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/10130.doc diff --git a/changelog.d/10130.doc b/changelog.d/10130.doc new file mode 100644 index 0000000000..42ed1f3eac --- /dev/null +++ b/changelog.d/10130.doc @@ -0,0 +1 @@ +Make a link in docs use HTTPS. Contributed by @RhnSharma. diff --git a/docs/turn-howto.md b/docs/turn-howto.md index 41738bbe69..6433446c2a 100644 --- a/docs/turn-howto.md +++ b/docs/turn-howto.md @@ -4,7 +4,7 @@ This document explains how to enable VoIP relaying on your Home Server with TURN. The synapse Matrix Home Server supports integration with TURN server via the -[TURN server REST API](). This +[TURN server REST API](). This allows the Home Server to generate credentials that are valid for use on the TURN server through the use of a secret shared between the Home Server and the TURN server.