From e986a7f919d9b3e0c08d7fa3b0f1141c85a1069a Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 23 Sep 2022 18:45:25 +0100 Subject: [PATCH 1/7] Add new columns tracking when we partial-joined --- .../main/delta/73/04partial_join_details.sql | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 synapse/storage/schema/main/delta/73/04partial_join_details.sql diff --git a/synapse/storage/schema/main/delta/73/04partial_join_details.sql b/synapse/storage/schema/main/delta/73/04partial_join_details.sql new file mode 100644 index 000000000000..0d086ef69913 --- /dev/null +++ b/synapse/storage/schema/main/delta/73/04partial_join_details.sql @@ -0,0 +1,23 @@ +/* Copyright 2022 The Matrix.org Foundation C.I.C + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * 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. + */ + +-- To ensure we correctly notify other homeservers about device list changes from our +-- users after a partial join transitions to a full join, we need to know when we began +-- the partial join. For now it's sufficient to know the device_list stream_id at the +-- time of the partial join, and the join event created for us during a partial join. +-- +-- Both columns are nullable without defaults, for backwards compatibility. +ALTER TABLE partial_state_rooms ADD COLUMN device_lists_stream_id BIGINT; +ALTER TABLE partial_state_rooms ADD COLUMN join_event_id TEXT REFERENCES events(event_id); From de0002b1e50c2718e521a3244befe598b66539f5 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 23 Sep 2022 18:45:54 +0100 Subject: [PATCH 2/7] Store when we partial-joined --- synapse/handlers/federation.py | 7 ++++++- synapse/storage/databases/main/room.py | 23 +++++++++++++++++++++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 583d5ecd7749..2d0f8b7ef7ed 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -583,7 +583,12 @@ async def do_invite_join( # Mark the room as having partial state. # The background process is responsible for unmarking this flag, # even if the join fails. - await self.store.store_partial_state_room(room_id, ret.servers_in_room) + await self.store.store_partial_state_room( + room_id, + ret.servers_in_room, + ret.event.event_id, + self.store.get_device_stream_token(), + ) try: max_stream_id = ( diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 5dd116d76653..b25947420459 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1777,28 +1777,47 @@ async def store_partial_state_room( self, room_id: str, servers: Collection[str], + join_event_id: str, + device_lists_stream_id: int, ) -> None: - """Mark the given room as containing events with partial state + """Mark the given room as containing events with partial state. + + We also store additional data that describes _when_ we first partial-joined this + room, which helps us to keep other homeservers in sync when we finally fully + join this room. Args: room_id: the ID of the room servers: other servers known to be in the room + join_event_id: the event ID of the join membership event returned in the + (partial) /send_join response. + device_lists_stream_id: the device_lists stream ID at the time when we first + joined the room. """ await self.db_pool.runInteraction( "store_partial_state_room", self._store_partial_state_room_txn, room_id, servers, + join_event_id, + device_lists_stream_id, ) def _store_partial_state_room_txn( - self, txn: LoggingTransaction, room_id: str, servers: Collection[str] + self, + txn: LoggingTransaction, + room_id: str, + servers: Collection[str], + join_event_id: str, + device_lists_stream_id: int, ) -> None: DatabasePool.simple_insert_txn( txn, table="partial_state_rooms", values={ "room_id": room_id, + "device_lists_stream_id": device_lists_stream_id, + "join_event_id": join_event_id, }, ) DatabasePool.simple_insert_many_txn( From 270da67145628b2fce898c488cb50eb04af4ed44 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 23 Sep 2022 18:50:24 +0100 Subject: [PATCH 3/7] Changelog --- changelog.d/13892.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13892.feature diff --git a/changelog.d/13892.feature b/changelog.d/13892.feature new file mode 100644 index 000000000000..df3f57653632 --- /dev/null +++ b/changelog.d/13892.feature @@ -0,0 +1 @@ +Faster remote room joins: record _when_ we first partial-join to a room. From b5d67c4f5bd551ae6232c13974cd5fdf50342b07 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 23 Sep 2022 20:14:45 +0100 Subject: [PATCH 4/7] Drop the foreign key constraint :( --- synapse/storage/schema/main/delta/73/04partial_join_details.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/schema/main/delta/73/04partial_join_details.sql b/synapse/storage/schema/main/delta/73/04partial_join_details.sql index 0d086ef69913..d9c99f57e379 100644 --- a/synapse/storage/schema/main/delta/73/04partial_join_details.sql +++ b/synapse/storage/schema/main/delta/73/04partial_join_details.sql @@ -20,4 +20,4 @@ -- -- Both columns are nullable without defaults, for backwards compatibility. ALTER TABLE partial_state_rooms ADD COLUMN device_lists_stream_id BIGINT; -ALTER TABLE partial_state_rooms ADD COLUMN join_event_id TEXT REFERENCES events(event_id); +ALTER TABLE partial_state_rooms ADD COLUMN join_event_id TEXT; From 0f419d1e896a18ac595b477844e18fd0e90a2b3a Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 26 Sep 2022 11:52:20 +0100 Subject: [PATCH 5/7] stream id column: NOT NULL DEFAULT 0 --- .../storage/schema/main/delta/73/04partial_join_details.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/storage/schema/main/delta/73/04partial_join_details.sql b/synapse/storage/schema/main/delta/73/04partial_join_details.sql index d9c99f57e379..6dc60b3f7570 100644 --- a/synapse/storage/schema/main/delta/73/04partial_join_details.sql +++ b/synapse/storage/schema/main/delta/73/04partial_join_details.sql @@ -18,6 +18,6 @@ -- the partial join. For now it's sufficient to know the device_list stream_id at the -- time of the partial join, and the join event created for us during a partial join. -- --- Both columns are nullable without defaults, for backwards compatibility. -ALTER TABLE partial_state_rooms ADD COLUMN device_lists_stream_id BIGINT; +-- Both columns are backwards compatible. +ALTER TABLE partial_state_rooms ADD COLUMN device_lists_stream_id BIGINT NOT NULL DEFAULT 0; ALTER TABLE partial_state_rooms ADD COLUMN join_event_id TEXT; From 94ee1e25b2424de4155e5655c628d223f8a99868 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 26 Sep 2022 13:37:09 +0100 Subject: [PATCH 6/7] Add back the foreign key constraint --- synapse/handlers/federation.py | 15 +++++-- synapse/storage/databases/main/room.py | 41 ++++++++++++++++--- .../main/delta/73/04partial_join_details.sql | 2 +- 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 2d0f8b7ef7ed..bec2735ddb2b 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -584,10 +584,9 @@ async def do_invite_join( # The background process is responsible for unmarking this flag, # even if the join fails. await self.store.store_partial_state_room( - room_id, - ret.servers_in_room, - ret.event.event_id, - self.store.get_device_stream_token(), + room_id=room_id, + servers=ret.servers_in_room, + device_lists_stream_id=self.store.get_device_stream_token(), ) try: @@ -613,6 +612,14 @@ async def do_invite_join( room_id, ) raise LimitExceededError(msg=e.msg, errcode=e.errcode, retry_after_ms=0) + else: + # Record the join event id for future use (when we finish the full + # join). We have to do this after persisting the event to keep foreign + # key constraints intact. + if ret.partial_state: + await self.store.write_partial_state_rooms_join_event_id( + room_id, event.event_id + ) finally: # Always kick off the background process that asynchronously fetches # state for the room. diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index b25947420459..0fa66b120437 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1777,7 +1777,6 @@ async def store_partial_state_room( self, room_id: str, servers: Collection[str], - join_event_id: str, device_lists_stream_id: int, ) -> None: """Mark the given room as containing events with partial state. @@ -1786,11 +1785,12 @@ async def store_partial_state_room( room, which helps us to keep other homeservers in sync when we finally fully join this room. + We do not include a `join_event_id` here---we need to wait for the join event + to be persisted first. + Args: room_id: the ID of the room servers: other servers known to be in the room - join_event_id: the event ID of the join membership event returned in the - (partial) /send_join response. device_lists_stream_id: the device_lists stream ID at the time when we first joined the room. """ @@ -1799,7 +1799,6 @@ async def store_partial_state_room( self._store_partial_state_room_txn, room_id, servers, - join_event_id, device_lists_stream_id, ) @@ -1808,7 +1807,6 @@ def _store_partial_state_room_txn( txn: LoggingTransaction, room_id: str, servers: Collection[str], - join_event_id: str, device_lists_stream_id: int, ) -> None: DatabasePool.simple_insert_txn( @@ -1817,7 +1815,8 @@ def _store_partial_state_room_txn( values={ "room_id": room_id, "device_lists_stream_id": device_lists_stream_id, - "join_event_id": join_event_id, + # To be updated later once the join event is persisted. + "join_event_id": None, }, ) DatabasePool.simple_insert_many_txn( @@ -1828,6 +1827,36 @@ def _store_partial_state_room_txn( ) self._invalidate_cache_and_stream(txn, self.is_partial_state_room, (room_id,)) + async def write_partial_state_rooms_join_event_id( + self, + room_id: str, + join_event_id: str, + ) -> None: + """Record the join event which resulted from a partial join. + + We do this separately to `store_partial_state_room` because we need to wait for + the join event to be persisted. Otherwise we violate a foreign key constraint. + """ + await self.db_pool.runInteraction( + "write_partial_state_rooms_join_event_id", + self._store_partial_state_room_txn, + room_id, + join_event_id, + ) + + async def _write_partial_state_rooms_join_event_id( + self, + txn: LoggingTransaction, + room_id: str, + join_event_id: str, + ) -> None: + DatabasePool.simple_update_txn( + txn, + table="partial_state_rooms", + keyvalues={"room_id": room_id}, + updatevalues={"join_event_id": join_event_id}, + ) + async def maybe_store_room_on_outlier_membership( self, room_id: str, room_version: RoomVersion ) -> None: diff --git a/synapse/storage/schema/main/delta/73/04partial_join_details.sql b/synapse/storage/schema/main/delta/73/04partial_join_details.sql index 6dc60b3f7570..5fb2bfe1a23d 100644 --- a/synapse/storage/schema/main/delta/73/04partial_join_details.sql +++ b/synapse/storage/schema/main/delta/73/04partial_join_details.sql @@ -20,4 +20,4 @@ -- -- Both columns are backwards compatible. ALTER TABLE partial_state_rooms ADD COLUMN device_lists_stream_id BIGINT NOT NULL DEFAULT 0; -ALTER TABLE partial_state_rooms ADD COLUMN join_event_id TEXT; +ALTER TABLE partial_state_rooms ADD COLUMN join_event_id TEXT REFERENCES events(event_id); From be843f7042448ef0e7f6acfbf4b1d4865b7296d1 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 27 Sep 2022 12:55:45 +0100 Subject: [PATCH 7/7] Fix a bug where dmr 30.3.1 would type w/o thinking --- synapse/storage/databases/main/room.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 0fa66b120437..064c332fb75c 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1839,12 +1839,12 @@ async def write_partial_state_rooms_join_event_id( """ await self.db_pool.runInteraction( "write_partial_state_rooms_join_event_id", - self._store_partial_state_room_txn, + self._write_partial_state_rooms_join_event_id, room_id, join_event_id, ) - async def _write_partial_state_rooms_join_event_id( + def _write_partial_state_rooms_join_event_id( self, txn: LoggingTransaction, room_id: str,