From cda6c453a07db259c66a0da2683cf4f7c3f992dd Mon Sep 17 00:00:00 2001 From: Nick Barrett Date: Fri, 8 Jul 2022 12:53:04 +0100 Subject: [PATCH 01/10] Add optional depth override argument to room member event creation Ths matches the equivalent argument in the create non member events. --- synapse/handlers/room_member.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 04c44b2ccb2d..b879158de439 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -285,6 +285,7 @@ async def _local_membership_update( allow_no_prev_events: bool = False, prev_event_ids: Optional[List[str]] = None, state_event_ids: Optional[List[str]] = None, + depth: Optional[int] = None, txn_id: Optional[str] = None, ratelimit: bool = True, content: Optional[dict] = None, @@ -315,6 +316,9 @@ async def _local_membership_update( prev_events are set so we need to set them ourself via this argument. This should normally be left as None, which will cause the auth_event_ids to be calculated based on the room state at the prev_events. + depth: Override the depth used to order the event in the DAG. + Should normally be set to None, which will cause the depth to be calculated + based on the prev_events. txn_id: ratelimit: @@ -370,6 +374,7 @@ async def _local_membership_update( allow_no_prev_events=allow_no_prev_events, prev_event_ids=prev_event_ids, state_event_ids=state_event_ids, + depth=depth, require_consent=require_consent, outlier=outlier, historical=historical, @@ -466,6 +471,7 @@ async def update_membership( allow_no_prev_events: bool = False, prev_event_ids: Optional[List[str]] = None, state_event_ids: Optional[List[str]] = None, + depth: Optional[int] = None, ) -> Tuple[str, int]: """Update a user's membership in a room. @@ -501,6 +507,9 @@ async def update_membership( prev_events are set so we need to set them ourself via this argument. This should normally be left as None, which will cause the auth_event_ids to be calculated based on the room state at the prev_events. + depth: Override the depth used to order the event in the DAG. + Should normally be set to None, which will cause the depth to be calculated + based on the prev_events. Returns: A tuple of the new event ID and stream ID. @@ -540,6 +549,7 @@ async def update_membership( allow_no_prev_events=allow_no_prev_events, prev_event_ids=prev_event_ids, state_event_ids=state_event_ids, + depth=depth, ) return result @@ -562,6 +572,7 @@ async def update_membership_locked( allow_no_prev_events: bool = False, prev_event_ids: Optional[List[str]] = None, state_event_ids: Optional[List[str]] = None, + depth: Optional[int] = None, ) -> Tuple[str, int]: """Helper for update_membership. @@ -599,6 +610,9 @@ async def update_membership_locked( prev_events are set so we need to set them ourself via this argument. This should normally be left as None, which will cause the auth_event_ids to be calculated based on the room state at the prev_events. + depth: Override the depth used to order the event in the DAG. + Should normally be set to None, which will cause the depth to be calculated + based on the prev_events. Returns: A tuple of the new event ID and stream ID. @@ -732,6 +746,7 @@ async def update_membership_locked( allow_no_prev_events=allow_no_prev_events, prev_event_ids=prev_event_ids, state_event_ids=state_event_ids, + depth=depth, content=content, require_consent=require_consent, outlier=outlier, @@ -967,6 +982,7 @@ async def update_membership_locked( ratelimit=ratelimit, prev_event_ids=latest_event_ids, state_event_ids=state_event_ids, + depth=depth, content=content, require_consent=require_consent, outlier=outlier, From 567af713418538643228fa77b3ed33641af3d990 Mon Sep 17 00:00:00 2001 From: Nick Barrett Date: Fri, 8 Jul 2022 13:21:39 +0100 Subject: [PATCH 02/10] Define depth when creating events in a new room This saves us a query per event sent into the room because we know the depth up front when the room is first created. --- synapse/handlers/room.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index a54f163c0a41..32e8a5279d5f 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -889,7 +889,7 @@ async def create_room( # override any attempt to set room versions via the creation_content creation_content["room_version"] = room_version.identifier - last_stream_id = await self._send_events_for_new_room( + last_stream_id, last_sent_event_id, depth = await self._send_events_for_new_room( requester, room_id, preset_config=preset_config, @@ -905,7 +905,7 @@ async def create_room( if "name" in config: name = config["name"] ( - _, + name_event, last_stream_id, ) = await self.event_creation_handler.create_and_send_nonmember_event( requester, @@ -917,12 +917,16 @@ async def create_room( "content": {"name": name}, }, ratelimit=False, + prev_event_ids=[last_sent_event_id], + depth=depth, ) + last_sent_event_id = name_event.event_id + depth += 1 if "topic" in config: topic = config["topic"] ( - _, + topic_event, last_stream_id, ) = await self.event_creation_handler.create_and_send_nonmember_event( requester, @@ -934,7 +938,11 @@ async def create_room( "content": {"topic": topic}, }, ratelimit=False, + prev_event_ids=[last_sent_event_id], + depth=depth, ) + last_sent_event_id = topic_event.event_id + depth += 1 # we avoid dropping the lock between invites, as otherwise joins can # start coming in and making the createRoom slow. @@ -959,7 +967,10 @@ async def create_room( ratelimit=False, content=content, new_room=True, + prev_event_ids=[last_sent_event_id], + depth=depth, ) + depth += 1 for invite_3pid in invite_3pid_list: id_server = invite_3pid["id_server"] @@ -1019,6 +1030,7 @@ async def _send_events_for_new_room( event_keys = {"room_id": room_id, "sender": creator_id, "state_key": ""} + depth = 1 last_sent_event_id: Optional[str] = None def create(etype: str, content: JsonDict, **kwargs: Any) -> JsonDict: @@ -1031,6 +1043,7 @@ def create(etype: str, content: JsonDict, **kwargs: Any) -> JsonDict: async def send(etype: str, content: JsonDict, **kwargs: Any) -> int: nonlocal last_sent_event_id + nonlocal depth event = create(etype, content, **kwargs) logger.debug("Sending %s in new room", etype) @@ -1047,9 +1060,11 @@ async def send(etype: str, content: JsonDict, **kwargs: Any) -> int: # Note: we don't pass state_event_ids here because this triggers # an additional query per event to look them up from the events table. prev_event_ids=[last_sent_event_id] if last_sent_event_id else [], + depth=depth, ) last_sent_event_id = sent_event.event_id + depth += 1 return last_stream_id @@ -1075,6 +1090,7 @@ async def send(etype: str, content: JsonDict, **kwargs: Any) -> int: content=creator_join_profile, new_room=True, prev_event_ids=[last_sent_event_id], + depth=depth, ) last_sent_event_id = member_event_id @@ -1168,7 +1184,7 @@ async def send(etype: str, content: JsonDict, **kwargs: Any) -> int: content={"algorithm": RoomEncryptionAlgorithms.DEFAULT}, ) - return last_sent_stream_id + return last_sent_stream_id, last_sent_event_id, depth def _generate_room_id(self) -> str: """Generates a random room ID. From 7ab5f1a481ce577d2afe24bb75da3ce7262e48a7 Mon Sep 17 00:00:00 2001 From: Nick Barrett Date: Tue, 12 Jul 2022 11:01:05 +0200 Subject: [PATCH 03/10] Linting --- synapse/handlers/room.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 32e8a5279d5f..310abf03724a 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -889,7 +889,11 @@ async def create_room( # override any attempt to set room versions via the creation_content creation_content["room_version"] = room_version.identifier - last_stream_id, last_sent_event_id, depth = await self._send_events_for_new_room( + ( + last_stream_id, + last_sent_event_id, + depth, + ) = await self._send_events_for_new_room( requester, room_id, preset_config=preset_config, @@ -1016,14 +1020,15 @@ async def _send_events_for_new_room( power_level_content_override: Optional[JsonDict] = None, creator_join_profile: Optional[JsonDict] = None, ratelimit: bool = True, - ) -> int: + ) -> Tuple[int, str, int]: """Sends the initial events into a new room. `power_level_content_override` doesn't apply when initial state has power level state event content. Returns: - The stream_id of the last event persisted. + A tuple containing the stream ID, event ID and depth of the last + event sent to the room. """ creator_id = creator.user.to_string() From 4c846e40dfa0bfcf6bc7461af80b7c4e8121dfc9 Mon Sep 17 00:00:00 2001 From: Nick Barrett Date: Tue, 12 Jul 2022 11:01:13 +0200 Subject: [PATCH 04/10] Update room create tests --- tests/rest/client/test_rooms.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index df7ffbe54538..c921131f035a 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -709,7 +709,7 @@ def test_post_room_no_keys(self) -> None: self.assertEqual(200, channel.code, channel.result) self.assertTrue("room_id" in channel.json_body) assert channel.resource_usage is not None - self.assertEqual(37, channel.resource_usage.db_txn_count) + self.assertEqual(32, channel.resource_usage.db_txn_count) def test_post_room_initial_state(self) -> None: # POST with initial_state config key, expect new room id @@ -722,7 +722,7 @@ def test_post_room_initial_state(self) -> None: self.assertEqual(200, channel.code, channel.result) self.assertTrue("room_id" in channel.json_body) assert channel.resource_usage is not None - self.assertEqual(41, channel.resource_usage.db_txn_count) + self.assertEqual(35, channel.resource_usage.db_txn_count) def test_post_room_visibility_key(self) -> None: # POST with visibility config key, expect new room id From 5f021b6ed02c8fe05a497fdc639d0f84ff1b34cb Mon Sep 17 00:00:00 2001 From: Nick Barrett Date: Tue, 12 Jul 2022 11:31:28 +0200 Subject: [PATCH 05/10] Add changelog file --- changelog.d/13224.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13224.misc diff --git a/changelog.d/13224.misc b/changelog.d/13224.misc new file mode 100644 index 000000000000..41f8693b7425 --- /dev/null +++ b/changelog.d/13224.misc @@ -0,0 +1 @@ +Further reduce queries used sending events when creating new rooms. Contributed by Nick @ Beeper (@fizzadar). From 98414efca28178c87afa71c0e58cc28e27cf82a8 Mon Sep 17 00:00:00 2001 From: Nick Barrett Date: Tue, 12 Jul 2022 18:58:56 +0200 Subject: [PATCH 06/10] Fix missing last sent event ID in membership sends on create room --- synapse/handlers/room.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 310abf03724a..1965331b597b 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -961,7 +961,7 @@ async def create_room( for invitee in invite_list: ( - _, + member_event_id, last_stream_id, ) = await self.room_member_handler.update_membership_locked( requester, @@ -974,6 +974,7 @@ async def create_room( prev_event_ids=[last_sent_event_id], depth=depth, ) + last_sent_event_id = member_event_id depth += 1 for invite_3pid in invite_3pid_list: From 7cedbd0cb9a3c983f4672276c43b3f2a5aef5b16 Mon Sep 17 00:00:00 2001 From: Nick Barrett Date: Tue, 12 Jul 2022 18:59:35 +0200 Subject: [PATCH 07/10] Pass depth & prev event during 3pid membership invites when creating rooms --- synapse/handlers/room.py | 9 ++++++++- synapse/handlers/room_member.py | 24 ++++++++++++++++++------ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 1965331b597b..90d604009883 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -984,7 +984,10 @@ async def create_room( medium = invite_3pid["medium"] # Note that do_3pid_invite can raise a ShadowBanError, but this was # handled above by emptying invite_3pid_list. - last_stream_id = await self.hs.get_room_member_handler().do_3pid_invite( + ( + member_event, + last_stream_id, + ) = await self.hs.get_room_member_handler().do_3pid_invite( room_id, requester.user, medium, @@ -993,7 +996,11 @@ async def create_room( requester, txn_id=None, id_access_token=id_access_token, + prev_event_ids=[last_sent_event_id], + depth=depth, ) + last_sent_event_id = member_event.event_id + depth += 1 result = {"room_id": room_id} diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index b879158de439..34f6c278ddfe 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1338,7 +1338,9 @@ async def do_3pid_invite( requester: Requester, txn_id: Optional[str], id_access_token: Optional[str] = None, - ) -> int: + prev_event_ids: Optional[List[str]] = None, + depth: Optional[int] = None, + ) -> Tuple[EventBase, int]: """Invite a 3PID to a room. Args: @@ -1351,9 +1353,13 @@ async def do_3pid_invite( txn_id: The transaction ID this is part of, or None if this is not part of a transaction. id_access_token: The optional identity server access token. + depth: Override the depth used to order the event in the DAG. + prev_event_ids: The event IDs to use as the prev events + Should normally be set to None, which will cause the depth to be calculated + based on the prev_events. Returns: - The new stream ID. + Tuple of event ID and stream ordering position Raises: ShadowBanError if the requester has been shadow-banned. @@ -1418,7 +1424,7 @@ async def do_3pid_invite( additional_fields=spam_check[1], ) - stream_id = await self._make_and_store_3pid_invite( + event, stream_id = await self._make_and_store_3pid_invite( requester, id_server, medium, @@ -1427,9 +1433,11 @@ async def do_3pid_invite( inviter, txn_id=txn_id, id_access_token=id_access_token, + prev_event_ids=prev_event_ids, + depth=depth, ) - return stream_id + return event, stream_id async def _make_and_store_3pid_invite( self, @@ -1441,7 +1449,9 @@ async def _make_and_store_3pid_invite( user: UserID, txn_id: Optional[str], id_access_token: Optional[str] = None, - ) -> int: + prev_event_ids: Optional[List[str]] = None, + depth: Optional[int] = None, + ) -> Tuple[EventBase, int]: room_state = await self._storage_controllers.state.get_current_state( room_id, StateFilter.from_types( @@ -1534,8 +1544,10 @@ async def _make_and_store_3pid_invite( }, ratelimit=False, txn_id=txn_id, + prev_event_ids=prev_event_ids, + depth=depth, ) - return stream_id + return event, stream_id async def _is_host_in_room(self, current_state_ids: StateMap[str]) -> bool: # Have we just created the room, and is this about to be the very From 8b2e781b61f229f0e40c4c96a37bc2b86a4bc702 Mon Sep 17 00:00:00 2001 From: Nick Barrett Date: Tue, 12 Jul 2022 19:05:35 +0200 Subject: [PATCH 08/10] Update 3pid create room tests --- tests/rest/client/test_rooms.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index c921131f035a..63566ddba93b 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -3283,7 +3283,7 @@ def test_threepid_invite_spamcheck_deprecated(self) -> None: # Mock a few functions to prevent the test from failing due to failing to talk to # a remote IS. We keep the mock for make_and_store_3pid_invite around so we # can check its call_count later on during the test. - make_invite_mock = Mock(return_value=make_awaitable(0)) + make_invite_mock = Mock(return_value=make_awaitable((0, {}))) self.hs.get_room_member_handler()._make_and_store_3pid_invite = make_invite_mock self.hs.get_identity_handler().lookup_3pid = Mock( return_value=make_awaitable(None), @@ -3344,7 +3344,7 @@ def test_threepid_invite_spamcheck(self) -> None: # Mock a few functions to prevent the test from failing due to failing to talk to # a remote IS. We keep the mock for make_and_store_3pid_invite around so we # can check its call_count later on during the test. - make_invite_mock = Mock(return_value=make_awaitable(0)) + make_invite_mock = Mock(return_value=make_awaitable((0, {}))) self.hs.get_room_member_handler()._make_and_store_3pid_invite = make_invite_mock self.hs.get_identity_handler().lookup_3pid = Mock( return_value=make_awaitable(None), From 72d95c8ecbd0b4fc26aad672113f97924dfeba8d Mon Sep 17 00:00:00 2001 From: Nick Barrett Date: Wed, 13 Jul 2022 08:10:29 +0200 Subject: [PATCH 09/10] Fix handling of 3pid invites --- synapse/handlers/room.py | 4 ++-- synapse/handlers/room_member.py | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 90d604009883..978d3ee39f60 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -985,7 +985,7 @@ async def create_room( # Note that do_3pid_invite can raise a ShadowBanError, but this was # handled above by emptying invite_3pid_list. ( - member_event, + member_event_id, last_stream_id, ) = await self.hs.get_room_member_handler().do_3pid_invite( room_id, @@ -999,7 +999,7 @@ async def create_room( prev_event_ids=[last_sent_event_id], depth=depth, ) - last_sent_event_id = member_event.event_id + last_sent_event_id = member_event_id depth += 1 result = {"room_id": room_id} diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 34f6c278ddfe..90e0b2160021 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1340,7 +1340,7 @@ async def do_3pid_invite( id_access_token: Optional[str] = None, prev_event_ids: Optional[List[str]] = None, depth: Optional[int] = None, - ) -> Tuple[EventBase, int]: + ) -> Tuple[str, int]: """Invite a 3PID to a room. Args: @@ -1405,7 +1405,7 @@ async def do_3pid_invite( # We don't check the invite against the spamchecker(s) here (through # user_may_invite) because we'll do it further down the line anyway (in # update_membership_locked). - _, stream_id = await self.update_membership( + event_id, stream_id = await self.update_membership( requester, UserID.from_string(invitee), room_id, "invite", txn_id=txn_id ) else: @@ -1436,8 +1436,9 @@ async def do_3pid_invite( prev_event_ids=prev_event_ids, depth=depth, ) + event_id = event.event_id - return event, stream_id + return event_id, stream_id async def _make_and_store_3pid_invite( self, From 0bedb823f992823911d8ea4a10a193c1d176d086 Mon Sep 17 00:00:00 2001 From: Nick Barrett Date: Wed, 13 Jul 2022 08:31:48 +0200 Subject: [PATCH 10/10] Fixup 3pid room creation unit test --- tests/rest/client/test_rooms.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 63566ddba93b..8ed5272b1667 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -3283,7 +3283,7 @@ def test_threepid_invite_spamcheck_deprecated(self) -> None: # Mock a few functions to prevent the test from failing due to failing to talk to # a remote IS. We keep the mock for make_and_store_3pid_invite around so we # can check its call_count later on during the test. - make_invite_mock = Mock(return_value=make_awaitable((0, {}))) + make_invite_mock = Mock(return_value=make_awaitable((Mock(event_id="abc"), 0))) self.hs.get_room_member_handler()._make_and_store_3pid_invite = make_invite_mock self.hs.get_identity_handler().lookup_3pid = Mock( return_value=make_awaitable(None), @@ -3344,7 +3344,7 @@ def test_threepid_invite_spamcheck(self) -> None: # Mock a few functions to prevent the test from failing due to failing to talk to # a remote IS. We keep the mock for make_and_store_3pid_invite around so we # can check its call_count later on during the test. - make_invite_mock = Mock(return_value=make_awaitable((0, {}))) + make_invite_mock = Mock(return_value=make_awaitable((Mock(event_id="abc"), 0))) self.hs.get_room_member_handler()._make_and_store_3pid_invite = make_invite_mock self.hs.get_identity_handler().lookup_3pid = Mock( return_value=make_awaitable(None),