From 88764dc3e9bf04f4ea293223c7abec452d879dae Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 15 Oct 2020 18:14:33 +0100 Subject: [PATCH 1/3] Optimise createRoom with multiple invites By not dropping the membership lock between invites, we can stop joins from grabbing the lock when we're half-done and slowing the whole thing down. (Is this a DoS vector? If someone invites thousands of people to a room, they will receive the invite, but won't be able to accept it until the invites have stopped churning? Though we should guard against abusive createRooms anyway.) --- synapse/handlers/room.py | 29 ++++++++++++++++++----------- synapse/handlers/room_member.py | 8 ++++++-- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index d5f7c78edf52..842dc5c6b962 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -762,22 +762,29 @@ async def create_room( ratelimit=False, ) - for invitee in invite_list: + # we avoid dropping the lock between invites, as otherwise joins can + # start coming in and making the createRoom slow. + # + # we also don't need to check the requester's shadow-ban here, as we + # have already done so above (and potentially emptied invite_list). + with (await self.room_member_handler.member_linearizer.queue((room_id,))): content = {} is_direct = config.get("is_direct", None) if is_direct: content["is_direct"] = is_direct - # Note that update_membership with an action of "invite" can raise a - # ShadowBanError, but this was handled above by emptying invite_list. - _, last_stream_id = await self.room_member_handler.update_membership( - requester, - UserID.from_string(invitee), - room_id, - "invite", - ratelimit=False, - content=content, - ) + for invitee in invite_list: + ( + _, + last_stream_id, + ) = await self.room_member_handler.update_membership_locked( + requester, + UserID.from_string(invitee), + room_id, + "invite", + ratelimit=False, + content=content, + ) for invite_3pid in invite_3pid_list: id_server = invite_3pid["id_server"] diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 8feba8c90a39..fef5559ddc5d 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -310,7 +310,7 @@ async def update_membership( key = (room_id,) with (await self.member_linearizer.queue(key)): - result = await self._update_membership( + result = await self.update_membership_locked( requester, target, room_id, @@ -325,7 +325,7 @@ async def update_membership( return result - async def _update_membership( + async def update_membership_locked( self, requester: Requester, target: UserID, @@ -338,6 +338,10 @@ async def _update_membership( content: Optional[dict] = None, require_consent: bool = True, ) -> Tuple[str, int]: + """Helper for update_membership. + + Assumes that the membership linearizer is already held for the room. + """ content_specified = bool(content) if content is None: content = {} From 172fe51d43ffbb567b19caf83ab4ed50fe1444f8 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 15 Oct 2020 18:18:48 +0100 Subject: [PATCH 2/3] changelog --- changelog.d/8559.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/8559.misc diff --git a/changelog.d/8559.misc b/changelog.d/8559.misc new file mode 100644 index 000000000000..50cec6dbf694 --- /dev/null +++ b/changelog.d/8559.misc @@ -0,0 +1 @@ +Optimise `/createRoom` fwith multiple invited users. From db62e76c27042c4043844ba325c26df28ecfb066 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Thu, 15 Oct 2020 18:31:54 +0100 Subject: [PATCH 3/3] Update 8559.misc --- changelog.d/8559.misc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/8559.misc b/changelog.d/8559.misc index 50cec6dbf694..d7bd00964e0a 100644 --- a/changelog.d/8559.misc +++ b/changelog.d/8559.misc @@ -1 +1 @@ -Optimise `/createRoom` fwith multiple invited users. +Optimise `/createRoom` with multiple invited users.