From 9795556052e5fe269f2164096e191fb941b44cd0 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 26 Aug 2024 14:42:55 -0500 Subject: [PATCH 01/28] Update comment --- synapse/storage/databases/main/events_bg_updates.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index ba0b97ae2a..af0a1ba880 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1624,10 +1624,11 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS "We should have at-least one event in the room (our own join membership event for example) " + "that isn't backfilled (negative `stream_ordering`) if we are joined to the room." ) - # Figure out the latest bump_stamp in the room. This could be `None` for a + # Figure out the latest `bump_stamp` in the room. This could be `None` for a # federated room you just joined where all of events are still `outliers` or # backfilled history. In the Sliding Sync API, we default to the user's - # membership event `stream_ordering` if we don't have a `bump_stamp`. + # membership event `stream_ordering` if we don't have a `bump_stamp` so + # having it as `None` in this table is fine. bump_stamp_event_pos_results = await self.get_last_event_pos_in_room( room_id, event_types=SLIDING_SYNC_DEFAULT_BUMP_EVENT_TYPES ) From addb91485f3475649be8e9c01867d4834e41be07 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 26 Aug 2024 16:11:56 -0500 Subject: [PATCH 02/28] Split test cases --- tests/storage/test_sliding_sync_tables.py | 359 +++++++++++----------- 1 file changed, 187 insertions(+), 172 deletions(-) diff --git a/tests/storage/test_sliding_sync_tables.py b/tests/storage/test_sliding_sync_tables.py index 34f42b6fd4..fb2340b446 100644 --- a/tests/storage/test_sliding_sync_tables.py +++ b/tests/storage/test_sliding_sync_tables.py @@ -81,9 +81,9 @@ class _SlidingSyncMembershipSnapshotResult: forgotten: bool = False -class SlidingSyncPrePopulatedTablesTestCase(HomeserverTestCase): +class SlidingSyncTablesTestCaseBase(HomeserverTestCase): """ - Tests to make sure the + Helpers to deal with testing that the `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` database tables are populated correctly. """ @@ -319,6 +319,14 @@ class SlidingSyncPrePopulatedTablesTestCase(HomeserverTestCase): return persisted_event + +class SlidingSyncTablesTestCase(SlidingSyncTablesTestCaseBase): + """ + Tests to make sure the + `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` database tables are + populated and updated correctly as new events are sent. + """ + def test_joined_room_with_no_info(self) -> None: """ Test joined room that doesn't have a room type, encryption, or name shows up in @@ -2407,6 +2415,183 @@ class SlidingSyncPrePopulatedTablesTestCase(HomeserverTestCase): user2_snapshot, ) + def test_membership_snapshot_forget(self) -> None: + """ + Test forgetting a room will update `sliding_sync_membership_snapshots` + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + user2_id = self.register_user("user2", "pass") + user2_tok = self.login(user2_id, "pass") + + room_id = self.helper.create_room_as(user2_id, tok=user2_tok) + + # User1 joins the room + self.helper.join(room_id, user1_id, tok=user1_tok) + # User1 leaves the room (we have to leave in order to forget the room) + self.helper.leave(room_id, user1_id, tok=user1_tok) + + state_map = self.get_success( + self.storage_controllers.state.get_current_state(room_id) + ) + + # Check on the `sliding_sync_membership_snapshots` table (nothing should be + # forgotten yet) + sliding_sync_membership_snapshots_results = ( + self._get_sliding_sync_membership_snapshots() + ) + self.assertIncludes( + set(sliding_sync_membership_snapshots_results.keys()), + { + (room_id, user1_id), + (room_id, user2_id), + }, + exact=True, + ) + # Holds the info according to the current state when the user joined + user1_snapshot = _SlidingSyncMembershipSnapshotResult( + room_id=room_id, + user_id=user1_id, + sender=user1_id, + membership_event_id=state_map[(EventTypes.Member, user1_id)].event_id, + membership=Membership.LEAVE, + event_stream_ordering=state_map[ + (EventTypes.Member, user1_id) + ].internal_metadata.stream_ordering, + has_known_state=True, + room_type=None, + room_name=None, + is_encrypted=False, + tombstone_successor_room_id=None, + # Room is not forgotten + forgotten=False, + ) + self.assertEqual( + sliding_sync_membership_snapshots_results.get((room_id, user1_id)), + user1_snapshot, + ) + # Holds the info according to the current state when the user joined + user2_snapshot = _SlidingSyncMembershipSnapshotResult( + room_id=room_id, + user_id=user2_id, + sender=user2_id, + membership_event_id=state_map[(EventTypes.Member, user2_id)].event_id, + membership=Membership.JOIN, + event_stream_ordering=state_map[ + (EventTypes.Member, user2_id) + ].internal_metadata.stream_ordering, + has_known_state=True, + room_type=None, + room_name=None, + is_encrypted=False, + tombstone_successor_room_id=None, + ) + self.assertEqual( + sliding_sync_membership_snapshots_results.get((room_id, user2_id)), + user2_snapshot, + ) + + # Forget the room + channel = self.make_request( + "POST", + f"/_matrix/client/r0/rooms/{room_id}/forget", + content={}, + access_token=user1_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + # Check on the `sliding_sync_membership_snapshots` table + sliding_sync_membership_snapshots_results = ( + self._get_sliding_sync_membership_snapshots() + ) + self.assertIncludes( + set(sliding_sync_membership_snapshots_results.keys()), + { + (room_id, user1_id), + (room_id, user2_id), + }, + exact=True, + ) + # Room is now forgotten for user1 + self.assertEqual( + sliding_sync_membership_snapshots_results.get((room_id, user1_id)), + attr.evolve(user1_snapshot, forgotten=True), + ) + # Nothing changed for user2 + self.assertEqual( + sliding_sync_membership_snapshots_results.get((room_id, user2_id)), + user2_snapshot, + ) + + def test_membership_snapshot_missing_forget( + self, + ) -> None: + """ + Test forgetting a room with no existing row in `sliding_sync_membership_snapshots`. + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + user2_id = self.register_user("user2", "pass") + user2_tok = self.login(user2_id, "pass") + + room_id = self.helper.create_room_as(user2_id, tok=user2_tok) + + # User1 joins the room + self.helper.join(room_id, user1_id, tok=user1_tok) + # User1 leaves the room (we have to leave in order to forget the room) + self.helper.leave(room_id, user1_id, tok=user1_tok) + + # Clean-up the `sliding_sync_membership_snapshots` table as if the inserts did not + # happen during event creation. + self.get_success( + self.store.db_pool.simple_delete_many( + table="sliding_sync_membership_snapshots", + column="room_id", + iterable=(room_id,), + keyvalues={}, + desc="sliding_sync_membership_snapshots.test_membership_snapshots_background_update_forgotten_missing", + ) + ) + + # We shouldn't find anything in the table because we just deleted them in + # preparation for the test. + sliding_sync_membership_snapshots_results = ( + self._get_sliding_sync_membership_snapshots() + ) + self.assertIncludes( + set(sliding_sync_membership_snapshots_results.keys()), + set(), + exact=True, + ) + + # Forget the room + channel = self.make_request( + "POST", + f"/_matrix/client/r0/rooms/{room_id}/forget", + content={}, + access_token=user1_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + # It doesn't explode + + # We still shouldn't find anything in the table because nothing has re-created them + sliding_sync_membership_snapshots_results = ( + self._get_sliding_sync_membership_snapshots() + ) + self.assertIncludes( + set(sliding_sync_membership_snapshots_results.keys()), + set(), + exact=True, + ) + + +class SlidingSyncTablesBackgroundUpdatesTestCase(SlidingSyncTablesTestCaseBase): + """ + Test the background updates that populate the `sliding_sync_joined_rooms` and + `sliding_sync_membership_snapshots` tables. + """ + def test_joined_background_update_missing(self) -> None: """ Test that the background update for `sliding_sync_joined_rooms` populates missing rows @@ -3987,173 +4172,3 @@ class SlidingSyncPrePopulatedTablesTestCase(HomeserverTestCase): sliding_sync_membership_snapshots_results.get((room_id, user2_id)), user2_snapshot, ) - - def test_membership_snapshot_forget(self) -> None: - """ - Test forgetting a room will update `sliding_sync_membership_snapshots` - """ - user1_id = self.register_user("user1", "pass") - user1_tok = self.login(user1_id, "pass") - user2_id = self.register_user("user2", "pass") - user2_tok = self.login(user2_id, "pass") - - room_id = self.helper.create_room_as(user2_id, tok=user2_tok) - - # User1 joins the room - self.helper.join(room_id, user1_id, tok=user1_tok) - # User1 leaves the room (we have to leave in order to forget the room) - self.helper.leave(room_id, user1_id, tok=user1_tok) - - state_map = self.get_success( - self.storage_controllers.state.get_current_state(room_id) - ) - - # Check on the `sliding_sync_membership_snapshots` table (nothing should be - # forgotten yet) - sliding_sync_membership_snapshots_results = ( - self._get_sliding_sync_membership_snapshots() - ) - self.assertIncludes( - set(sliding_sync_membership_snapshots_results.keys()), - { - (room_id, user1_id), - (room_id, user2_id), - }, - exact=True, - ) - # Holds the info according to the current state when the user joined - user1_snapshot = _SlidingSyncMembershipSnapshotResult( - room_id=room_id, - user_id=user1_id, - sender=user1_id, - membership_event_id=state_map[(EventTypes.Member, user1_id)].event_id, - membership=Membership.LEAVE, - event_stream_ordering=state_map[ - (EventTypes.Member, user1_id) - ].internal_metadata.stream_ordering, - has_known_state=True, - room_type=None, - room_name=None, - is_encrypted=False, - tombstone_successor_room_id=None, - # Room is not forgotten - forgotten=False, - ) - self.assertEqual( - sliding_sync_membership_snapshots_results.get((room_id, user1_id)), - user1_snapshot, - ) - # Holds the info according to the current state when the user joined - user2_snapshot = _SlidingSyncMembershipSnapshotResult( - room_id=room_id, - user_id=user2_id, - sender=user2_id, - membership_event_id=state_map[(EventTypes.Member, user2_id)].event_id, - membership=Membership.JOIN, - event_stream_ordering=state_map[ - (EventTypes.Member, user2_id) - ].internal_metadata.stream_ordering, - has_known_state=True, - room_type=None, - room_name=None, - is_encrypted=False, - tombstone_successor_room_id=None, - ) - self.assertEqual( - sliding_sync_membership_snapshots_results.get((room_id, user2_id)), - user2_snapshot, - ) - - # Forget the room - channel = self.make_request( - "POST", - f"/_matrix/client/r0/rooms/{room_id}/forget", - content={}, - access_token=user1_tok, - ) - self.assertEqual(channel.code, 200, channel.result) - - # Check on the `sliding_sync_membership_snapshots` table - sliding_sync_membership_snapshots_results = ( - self._get_sliding_sync_membership_snapshots() - ) - self.assertIncludes( - set(sliding_sync_membership_snapshots_results.keys()), - { - (room_id, user1_id), - (room_id, user2_id), - }, - exact=True, - ) - # Room is now forgotten for user1 - self.assertEqual( - sliding_sync_membership_snapshots_results.get((room_id, user1_id)), - attr.evolve(user1_snapshot, forgotten=True), - ) - # Nothing changed for user2 - self.assertEqual( - sliding_sync_membership_snapshots_results.get((room_id, user2_id)), - user2_snapshot, - ) - - def test_membership_snapshot_missing_forget( - self, - ) -> None: - """ - Test forgetting a room with no existing row in `sliding_sync_membership_snapshots`. - """ - user1_id = self.register_user("user1", "pass") - user1_tok = self.login(user1_id, "pass") - user2_id = self.register_user("user2", "pass") - user2_tok = self.login(user2_id, "pass") - - room_id = self.helper.create_room_as(user2_id, tok=user2_tok) - - # User1 joins the room - self.helper.join(room_id, user1_id, tok=user1_tok) - # User1 leaves the room (we have to leave in order to forget the room) - self.helper.leave(room_id, user1_id, tok=user1_tok) - - # Clean-up the `sliding_sync_membership_snapshots` table as if the inserts did not - # happen during event creation. - self.get_success( - self.store.db_pool.simple_delete_many( - table="sliding_sync_membership_snapshots", - column="room_id", - iterable=(room_id,), - keyvalues={}, - desc="sliding_sync_membership_snapshots.test_membership_snapshots_background_update_forgotten_missing", - ) - ) - - # We shouldn't find anything in the table because we just deleted them in - # preparation for the test. - sliding_sync_membership_snapshots_results = ( - self._get_sliding_sync_membership_snapshots() - ) - self.assertIncludes( - set(sliding_sync_membership_snapshots_results.keys()), - set(), - exact=True, - ) - - # Forget the room - channel = self.make_request( - "POST", - f"/_matrix/client/r0/rooms/{room_id}/forget", - content={}, - access_token=user1_tok, - ) - self.assertEqual(channel.code, 200, channel.result) - - # It doesn't explode - - # We still shouldn't find anything in the table because nothing has re-created them - sliding_sync_membership_snapshots_results = ( - self._get_sliding_sync_membership_snapshots() - ) - self.assertIncludes( - set(sliding_sync_membership_snapshots_results.keys()), - set(), - exact=True, - ) From 8bddbe23bda8073ef6ac662e034cb0af488167df Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 26 Aug 2024 16:19:47 -0500 Subject: [PATCH 03/28] Clear out-of-date rows --- synapse/storage/prepare_database.py | 162 +++++++++++++++++++++++++++- 1 file changed, 161 insertions(+), 1 deletion(-) diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index aaffe5ecc9..2b323c0272 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -37,10 +37,15 @@ from typing import ( import attr from synapse.config.homeserver import HomeServerConfig -from synapse.storage.database import LoggingDatabaseConnection, LoggingTransaction +from synapse.storage.database import ( + DatabasePool, + LoggingDatabaseConnection, + LoggingTransaction, +) from synapse.storage.engines import BaseDatabaseEngine, PostgresEngine, Sqlite3Engine from synapse.storage.schema import SCHEMA_COMPAT_VERSION, SCHEMA_VERSION from synapse.storage.types import Cursor +from synapse.util.iterutils import batch_iter logger = logging.getLogger(__name__) @@ -567,6 +572,161 @@ def _upgrade_existing_database( logger.info("Schema now up to date") + # FIXME: This can be removed once we bump `SCHEMA_COMPAT_VERSION` and run the + # foreground update for + # `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` (tracked by + # https://github.com/element-hq/synapse/issues/TODO) + _clear_out_of_date_sliding_sync_tables( + txn=cur, + ) + + +def _clear_out_of_date_sliding_sync_tables( + txn: LoggingTransaction, +) -> None: + """ + Clears out-of-date entries from the + `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` tables. + + This accounts for when someone downgrades their Synapse version and then upgrades it + again. This will ensure that we don't have any out-of-date data in the + `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` tables. + + FIXME: This can be removed once we bump `SCHEMA_COMPAT_VERSION` and run the + foreground update for + `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` (tracked by + https://github.com/element-hq/synapse/issues/TODO) + """ + + _clear_out_of_date_sliding_sync_joined_rooms_table(txn) + _clear_out_of_date_sliding_sync_membership_snapshots_table(txn) + + +def _clear_out_of_date_sliding_sync_joined_rooms_table( + txn: LoggingTransaction, +) -> None: + """ + Clears out-of-date entries from the `sliding_sync_joined_rooms` table. + + This accounts for when someone downgrades their Synapse version and then upgrades it + again. This will ensure that we don't have any out-of-date data in the + `sliding_sync_joined_rooms` table. + + FIXME: This can be removed once we bump `SCHEMA_COMPAT_VERSION` and run the + foreground update for + `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` (tracked by + https://github.com/element-hq/synapse/issues/TODO) + """ + + # Find the point when we stopped writing to the `sliding_sync_joined_rooms` table + txn.execute( + """ + SELECT event_stream_ordering + FROM sliding_sync_joined_rooms + ORDER BY event_stream_ordering DESC + LIMIT 1 + """, + ) + + row = txn.fetchone() + # We have nothing written to the `sliding_sync_joined_rooms` table so there is + # nothing to clean up + if row is None: + return + + max_stream_ordering_sliding_sync_joined_rooms_table = row[0] + + txn.execute( + """ + SELECT DISTINCT(room_id) + FROM events + WHERE stream_ordering > ? + ORDER BY stream_ordering DESC + """, + (max_stream_ordering_sliding_sync_joined_rooms_table,), + ) + + room_rows = txn.fetchall() + # No new events have been written to the `events` table since the last time we wrote + # to the `sliding_sync_joined_rooms` table so there is nothing to clean up. This is + # the expected normal scenario for people who have not downgraded their Synapse + # version. + if not room_rows: + return + + for chunk in batch_iter(room_rows, 1000): + # Handle updating the `sliding_sync_joined_rooms` table + # + DatabasePool.simple_delete_many_batch_txn( + txn, + table="sliding_sync_joined_rooms", + keys=("room_id",), + values=chunk, + ) + + +def _clear_out_of_date_sliding_sync_membership_snapshots_table( + txn: LoggingTransaction, +) -> None: + """ + Clears out-of-date entries from the `sliding_sync_membership_snapshots` table. + + This accounts for when someone downgrades their Synapse version and then upgrades it + again. This will ensure that we don't have any out-of-date data in the + `sliding_sync_membership_snapshots` table. + + FIXME: This can be removed once we bump `SCHEMA_COMPAT_VERSION` and run the + foreground update for + `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` (tracked by + https://github.com/element-hq/synapse/issues/TODO) + """ + + # Find the point when we stopped writing to the `sliding_sync_membership_snapshots` table + txn.execute( + """ + SELECT event_stream_ordering + FROM sliding_sync_membership_snapshots + ORDER BY event_stream_ordering DESC + LIMIT 1 + """, + ) + + row = txn.fetchone() + # We have nothing written to the `sliding_sync_membership_snapshots` table so there is + # nothing to clean up + if row is None: + return + + max_stream_ordering_sliding_sync_membership_snapshots_table = row[0] + + txn.execute( + """ + SELECT DISTINCT(user_id, room_id) + FROM room_memberships + WHERE event_stream_ordering > ? + ORDER BY event_stream_ordering DESC + """, + (max_stream_ordering_sliding_sync_membership_snapshots_table,), + ) + + membership_rows = txn.fetchall() + # No new events have been written to the `events` table since the last time we wrote + # to the `sliding_sync_membership_snapshots` table so there is nothing to clean up. + # This is the expected normal scenario for people who have not downgraded their + # Synapse version. + if not membership_rows: + return + + for chunk in batch_iter(membership_rows, 1000): + # Handle updating the `sliding_sync_membership_snapshots` table + # + DatabasePool.simple_delete_many_batch_txn( + txn, + table="sliding_sync_membership_snapshots", + keys=("user_id", "room_id"), + values=chunk, + ) + def _apply_module_schemas( txn: Cursor, database_engine: BaseDatabaseEngine, config: HomeServerConfig From a94c1dd62c73bd878a539e04ce132c8f178fe4c9 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 26 Aug 2024 16:27:20 -0500 Subject: [PATCH 04/28] Add more context for why --- synapse/storage/prepare_database.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index 2b323c0272..83993cb969 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -589,8 +589,13 @@ def _clear_out_of_date_sliding_sync_tables( `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` tables. This accounts for when someone downgrades their Synapse version and then upgrades it - again. This will ensure that we don't have any out-of-date data in the - `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` tables. + again. This will ensure that we don't have any out-of-date/stale data in the + `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` tables since any new + events sent in rooms would have also needed to be written to the sliding sync + tables. For example a new event needs to bump `event_stream_ordering` in + `sliding_sync_joined_rooms` table or some state in the room changing (like the room + name). Or another example of someone's membership changing in a room affecting + `sliding_sync_membership_snapshots`. FIXME: This can be removed once we bump `SCHEMA_COMPAT_VERSION` and run the foreground update for @@ -609,8 +614,11 @@ def _clear_out_of_date_sliding_sync_joined_rooms_table( Clears out-of-date entries from the `sliding_sync_joined_rooms` table. This accounts for when someone downgrades their Synapse version and then upgrades it - again. This will ensure that we don't have any out-of-date data in the - `sliding_sync_joined_rooms` table. + again. This will ensure that we don't have any out-of-date/stale data in the + `sliding_sync_joined_rooms` table since any new events sent in rooms would have also + needed to be written to the `sliding_sync_joined_rooms` table or some state in the + room changing (like the room name). For example a new event needs to bump + `event_stream_ordering` in `sliding_sync_joined_rooms`. FIXME: This can be removed once we bump `SCHEMA_COMPAT_VERSION` and run the foreground update for @@ -672,8 +680,10 @@ def _clear_out_of_date_sliding_sync_membership_snapshots_table( Clears out-of-date entries from the `sliding_sync_membership_snapshots` table. This accounts for when someone downgrades their Synapse version and then upgrades it - again. This will ensure that we don't have any out-of-date data in the - `sliding_sync_membership_snapshots` table. + again. This will ensure that we don't have any out-of-date/stale data in the + `sliding_sync_membership_snapshots` table since any new membership changes in rooms + would have also needed to be written to the `sliding_sync_membership_snapshots` + table. FIXME: This can be removed once we bump `SCHEMA_COMPAT_VERSION` and run the foreground update for From 6a44686dc3002a691ca1a6bdcc1fc1a03bf0c085 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 26 Aug 2024 16:32:59 -0500 Subject: [PATCH 05/28] Why it matters --- synapse/storage/prepare_database.py | 48 ++++++++++------------------- 1 file changed, 17 insertions(+), 31 deletions(-) diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index 83993cb969..01559da03f 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -576,20 +576,20 @@ def _upgrade_existing_database( # foreground update for # `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` (tracked by # https://github.com/element-hq/synapse/issues/TODO) - _clear_out_of_date_sliding_sync_tables( + _clear_stale_data_in_sliding_sync_tables( txn=cur, ) -def _clear_out_of_date_sliding_sync_tables( +def _clear_stale_data_in_sliding_sync_tables( txn: LoggingTransaction, ) -> None: """ - Clears out-of-date entries from the + Clears stale/out-of-date entries from the `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` tables. This accounts for when someone downgrades their Synapse version and then upgrades it - again. This will ensure that we don't have any out-of-date/stale data in the + again. This will ensure that we don't have any stale/out-of-date data in the `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` tables since any new events sent in rooms would have also needed to be written to the sliding sync tables. For example a new event needs to bump `event_stream_ordering` in @@ -597,33 +597,28 @@ def _clear_out_of_date_sliding_sync_tables( name). Or another example of someone's membership changing in a room affecting `sliding_sync_membership_snapshots`. + This way, if a row exists in the sliding sync tables, we are able to rely on it + (accurate data). So if a row doesn't exist, we use a fallback to get the same info + until the background updates fill in the rows or a new event comes in triggering it + to be fully inserted. + FIXME: This can be removed once we bump `SCHEMA_COMPAT_VERSION` and run the foreground update for `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` (tracked by https://github.com/element-hq/synapse/issues/TODO) """ - _clear_out_of_date_sliding_sync_joined_rooms_table(txn) - _clear_out_of_date_sliding_sync_membership_snapshots_table(txn) + _clear_stale_data_in_sliding_sync_joined_rooms_table(txn) + _clear_stale_data_in_sliding_sync_membership_snapshots_table(txn) -def _clear_out_of_date_sliding_sync_joined_rooms_table( +def _clear_stale_data_in_sliding_sync_joined_rooms_table( txn: LoggingTransaction, ) -> None: """ - Clears out-of-date entries from the `sliding_sync_joined_rooms` table. + Clears stale/out-of-date entries from the `sliding_sync_joined_rooms` table. - This accounts for when someone downgrades their Synapse version and then upgrades it - again. This will ensure that we don't have any out-of-date/stale data in the - `sliding_sync_joined_rooms` table since any new events sent in rooms would have also - needed to be written to the `sliding_sync_joined_rooms` table or some state in the - room changing (like the room name). For example a new event needs to bump - `event_stream_ordering` in `sliding_sync_joined_rooms`. - - FIXME: This can be removed once we bump `SCHEMA_COMPAT_VERSION` and run the - foreground update for - `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` (tracked by - https://github.com/element-hq/synapse/issues/TODO) + See `_clear_out_of_date_sliding_sync_tables()` description above for more context. """ # Find the point when we stopped writing to the `sliding_sync_joined_rooms` table @@ -673,22 +668,13 @@ def _clear_out_of_date_sliding_sync_joined_rooms_table( ) -def _clear_out_of_date_sliding_sync_membership_snapshots_table( +def _clear_stale_data_in_sliding_sync_membership_snapshots_table( txn: LoggingTransaction, ) -> None: """ - Clears out-of-date entries from the `sliding_sync_membership_snapshots` table. + Clears stale/out-of-date entries from the `sliding_sync_membership_snapshots` table. - This accounts for when someone downgrades their Synapse version and then upgrades it - again. This will ensure that we don't have any out-of-date/stale data in the - `sliding_sync_membership_snapshots` table since any new membership changes in rooms - would have also needed to be written to the `sliding_sync_membership_snapshots` - table. - - FIXME: This can be removed once we bump `SCHEMA_COMPAT_VERSION` and run the - foreground update for - `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` (tracked by - https://github.com/element-hq/synapse/issues/TODO) + See `_clear_out_of_date_sliding_sync_tables()` description above for more context. """ # Find the point when we stopped writing to the `sliding_sync_membership_snapshots` table From eb3c84cf45e67c25116de55deeb9a78275dd8e6c Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 26 Aug 2024 17:31:26 -0500 Subject: [PATCH 06/28] Kick-off background update for out-of-date snapshots --- synapse/storage/database.py | 4 +- .../databases/main/events_bg_updates.py | 5 ++ synapse/storage/prepare_database.py | 47 ++++++++++++++----- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/synapse/storage/database.py b/synapse/storage/database.py index ba2616b479..da50fd7f83 100644 --- a/synapse/storage/database.py +++ b/synapse/storage/database.py @@ -1323,7 +1323,7 @@ class DatabasePool: if lock: # We need to lock the table :( - self.engine.lock_table(txn, table) + txn.database_engine.lock_table(txn, table) def _getwhere(key: str) -> str: # If the value we're passing in is None (aka NULL), we need to use @@ -1377,8 +1377,8 @@ class DatabasePool: # successfully inserted return True + @staticmethod def simple_upsert_txn_native_upsert( - self, txn: LoggingTransaction, table: str, keyvalues: Mapping[str, Any], diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index af0a1ba880..1287b995fb 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1552,6 +1552,11 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS def _get_rooms_to_update_txn(txn: LoggingTransaction) -> List[str]: # Fetch the set of room IDs that we want to update + # + # We use `current_state_events` table as the barometer for whether the + # server is still participating in the room because if we're + # `no_longer_in_room`, this table would be cleared out for the given + # `room_id`. txn.execute( """ SELECT DISTINCT room_id FROM current_state_events diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index 01559da03f..b127e83e61 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -42,6 +42,7 @@ from synapse.storage.database import ( LoggingDatabaseConnection, LoggingTransaction, ) +from synapse.storage.databases.main.events_bg_updates import _BackgroundUpdates from synapse.storage.engines import BaseDatabaseEngine, PostgresEngine, Sqlite3Engine from synapse.storage.schema import SCHEMA_COMPAT_VERSION, SCHEMA_VERSION from synapse.storage.types import Cursor @@ -576,12 +577,12 @@ def _upgrade_existing_database( # foreground update for # `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` (tracked by # https://github.com/element-hq/synapse/issues/TODO) - _clear_stale_data_in_sliding_sync_tables( + _resolve_stale_data_in_sliding_sync_tables( txn=cur, ) -def _clear_stale_data_in_sliding_sync_tables( +def _resolve_stale_data_in_sliding_sync_tables( txn: LoggingTransaction, ) -> None: """ @@ -598,7 +599,7 @@ def _clear_stale_data_in_sliding_sync_tables( `sliding_sync_membership_snapshots`. This way, if a row exists in the sliding sync tables, we are able to rely on it - (accurate data). So if a row doesn't exist, we use a fallback to get the same info + (accurate data). And if a row doesn't exist, we use a fallback to get the same info until the background updates fill in the rows or a new event comes in triggering it to be fully inserted. @@ -608,17 +609,20 @@ def _clear_stale_data_in_sliding_sync_tables( https://github.com/element-hq/synapse/issues/TODO) """ - _clear_stale_data_in_sliding_sync_joined_rooms_table(txn) - _clear_stale_data_in_sliding_sync_membership_snapshots_table(txn) + _resolve_stale_data_in_sliding_sync_joined_rooms_table(txn) + _resolve_stale_data_in_sliding_sync_membership_snapshots_table(txn) -def _clear_stale_data_in_sliding_sync_joined_rooms_table( +def _resolve_stale_data_in_sliding_sync_joined_rooms_table( txn: LoggingTransaction, ) -> None: """ - Clears stale/out-of-date entries from the `sliding_sync_joined_rooms` table. + Clears stale/out-of-date entries from the `sliding_sync_joined_rooms` table and + kicks-off the background update to catch-up with what we missed while Synapse was + downgraded. - See `_clear_out_of_date_sliding_sync_tables()` description above for more context. + See `_resolve_stale_data_in_sliding_sync_tables()` description above for more + context. """ # Find the point when we stopped writing to the `sliding_sync_joined_rooms` table @@ -668,13 +672,16 @@ def _clear_stale_data_in_sliding_sync_joined_rooms_table( ) -def _clear_stale_data_in_sliding_sync_membership_snapshots_table( +def _resolve_stale_data_in_sliding_sync_membership_snapshots_table( txn: LoggingTransaction, ) -> None: """ - Clears stale/out-of-date entries from the `sliding_sync_membership_snapshots` table. + Clears stale/out-of-date entries from the `sliding_sync_membership_snapshots` table + and kicks-off the background update to catch-up with what we missed while Synapse + was downgraded. - See `_clear_out_of_date_sliding_sync_tables()` description above for more context. + See `_resolve_stale_data_in_sliding_sync_tables()` description above for more + context. """ # Find the point when we stopped writing to the `sliding_sync_membership_snapshots` table @@ -698,7 +705,7 @@ def _clear_stale_data_in_sliding_sync_membership_snapshots_table( txn.execute( """ SELECT DISTINCT(user_id, room_id) - FROM room_memberships + FROM local_current_membership WHERE event_stream_ordering > ? ORDER BY event_stream_ordering DESC """, @@ -723,6 +730,22 @@ def _clear_stale_data_in_sliding_sync_membership_snapshots_table( values=chunk, ) + # Now kick-off the background update to catch-up with what we missed while Synapse + # was downgraded. + DatabasePool.simple_upsert_txn_native_upsert( + txn, + table="background_updates", + keyvalues={ + "update_name": _BackgroundUpdates.SLIDING_SYNC_MEMBERSHIP_SNAPSHOTS_BG_UPDATE + }, + values={}, + # Only insert the row if it doesn't already exist. If it already exists, we will + # eventually fill in the rows we're trying to populate. + insertion_values={ + "progress_json": f'{ "last_event_stream_ordering": {str(max_stream_ordering_sliding_sync_membership_snapshots_table)} }', + }, + ) + def _apply_module_schemas( txn: Cursor, database_engine: BaseDatabaseEngine, config: HomeServerConfig From 53473a0eb4a839d0f168c88e6d14f145d37fed7d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 26 Aug 2024 18:35:49 -0500 Subject: [PATCH 07/28] Adapt `sliding_sync_joined_rooms` background update to use `event_stream_ordering` for progress This way we can re-use it for the catch-up background process --- .../databases/main/events_bg_updates.py | 118 ++++++++++++++---- synapse/storage/prepare_database.py | 16 +++ 2 files changed, 110 insertions(+), 24 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 1287b995fb..38a66112bd 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -20,6 +20,7 @@ # import logging +from collections import OrderedDict from typing import TYPE_CHECKING, Dict, List, Optional, Set, Tuple, cast import attr @@ -112,6 +113,22 @@ class _CalculateChainCover: finished_room_map: Dict[str, Tuple[int, int]] +@attr.s(slots=True, frozen=True, auto_attribs=True) +class _JoinedRoomStreamOrderingUpdate: + """ + Intermediate container class used in `SLIDING_SYNC_JOINED_ROOMS_BG_UPDATE` + """ + + # The most recent event stream_ordering for the room + most_recent_event_stream_ordering: int + # The most recent event `bump_stamp` for the room + most_recent_bump_stamp: Optional[int] + # The `stream_ordering` in the `current_state_delta_stream` that we got the state + # values from. We can use this to check if the current state has been updated since + # we last checked. + last_current_state_delta_stream_id: int + + class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseStore): def __init__( self, @@ -1548,28 +1565,49 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS """ Background update to populate the `sliding_sync_joined_rooms` table. """ - last_room_id = progress.get("last_room_id", "") + last_event_stream_ordering = progress.get( + "last_event_stream_ordering", -(1 << 31) + ) - def _get_rooms_to_update_txn(txn: LoggingTransaction) -> List[str]: + def _get_rooms_to_update_txn(txn: LoggingTransaction) -> List[Tuple[str, int]]: + """ + Returns: + A list of room ID's to update along with the progress value + (event_stream_ordering) indicating the continuation point in the + `current_state_events` table for the next batch. + """ # Fetch the set of room IDs that we want to update # # We use `current_state_events` table as the barometer for whether the # server is still participating in the room because if we're # `no_longer_in_room`, this table would be cleared out for the given # `room_id`. + # + # Because we're using `event_stream_ordering` as the progress marker, we're + # going to be pulling out the same rooms over and over again but we can + # at-least re-use this background update for the catch-up background + # process as well (see `_resolve_stale_data_in_sliding_sync_tables()`). + # + # It's important to sort by `event_stream_ordering` *ascending* (oldest to + # newest) so that if we see that this background update in progress and want + # to start the catch-up process, we can safely assume that it will + # eventually get to the rooms we want to catch-up on anyway (see + # `_resolve_stale_data_in_sliding_sync_tables()`). txn.execute( """ - SELECT DISTINCT room_id FROM current_state_events - WHERE room_id > ? - ORDER BY room_id ASC + SELECT room_id, max(event_stream_ordering) + FROM current_state_events + WHERE event_stream_ordering > ? + GROUP BY room_id + ORDER BY event_stream_ordering ASC LIMIT ? """, - (last_room_id, batch_size), + (last_event_stream_ordering, batch_size), ) - rooms_to_update_rows = cast(List[Tuple[str]], txn.fetchall()) + rooms_to_update_rows = cast(List[Tuple[str, int]], txn.fetchall()) - return [row[0] for row in rooms_to_update_rows] + return rooms_to_update_rows rooms_to_update = await self.db_pool.runInteraction( "_sliding_sync_joined_rooms_bg_update._get_rooms_to_update_txn", @@ -1582,13 +1620,21 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS ) return 0 - # Map from room_id to insert/update state values in the `sliding_sync_joined_rooms` table + # Map from room_id to insert/update state values in the `sliding_sync_joined_rooms` table. joined_room_updates: Dict[str, SlidingSyncStateInsertValues] = {} # Map from room_id to stream_ordering/bump_stamp/last_current_state_delta_stream_id values joined_room_stream_ordering_updates: Dict[ - str, Tuple[int, Optional[int], int] + str, _JoinedRoomStreamOrderingUpdate ] = {} - for room_id in rooms_to_update: + # Map from room_id to the progress value (event_stream_ordering) + # + # This needs to be an `OrderedDict` because we need to process things in + # `event_stream_ordering` order *ascending* to save our progress position + # correctly if we need to exit early. + room_id_to_progress_marker_map: OrderedDict[str, int] = OrderedDict() + for room_id, progress_event_stream_ordering in rooms_to_update: + room_id_to_progress_marker_map[room_id] = progress_event_stream_ordering + current_state_ids_map, last_current_state_delta_stream_id = ( await self.db_pool.runInteraction( "_sliding_sync_joined_rooms_bg_update._get_relevant_sliding_sync_current_state_event_ids_txn", @@ -1645,21 +1691,36 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS most_recent_bump_stamp = bump_stamp_event_pos_results[1].stream joined_room_stream_ordering_updates[room_id] = ( - most_recent_event_stream_ordering, - most_recent_bump_stamp, - last_current_state_delta_stream_id, + _JoinedRoomStreamOrderingUpdate( + most_recent_event_stream_ordering=most_recent_event_stream_ordering, + most_recent_bump_stamp=most_recent_bump_stamp, + last_current_state_delta_stream_id=last_current_state_delta_stream_id, + ) ) def _fill_table_txn(txn: LoggingTransaction) -> None: # Handle updating the `sliding_sync_joined_rooms` table # last_successful_room_id: Optional[str] = None - for room_id, insert_map in joined_room_updates.items(): - ( - event_stream_ordering, - bump_stamp, - last_current_state_delta_stream_id, - ) = joined_room_stream_ordering_updates[room_id] + # Process the rooms in `event_stream_ordering` order *ascending* so we can + # save our position correctly if we need to exit early. + # `progress_event_stream_ordering` is an `OrderedDict` which remembers + # insertion order (and we inserted in the correct order) so this should be + # the correct thing to do. + for ( + room_id, + progress_event_stream_ordering, + ) in room_id_to_progress_marker_map.items(): + update_map = joined_room_updates[room_id] + + joined_room_update = joined_room_stream_ordering_updates[room_id] + event_stream_ordering = ( + joined_room_update.most_recent_event_stream_ordering + ) + bump_stamp = joined_room_update.most_recent_bump_stamp + last_current_state_delta_stream_id = ( + joined_room_update.last_current_state_delta_stream_id + ) # Check if the current state has been updated since we gathered it state_deltas_since_we_gathered_current_state = ( @@ -1673,7 +1734,7 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS ) ) for state_delta in state_deltas_since_we_gathered_current_state: - # We only need to check if the state is relevant to the + # We only need to check for the state is relevant to the # `sliding_sync_joined_rooms` table. if ( state_delta.event_type, @@ -1684,7 +1745,9 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS self.db_pool.updates._background_update_progress_txn( txn, _BackgroundUpdates.SLIDING_SYNC_JOINED_ROOMS_BG_UPDATE, - {"last_room_id": room_id}, + { + "last_event_stream_ordering": progress_event_stream_ordering + }, ) # Raising exception so we can just exit and try again. It would # be hard to resolve this within the transaction because we need @@ -1706,7 +1769,7 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS txn, table="sliding_sync_joined_rooms", keyvalues={"room_id": room_id}, - values=insert_map, + values=update_map, insertion_values={ # The reason we're only *inserting* (not *updating*) `event_stream_ordering` # and `bump_stamp` is because if they are present, that means they are already @@ -1724,9 +1787,10 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS ) # Update the progress + _ = room_id_to_progress_marker_map.values() await self.db_pool.updates._background_update_progress( _BackgroundUpdates.SLIDING_SYNC_JOINED_ROOMS_BG_UPDATE, - {"last_room_id": rooms_to_update[-1]}, + {"last_event_stream_ordering": rooms_to_update[-1][1]}, ) return len(rooms_to_update) @@ -1745,6 +1809,12 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS txn: LoggingTransaction, ) -> List[Tuple[str, str, str, str, str, int, bool]]: # Fetch the set of event IDs that we want to update + # + # It's important to sort by `event_stream_ordering` *ascending* (oldest to + # newest) so that if we see that this background update in progress and want + # to start the catch-up process, we can safely assume that it will + # eventually get to the rooms we want to catch-up on anyway (see + # `_resolve_stale_data_in_sliding_sync_tables()`). txn.execute( """ SELECT diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index b127e83e61..13dcf9803d 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -671,6 +671,22 @@ def _resolve_stale_data_in_sliding_sync_joined_rooms_table( values=chunk, ) + # Now kick-off the background update to catch-up with what we missed while Synapse + # was downgraded. + DatabasePool.simple_upsert_txn_native_upsert( + txn, + table="background_updates", + keyvalues={ + "update_name": _BackgroundUpdates.SLIDING_SYNC_JOINED_ROOMS_BG_UPDATE + }, + values={}, + # Only insert the row if it doesn't already exist. If it already exists, we will + # eventually fill in the rows we're trying to populate. + insertion_values={ + "progress_json": f'{ "last_event_stream_ordering": {str(max_stream_ordering_sliding_sync_joined_rooms_table)} }', + }, + ) + def _resolve_stale_data_in_sliding_sync_membership_snapshots_table( txn: LoggingTransaction, From 7fe5d31e201d0e89b898608f20cc8e98e0d31e4f Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 26 Aug 2024 18:52:10 -0500 Subject: [PATCH 08/28] Note down caveat about `forgotten` --- synapse/storage/prepare_database.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index 13dcf9803d..9d712d4118 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -718,6 +718,12 @@ def _resolve_stale_data_in_sliding_sync_membership_snapshots_table( max_stream_ordering_sliding_sync_membership_snapshots_table = row[0] + # XXX: Since `forgotten` is simply a flag on the `room_memberships` table that is + # set out-of-band, there is no way to tell whether it was set while Synapse was + # downgraded. The only thing the user can do is `/forget` again if they run into + # this. + # + # This only picks up changes to memberships. txn.execute( """ SELECT DISTINCT(user_id, room_id) From 7a0c2810286463e9e55cbe463c4bea39d55e3fd4 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 26 Aug 2024 19:43:52 -0500 Subject: [PATCH 09/28] Add placeholder tests --- tests/storage/test_sliding_sync_tables.py | 24 +++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/storage/test_sliding_sync_tables.py b/tests/storage/test_sliding_sync_tables.py index fb2340b446..012176784c 100644 --- a/tests/storage/test_sliding_sync_tables.py +++ b/tests/storage/test_sliding_sync_tables.py @@ -4172,3 +4172,27 @@ class SlidingSyncTablesBackgroundUpdatesTestCase(SlidingSyncTablesTestCaseBase): sliding_sync_membership_snapshots_results.get((room_id, user2_id)), user2_snapshot, ) + + +class SlidingSyncTablesCatchUpBackgroundUpdatesTestCase(SlidingSyncTablesTestCaseBase): + """ + Test the background updates for catch-up after Synapse downgrade populate the `sliding_sync_joined_rooms` and + `sliding_sync_membership_snapshots` tables. + + FIXME: This can be removed once we bump `SCHEMA_COMPAT_VERSION` and run the + foreground update for + `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` (tracked by + https://github.com/element-hq/synapse/issues/TODO) + """ + + def test_joined_background_update_catch_up(self) -> None: + """ + TODO + """ + pass + + def test_membership_snapshots_background_update_catch_up(self) -> None: + """ + TODO + """ + pass From 9764f626ea208be584424bf79b777e85f4b2ca92 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 27 Aug 2024 12:09:00 -0500 Subject: [PATCH 10/28] Fix query in Postgres --- synapse/storage/databases/main/events_bg_updates.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 38a66112bd..52b4450bbc 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1595,11 +1595,11 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS # `_resolve_stale_data_in_sliding_sync_tables()`). txn.execute( """ - SELECT room_id, max(event_stream_ordering) + SELECT room_id, MAX(event_stream_ordering) FROM current_state_events WHERE event_stream_ordering > ? GROUP BY room_id - ORDER BY event_stream_ordering ASC + ORDER BY MAX(event_stream_ordering) ASC LIMIT ? """, (last_event_stream_ordering, batch_size), From c51a309da59e6ac50467ffcdce70ad7fc21c58f9 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 27 Aug 2024 17:11:51 -0500 Subject: [PATCH 11/28] Maybe: always start background update --- synapse/storage/prepare_database.py | 96 +++++++++++++++++------------ 1 file changed, 56 insertions(+), 40 deletions(-) diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index 9d712d4118..16b4dea523 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -24,6 +24,7 @@ import os import re from collections import Counter from typing import ( + cast, Collection, Counter as CounterType, Generator, @@ -36,6 +37,7 @@ from typing import ( import attr +from synapse.util import Clock, json_encoder from synapse.config.homeserver import HomeServerConfig from synapse.storage.database import ( DatabasePool, @@ -683,7 +685,13 @@ def _resolve_stale_data_in_sliding_sync_joined_rooms_table( # Only insert the row if it doesn't already exist. If it already exists, we will # eventually fill in the rows we're trying to populate. insertion_values={ - "progress_json": f'{ "last_event_stream_ordering": {str(max_stream_ordering_sliding_sync_joined_rooms_table)} }', + "progress_json": json_encoder.encode( + { + "last_event_stream_ordering": { + str(max_stream_ordering_sliding_sync_joined_rooms_table) + } + } + ), }, ) @@ -710,50 +718,58 @@ def _resolve_stale_data_in_sliding_sync_membership_snapshots_table( """, ) - row = txn.fetchone() - # We have nothing written to the `sliding_sync_membership_snapshots` table so there is - # nothing to clean up - if row is None: - return + # If we have nothing written to the `sliding_sync_membership_snapshots` table, + # there is nothing to clean up + row = cast(Tuple[int], txn.fetchone()) + max_stream_ordering_sliding_sync_membership_snapshots_table = None + if row is not None: + max_stream_ordering_sliding_sync_membership_snapshots_table = row[0] - max_stream_ordering_sliding_sync_membership_snapshots_table = row[0] - - # XXX: Since `forgotten` is simply a flag on the `room_memberships` table that is - # set out-of-band, there is no way to tell whether it was set while Synapse was - # downgraded. The only thing the user can do is `/forget` again if they run into - # this. - # - # This only picks up changes to memberships. - txn.execute( - """ - SELECT DISTINCT(user_id, room_id) - FROM local_current_membership - WHERE event_stream_ordering > ? - ORDER BY event_stream_ordering DESC - """, - (max_stream_ordering_sliding_sync_membership_snapshots_table,), - ) - - membership_rows = txn.fetchall() - # No new events have been written to the `events` table since the last time we wrote - # to the `sliding_sync_membership_snapshots` table so there is nothing to clean up. - # This is the expected normal scenario for people who have not downgraded their - # Synapse version. - if not membership_rows: - return - - for chunk in batch_iter(membership_rows, 1000): - # Handle updating the `sliding_sync_membership_snapshots` table + # XXX: Since `forgotten` is simply a flag on the `room_memberships` table that is + # set out-of-band, there is no way to tell whether it was set while Synapse was + # downgraded. The only thing the user can do is `/forget` again if they run into + # this. # - DatabasePool.simple_delete_many_batch_txn( - txn, - table="sliding_sync_membership_snapshots", - keys=("user_id", "room_id"), - values=chunk, + # This only picks up changes to memberships. + txn.execute( + """ + SELECT DISTINCT(user_id, room_id) + FROM local_current_membership + WHERE event_stream_ordering > ? + ORDER BY event_stream_ordering DESC + """, + (max_stream_ordering_sliding_sync_membership_snapshots_table,), ) + membership_rows = txn.fetchall() + # No new events have been written to the `events` table since the last time we wrote + # to the `sliding_sync_membership_snapshots` table so there is nothing to clean up. + # This is the expected normal scenario for people who have not downgraded their + # Synapse version. + if not membership_rows: + return + + for chunk in batch_iter(membership_rows, 1000): + # Handle updating the `sliding_sync_membership_snapshots` table + # + DatabasePool.simple_delete_many_batch_txn( + txn, + table="sliding_sync_membership_snapshots", + keys=("user_id", "room_id"), + values=chunk, + ) + # Now kick-off the background update to catch-up with what we missed while Synapse # was downgraded. + # + progress_json = {} + if max_stream_ordering_sliding_sync_membership_snapshots_table is not None: + progress_json["last_event_stream_ordering"] = ( + max_stream_ordering_sliding_sync_membership_snapshots_table + ) + + # We still need to kick off the background update to catch-up regardless of whether + # there was anything to clean up. DatabasePool.simple_upsert_txn_native_upsert( txn, table="background_updates", @@ -764,7 +780,7 @@ def _resolve_stale_data_in_sliding_sync_membership_snapshots_table( # Only insert the row if it doesn't already exist. If it already exists, we will # eventually fill in the rows we're trying to populate. insertion_values={ - "progress_json": f'{ "last_event_stream_ordering": {str(max_stream_ordering_sliding_sync_membership_snapshots_table)} }', + "progress_json": json_encoder.encode(progress_json), }, ) From 9a7d8c2be44072da69b12a3edee02f04271d94c8 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 27 Aug 2024 17:28:07 -0500 Subject: [PATCH 12/28] Start catch-up if nothing written yet --- synapse/storage/prepare_database.py | 99 ++++++++++++++++------------- 1 file changed, 55 insertions(+), 44 deletions(-) diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index 16b4dea523..2527766e2d 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -48,6 +48,7 @@ from synapse.storage.databases.main.events_bg_updates import _BackgroundUpdates from synapse.storage.engines import BaseDatabaseEngine, PostgresEngine, Sqlite3Engine from synapse.storage.schema import SCHEMA_COMPAT_VERSION, SCHEMA_VERSION from synapse.storage.types import Cursor +from synapse.types import JsonDict from synapse.util.iterutils import batch_iter logger = logging.getLogger(__name__) @@ -637,44 +638,56 @@ def _resolve_stale_data_in_sliding_sync_joined_rooms_table( """, ) - row = txn.fetchone() - # We have nothing written to the `sliding_sync_joined_rooms` table so there is + # If we have nothing written to the `sliding_sync_joined_rooms` table, there is # nothing to clean up - if row is None: - return + row = cast(Optional[Tuple[int]], txn.fetchone()) + max_stream_ordering_sliding_sync_joined_rooms_table = None + if row is not None: + (max_stream_ordering_sliding_sync_joined_rooms_table,) = row - max_stream_ordering_sliding_sync_joined_rooms_table = row[0] - - txn.execute( - """ - SELECT DISTINCT(room_id) - FROM events - WHERE stream_ordering > ? - ORDER BY stream_ordering DESC - """, - (max_stream_ordering_sliding_sync_joined_rooms_table,), - ) - - room_rows = txn.fetchall() - # No new events have been written to the `events` table since the last time we wrote - # to the `sliding_sync_joined_rooms` table so there is nothing to clean up. This is - # the expected normal scenario for people who have not downgraded their Synapse - # version. - if not room_rows: - return - - for chunk in batch_iter(room_rows, 1000): - # Handle updating the `sliding_sync_joined_rooms` table - # - DatabasePool.simple_delete_many_batch_txn( - txn, - table="sliding_sync_joined_rooms", - keys=("room_id",), - values=chunk, + txn.execute( + """ + SELECT DISTINCT(room_id) + FROM events + WHERE stream_ordering > ? + ORDER BY stream_ordering DESC + """, + (max_stream_ordering_sliding_sync_joined_rooms_table,), ) + room_rows = txn.fetchall() + # No new events have been written to the `events` table since the last time we wrote + # to the `sliding_sync_joined_rooms` table so there is nothing to clean up. This is + # the expected normal scenario for people who have not downgraded their Synapse + # version. + if not room_rows: + return + + # 1000 is an arbitrary batch size with no testing + for chunk in batch_iter(room_rows, 1000): + # Handle updating the `sliding_sync_joined_rooms` table + # + DatabasePool.simple_delete_many_batch_txn( + txn, + table="sliding_sync_joined_rooms", + keys=("room_id",), + values=chunk, + ) + # Now kick-off the background update to catch-up with what we missed while Synapse # was downgraded. + # + # We may need to catch-up on everything if we have nothing written to the + # `sliding_sync_joined_rooms` table yet. This could happen if someone had zero rooms + # on their server (so the normal background update completes), downgrade Synapse + # versions, join and create some new rooms, and upgrade again. + # + progress_json: JsonDict = {} + if max_stream_ordering_sliding_sync_joined_rooms_table is not None: + progress_json["last_event_stream_ordering"] = ( + max_stream_ordering_sliding_sync_joined_rooms_table + ) + DatabasePool.simple_upsert_txn_native_upsert( txn, table="background_updates", @@ -685,13 +698,7 @@ def _resolve_stale_data_in_sliding_sync_joined_rooms_table( # Only insert the row if it doesn't already exist. If it already exists, we will # eventually fill in the rows we're trying to populate. insertion_values={ - "progress_json": json_encoder.encode( - { - "last_event_stream_ordering": { - str(max_stream_ordering_sliding_sync_joined_rooms_table) - } - } - ), + "progress_json": json_encoder.encode(progress_json), }, ) @@ -720,10 +727,10 @@ def _resolve_stale_data_in_sliding_sync_membership_snapshots_table( # If we have nothing written to the `sliding_sync_membership_snapshots` table, # there is nothing to clean up - row = cast(Tuple[int], txn.fetchone()) + row = cast(Optional[Tuple[int]], txn.fetchone()) max_stream_ordering_sliding_sync_membership_snapshots_table = None if row is not None: - max_stream_ordering_sliding_sync_membership_snapshots_table = row[0] + (max_stream_ordering_sliding_sync_membership_snapshots_table,) = row # XXX: Since `forgotten` is simply a flag on the `room_memberships` table that is # set out-of-band, there is no way to tell whether it was set while Synapse was @@ -749,6 +756,7 @@ def _resolve_stale_data_in_sliding_sync_membership_snapshots_table( if not membership_rows: return + # 1000 is an arbitrary batch size with no testing for chunk in batch_iter(membership_rows, 1000): # Handle updating the `sliding_sync_membership_snapshots` table # @@ -762,14 +770,17 @@ def _resolve_stale_data_in_sliding_sync_membership_snapshots_table( # Now kick-off the background update to catch-up with what we missed while Synapse # was downgraded. # - progress_json = {} + # We may need to catch-up on everything if we have nothing written to the + # `sliding_sync_membership_snapshots` table yet. This could happen if someone had + # zero rooms on their server (so the normal background update completes), downgrade + # Synapse versions, join and create some new rooms, and upgrade again. + # + progress_json: JsonDict = {} if max_stream_ordering_sliding_sync_membership_snapshots_table is not None: progress_json["last_event_stream_ordering"] = ( max_stream_ordering_sliding_sync_membership_snapshots_table ) - # We still need to kick off the background update to catch-up regardless of whether - # there was anything to clean up. DatabasePool.simple_upsert_txn_native_upsert( txn, table="background_updates", From 4dc9e268e68235d12bc3cb2119226660121b6d5d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 27 Aug 2024 18:08:17 -0500 Subject: [PATCH 13/28] Add test for catch-up background update --- synapse/storage/prepare_database.py | 4 +- tests/storage/test_sliding_sync_tables.py | 105 ++++++++++++++++++++-- 2 files changed, 102 insertions(+), 7 deletions(-) diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index 2527766e2d..31f99782ee 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -24,7 +24,6 @@ import os import re from collections import Counter from typing import ( - cast, Collection, Counter as CounterType, Generator, @@ -33,11 +32,11 @@ from typing import ( Optional, TextIO, Tuple, + cast, ) import attr -from synapse.util import Clock, json_encoder from synapse.config.homeserver import HomeServerConfig from synapse.storage.database import ( DatabasePool, @@ -49,6 +48,7 @@ from synapse.storage.engines import BaseDatabaseEngine, PostgresEngine, Sqlite3E from synapse.storage.schema import SCHEMA_COMPAT_VERSION, SCHEMA_VERSION from synapse.storage.types import Cursor from synapse.types import JsonDict +from synapse.util import json_encoder from synapse.util.iterutils import batch_iter logger = logging.getLogger(__name__) diff --git a/tests/storage/test_sliding_sync_tables.py b/tests/storage/test_sliding_sync_tables.py index 012176784c..23a0aee25c 100644 --- a/tests/storage/test_sliding_sync_tables.py +++ b/tests/storage/test_sliding_sync_tables.py @@ -34,6 +34,9 @@ from synapse.rest.client import login, room from synapse.server import HomeServer from synapse.storage.databases.main.events import DeltaState from synapse.storage.databases.main.events_bg_updates import _BackgroundUpdates +from synapse.storage.prepare_database import ( + _resolve_stale_data_in_sliding_sync_joined_rooms_table, +) from synapse.util import Clock from tests.test_utils.event_injection import create_event @@ -4176,8 +4179,14 @@ class SlidingSyncTablesBackgroundUpdatesTestCase(SlidingSyncTablesTestCaseBase): class SlidingSyncTablesCatchUpBackgroundUpdatesTestCase(SlidingSyncTablesTestCaseBase): """ - Test the background updates for catch-up after Synapse downgrade populate the `sliding_sync_joined_rooms` and - `sliding_sync_membership_snapshots` tables. + Test the background updates for catch-up after Synapse downgrade to populate the + `sliding_sync_joined_rooms` and `sliding_sync_membership_snapshots` tables. + + This to test the "catch-up" version of the background update vs the "normal" + background update to populate the tables with all of the historical data. Both + versions share the same background update but just serve different purposes. We + check if the "catch-up" version needs to run on start-up based on whether there have + been any changes to rooms that aren't reflected in the sliding sync tables. FIXME: This can be removed once we bump `SCHEMA_COMPAT_VERSION` and run the foreground update for @@ -4187,12 +4196,98 @@ class SlidingSyncTablesCatchUpBackgroundUpdatesTestCase(SlidingSyncTablesTestCas def test_joined_background_update_catch_up(self) -> None: """ - TODO + Test that new events while Synapse is downgraded (making + `sliding_sync_joined_rooms` stale) will be caught when Synapse is upgraded and + the catch-up routine is run. """ - pass + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + + # Instead of testing with various levels of room state that should appear in the + # table, we're only using one room to keep this test simple. Because the + # underlying background update to populate these tables is the same as this + # catch-up routine, we are going to rely on + # `SlidingSyncTablesBackgroundUpdatesTestCase` to cover that logic. + room_id_with_info = self.helper.create_room_as(user1_id, tok=user1_tok) + + # Get a snapshot of the `sliding_sync_joined_rooms` table before we add some state + sliding_sync_joined_rooms_results_before_state = ( + self._get_sliding_sync_joined_rooms() + ) + self.assertIncludes( + set(sliding_sync_joined_rooms_results_before_state.keys()), + {room_id_with_info}, + exact=True, + ) + + # Add a room name + self.helper.send_state( + room_id_with_info, + EventTypes.Name, + {"name": "my super duper room"}, + tok=user1_tok, + ) + + # Make sure all of the background updates have finished before we start the + # catch-up. Even though it should work fine if the other background update is + # still running, we want to see the catch-up routine restore the progress + # correctly. + # + # We also don't want the normal background update messing with our results so we + # run this before we do our manual database clean-up to simulate new events + # being sent while Synapse was downgraded. + self.wait_for_background_updates() + + # Clean-up the `sliding_sync_joined_rooms` table as if the the room name + # never made it into the table. This is to simulate the room name event + # being sent while Synapse was downgraded. + self.get_success( + self.store.db_pool.simple_update( + table="sliding_sync_joined_rooms", + keyvalues={"room_id": room_id_with_info}, + updatevalues={ + # Clear the room name + "room_name": None, + # Reset the `event_stream_ordering` back to the value before the room name + "event_stream_ordering": sliding_sync_joined_rooms_results_before_state[ + room_id_with_info + ].event_stream_ordering, + }, + desc="sliding_sync_joined_rooms.test_joined_background_update_catch_up", + ) + ) + + # The function under test. It should clear out stale data and start the + # background update to catch-up on the missing data. + self.get_success( + self.store.db_pool.runInteraction( + "_resolve_stale_data_in_sliding_sync_joined_rooms_table", + _resolve_stale_data_in_sliding_sync_joined_rooms_table, + ) + ) + + # Ensure that the stale data is deleted from the table + sliding_sync_joined_rooms_results = self._get_sliding_sync_joined_rooms() + self.assertIncludes( + set(sliding_sync_joined_rooms_results.keys()), + set(), + exact=True, + ) + + # Wait for the catch-up background update to finish + self.store.db_pool.updates._all_done = False + self.wait_for_background_updates() + + # Ensure that the table is populated correctly after the catch-up background + # update finishes + sliding_sync_joined_rooms_results = self._get_sliding_sync_joined_rooms() + self.assertIncludes( + set(sliding_sync_joined_rooms_results.keys()), + {room_id_with_info}, + exact=True, + ) def test_membership_snapshots_background_update_catch_up(self) -> None: """ TODO """ - pass From c8e17f7479d8e27144fb6705aa2b6797ff570532 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 27 Aug 2024 18:21:25 -0500 Subject: [PATCH 14/28] Add test when no rooms --- tests/storage/test_sliding_sync_tables.py | 104 ++++++++++++++++++++-- 1 file changed, 97 insertions(+), 7 deletions(-) diff --git a/tests/storage/test_sliding_sync_tables.py b/tests/storage/test_sliding_sync_tables.py index 23a0aee25c..01563d4a62 100644 --- a/tests/storage/test_sliding_sync_tables.py +++ b/tests/storage/test_sliding_sync_tables.py @@ -4208,7 +4208,7 @@ class SlidingSyncTablesCatchUpBackgroundUpdatesTestCase(SlidingSyncTablesTestCas # underlying background update to populate these tables is the same as this # catch-up routine, we are going to rely on # `SlidingSyncTablesBackgroundUpdatesTestCase` to cover that logic. - room_id_with_info = self.helper.create_room_as(user1_id, tok=user1_tok) + room_id = self.helper.create_room_as(user1_id, tok=user1_tok) # Get a snapshot of the `sliding_sync_joined_rooms` table before we add some state sliding_sync_joined_rooms_results_before_state = ( @@ -4216,13 +4216,13 @@ class SlidingSyncTablesCatchUpBackgroundUpdatesTestCase(SlidingSyncTablesTestCas ) self.assertIncludes( set(sliding_sync_joined_rooms_results_before_state.keys()), - {room_id_with_info}, + {room_id}, exact=True, ) # Add a room name self.helper.send_state( - room_id_with_info, + room_id, EventTypes.Name, {"name": "my super duper room"}, tok=user1_tok, @@ -4244,16 +4244,16 @@ class SlidingSyncTablesCatchUpBackgroundUpdatesTestCase(SlidingSyncTablesTestCas self.get_success( self.store.db_pool.simple_update( table="sliding_sync_joined_rooms", - keyvalues={"room_id": room_id_with_info}, + keyvalues={"room_id": room_id}, updatevalues={ # Clear the room name "room_name": None, # Reset the `event_stream_ordering` back to the value before the room name "event_stream_ordering": sliding_sync_joined_rooms_results_before_state[ - room_id_with_info + room_id ].event_stream_ordering, }, - desc="sliding_sync_joined_rooms.test_joined_background_update_catch_up", + desc="simulate new events while Synapse was downgraded", ) ) @@ -4283,7 +4283,97 @@ class SlidingSyncTablesCatchUpBackgroundUpdatesTestCase(SlidingSyncTablesTestCas sliding_sync_joined_rooms_results = self._get_sliding_sync_joined_rooms() self.assertIncludes( set(sliding_sync_joined_rooms_results.keys()), - {room_id_with_info}, + {room_id}, + exact=True, + ) + + def test_joined_background_update_catch_up_no_rooms(self) -> None: + """ + Test that if you start your homeserver with no rooms on a Synapse version that + supports the sliding sync tables and the historical background update completes + (because no rooms to process), then Synapse is downgraded and new rooms are + created/joined; when Synapse is upgraded, the rooms will be processed catch-up + routine is run. + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + + # Instead of testing with various levels of room state that should appear in the + # table, we're only using one room to keep this test simple. Because the + # underlying background update to populate these tables is the same as this + # catch-up routine, we are going to rely on + # `SlidingSyncTablesBackgroundUpdatesTestCase` to cover that logic. + room_id = self.helper.create_room_as(user1_id, tok=user1_tok) + + # Get a snapshot of the `sliding_sync_joined_rooms` table before we add some state + sliding_sync_joined_rooms_results_before_state = ( + self._get_sliding_sync_joined_rooms() + ) + self.assertIncludes( + set(sliding_sync_joined_rooms_results_before_state.keys()), + {room_id}, + exact=True, + ) + + # Make sure all of the background updates have finished before we start the + # catch-up. Even though it should work fine if the other background update is + # still running, we want to see the catch-up routine restore the progress + # correctly. + # + # We also don't want the normal background update messing with our results so we + # run this before we do our manual database clean-up to simulate room being + # created while Synapse was downgraded. + self.wait_for_background_updates() + + # Clean-up the `sliding_sync_joined_rooms` table as if the the room never made + # it into the table. This is to simulate the room being created while Synapse + # was downgraded. + self.get_success( + self.store.db_pool.simple_delete_many( + table="sliding_sync_joined_rooms", + column="room_id", + iterable=(room_id,), + keyvalues={}, + desc="simulate room being created while Synapse was downgraded", + ) + ) + + # We shouldn't find anything in the table because we just deleted them in + # preparation for the test. + sliding_sync_joined_rooms_results = self._get_sliding_sync_joined_rooms() + self.assertIncludes( + set(sliding_sync_joined_rooms_results.keys()), + set(), + exact=True, + ) + + # The function under test. It should clear out stale data and start the + # background update to catch-up on the missing data. + self.get_success( + self.store.db_pool.runInteraction( + "_resolve_stale_data_in_sliding_sync_joined_rooms_table", + _resolve_stale_data_in_sliding_sync_joined_rooms_table, + ) + ) + + # We still shouldn't find any data yet + sliding_sync_joined_rooms_results = self._get_sliding_sync_joined_rooms() + self.assertIncludes( + set(sliding_sync_joined_rooms_results.keys()), + set(), + exact=True, + ) + + # Wait for the catch-up background update to finish + self.store.db_pool.updates._all_done = False + self.wait_for_background_updates() + + # Ensure that the table is populated correctly after the catch-up background + # update finishes + sliding_sync_joined_rooms_results = self._get_sliding_sync_joined_rooms() + self.assertIncludes( + set(sliding_sync_joined_rooms_results.keys()), + {room_id}, exact=True, ) From e5e7269998fe16b4125f122b716956600a5384d5 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 27 Aug 2024 18:49:53 -0500 Subject: [PATCH 15/28] Add more tests --- tests/storage/test_sliding_sync_tables.py | 184 +++++++++++++++++++++- 1 file changed, 182 insertions(+), 2 deletions(-) diff --git a/tests/storage/test_sliding_sync_tables.py b/tests/storage/test_sliding_sync_tables.py index 01563d4a62..34c2ed7041 100644 --- a/tests/storage/test_sliding_sync_tables.py +++ b/tests/storage/test_sliding_sync_tables.py @@ -4194,7 +4194,74 @@ class SlidingSyncTablesCatchUpBackgroundUpdatesTestCase(SlidingSyncTablesTestCas https://github.com/element-hq/synapse/issues/TODO) """ - def test_joined_background_update_catch_up(self) -> None: + def test_joined_background_update_catch_up_new_room(self) -> None: + """ + Test that new rooms while Synapse is downgraded (making + `sliding_sync_joined_rooms` stale) will be caught when Synapse is upgraded and + the catch-up routine is run. + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + + # Instead of testing with various levels of room state that should appear in the + # table, we're only using one room to keep this test simple. Because the + # underlying background update to populate these tables is the same as this + # catch-up routine, we are going to rely on + # `SlidingSyncTablesBackgroundUpdatesTestCase` to cover that logic. + room_id = self.helper.create_room_as(user1_id, tok=user1_tok) + + # Make sure all of the background updates have finished before we start the + # catch-up. Even though it should work fine if the other background update is + # still running, we want to see the catch-up routine restore the progress + # correctly. + # + # We also don't want the normal background update messing with our results so we + # run this before we do our manual database clean-up to simulate new events + # being sent while Synapse was downgraded. + self.wait_for_background_updates() + + # Clean-up the `sliding_sync_joined_rooms` table as if the the room never made + # it into the table. This is to simulate the a new room while Synapse was + # downgraded. + self.get_success( + self.store.db_pool.simple_delete( + table="sliding_sync_joined_rooms", + keyvalues={"room_id": room_id}, + desc="simulate new room while Synapse was downgraded", + ) + ) + + # The function under test. It should clear out stale data and start the + # background update to catch-up on the missing data. + self.get_success( + self.store.db_pool.runInteraction( + "_resolve_stale_data_in_sliding_sync_joined_rooms_table", + _resolve_stale_data_in_sliding_sync_joined_rooms_table, + ) + ) + + # We shouldn't see any new data yet + sliding_sync_joined_rooms_results = self._get_sliding_sync_joined_rooms() + self.assertIncludes( + set(sliding_sync_joined_rooms_results.keys()), + set(), + exact=True, + ) + + # Wait for the catch-up background update to finish + self.store.db_pool.updates._all_done = False + self.wait_for_background_updates() + + # Ensure that the table is populated correctly after the catch-up background + # update finishes + sliding_sync_joined_rooms_results = self._get_sliding_sync_joined_rooms() + self.assertIncludes( + set(sliding_sync_joined_rooms_results.keys()), + {room_id}, + exact=True, + ) + + def test_joined_background_update_catch_up_room_state_change(self) -> None: """ Test that new events while Synapse is downgraded (making `sliding_sync_joined_rooms` stale) will be caught when Synapse is upgraded and @@ -4377,7 +4444,120 @@ class SlidingSyncTablesCatchUpBackgroundUpdatesTestCase(SlidingSyncTablesTestCas exact=True, ) - def test_membership_snapshots_background_update_catch_up(self) -> None: + def test_membership_snapshots_background_update_catch_up_new_membership( + self, + ) -> None: + """ + Test that completely new membership while Synapse is downgraded (making + `sliding_sync_membership_snapshots` stale) will be caught when Synapse is + upgraded and the catch-up routine is run. + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + user2_id = self.register_user("user2", "pass") + user2_tok = self.login(user2_id, "pass") + + # Instead of testing with various levels of room state that should appear in the + # table, we're only using one room to keep this test simple. Because the + # underlying background update to populate these tables is the same as this + # catch-up routine, we are going to rely on + # `SlidingSyncTablesBackgroundUpdatesTestCase` to cover that logic. + room_id = self.helper.create_room_as(user1_id, tok=user1_tok) + # User2 joins the room + self.helper.join(room_id, user2_id, tok=user2_tok) + + # Make sure all of the background updates have finished before we start the + # catch-up. Even though it should work fine if the other background update is + # still running, we want to see the catch-up routine restore the progress + # correctly. + # + # We also don't want the normal background update messing with our results so we + # run this before we do our manual database clean-up to simulate new events + # being sent while Synapse was downgraded. + self.wait_for_background_updates() + + # Clean-up the `sliding_sync_membership_snapshots` table as if the user2 + # membership never made it into the table. This is to simulate a membership + # change while Synapse was downgraded. + num_deleted = self.get_success( + self.store.db_pool.simple_delete( + table="sliding_sync_membership_snapshots", + keyvalues={"room_id": room_id, "user_id": user2_id}, + desc="simulate new membership while Synapse was downgraded", + ) + ) + self.assertGreater( + num_deleted, + 0, + f"Expected to delete one row but found none for ({room_id}, {user2_id})", + ) + + # We shouldn't find the user2 membership in the table because we just deleted it + # in preparation for the test. + sliding_sync_membership_snapshots_results = ( + self._get_sliding_sync_membership_snapshots() + ) + self.assertIncludes( + set(sliding_sync_membership_snapshots_results.keys()), + { + (room_id, user1_id), + }, + exact=True, + ) + + # The function under test. It should clear out stale data and start the + # background update to catch-up on the missing data. + self.get_success( + self.store.db_pool.runInteraction( + "_resolve_stale_data_in_sliding_sync_joined_rooms_table", + _resolve_stale_data_in_sliding_sync_joined_rooms_table, + ) + ) + + # We still shouldn't find any data yet + sliding_sync_membership_snapshots_results = ( + self._get_sliding_sync_membership_snapshots() + ) + self.assertIncludes( + set(sliding_sync_membership_snapshots_results.keys()), + { + (room_id, user1_id), + }, + exact=True, + ) + + # Wait for the catch-up background update to finish + self.store.db_pool.updates._all_done = False + self.wait_for_background_updates() + + # Ensure that the table is populated correctly after the catch-up background + # update finishes + sliding_sync_membership_snapshots_results = ( + self._get_sliding_sync_membership_snapshots() + ) + self.assertIncludes( + set(sliding_sync_membership_snapshots_results.keys()), + { + (room_id, user1_id), + (room_id, user2_id), + }, + exact=True, + ) + + def test_membership_snapshots_background_update_catch_up_membership_change( + self, + ) -> None: + """ + Test that membership changes while Synapse is downgraded (making + `sliding_sync_membership_snapshots` stale) will be caught when Synapse is upgraded and + the catch-up routine is run. + """ + TODO + + def test_membership_snapshots_background_update_catch_up_no_membership( + self, + ) -> None: """ TODO """ + TODO From 85a60c3132a2651463d80f5eec3b2ca3402d6575 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 27 Aug 2024 19:27:24 -0500 Subject: [PATCH 16/28] More tests --- synapse/storage/prepare_database.py | 10 +- tests/storage/test_sliding_sync_tables.py | 156 ++++++++++++++++++++-- 2 files changed, 153 insertions(+), 13 deletions(-) diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index 31f99782ee..ccdce90908 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -647,10 +647,11 @@ def _resolve_stale_data_in_sliding_sync_joined_rooms_table( txn.execute( """ - SELECT DISTINCT(room_id) + SELECT room_id FROM events WHERE stream_ordering > ? - ORDER BY stream_ordering DESC + GROUP BY room_id + ORDER BY MAX(stream_ordering) ASC """, (max_stream_ordering_sliding_sync_joined_rooms_table,), ) @@ -740,10 +741,10 @@ def _resolve_stale_data_in_sliding_sync_membership_snapshots_table( # This only picks up changes to memberships. txn.execute( """ - SELECT DISTINCT(user_id, room_id) + SELECT user_id, room_id FROM local_current_membership WHERE event_stream_ordering > ? - ORDER BY event_stream_ordering DESC + ORDER BY event_stream_ordering ASC """, (max_stream_ordering_sliding_sync_membership_snapshots_table,), ) @@ -781,6 +782,7 @@ def _resolve_stale_data_in_sliding_sync_membership_snapshots_table( max_stream_ordering_sliding_sync_membership_snapshots_table ) + logger.info("asdf insert catch-up bg update progress_json %s", progress_json) DatabasePool.simple_upsert_txn_native_upsert( txn, table="background_updates", diff --git a/tests/storage/test_sliding_sync_tables.py b/tests/storage/test_sliding_sync_tables.py index 34c2ed7041..e1b5c1325e 100644 --- a/tests/storage/test_sliding_sync_tables.py +++ b/tests/storage/test_sliding_sync_tables.py @@ -36,6 +36,7 @@ from synapse.storage.databases.main.events import DeltaState from synapse.storage.databases.main.events_bg_updates import _BackgroundUpdates from synapse.storage.prepare_database import ( _resolve_stale_data_in_sliding_sync_joined_rooms_table, + _resolve_stale_data_in_sliding_sync_membership_snapshots_table, ) from synapse.util import Clock @@ -4466,6 +4467,19 @@ class SlidingSyncTablesCatchUpBackgroundUpdatesTestCase(SlidingSyncTablesTestCas # User2 joins the room self.helper.join(room_id, user2_id, tok=user2_tok) + # Both users are joined to the room + sliding_sync_membership_snapshots_results = ( + self._get_sliding_sync_membership_snapshots() + ) + self.assertIncludes( + set(sliding_sync_membership_snapshots_results.keys()), + { + (room_id, user1_id), + (room_id, user2_id), + }, + exact=True, + ) + # Make sure all of the background updates have finished before we start the # catch-up. Even though it should work fine if the other background update is # still running, we want to see the catch-up routine restore the progress @@ -4479,18 +4493,13 @@ class SlidingSyncTablesCatchUpBackgroundUpdatesTestCase(SlidingSyncTablesTestCas # Clean-up the `sliding_sync_membership_snapshots` table as if the user2 # membership never made it into the table. This is to simulate a membership # change while Synapse was downgraded. - num_deleted = self.get_success( + self.get_success( self.store.db_pool.simple_delete( table="sliding_sync_membership_snapshots", keyvalues={"room_id": room_id, "user_id": user2_id}, desc="simulate new membership while Synapse was downgraded", ) ) - self.assertGreater( - num_deleted, - 0, - f"Expected to delete one row but found none for ({room_id}, {user2_id})", - ) # We shouldn't find the user2 membership in the table because we just deleted it # in preparation for the test. @@ -4509,8 +4518,8 @@ class SlidingSyncTablesCatchUpBackgroundUpdatesTestCase(SlidingSyncTablesTestCas # background update to catch-up on the missing data. self.get_success( self.store.db_pool.runInteraction( - "_resolve_stale_data_in_sliding_sync_joined_rooms_table", - _resolve_stale_data_in_sliding_sync_joined_rooms_table, + "_resolve_stale_data_in_sliding_sync_membership_snapshots_table", + _resolve_stale_data_in_sliding_sync_membership_snapshots_table, ) ) @@ -4552,7 +4561,136 @@ class SlidingSyncTablesCatchUpBackgroundUpdatesTestCase(SlidingSyncTablesTestCas `sliding_sync_membership_snapshots` stale) will be caught when Synapse is upgraded and the catch-up routine is run. """ - TODO + + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + user2_id = self.register_user("user2", "pass") + user2_tok = self.login(user2_id, "pass") + + # Instead of testing with various levels of room state that should appear in the + # table, we're only using one room to keep this test simple. Because the + # underlying background update to populate these tables is the same as this + # catch-up routine, we are going to rely on + # `SlidingSyncTablesBackgroundUpdatesTestCase` to cover that logic. + room_id = self.helper.create_room_as(user1_id, tok=user1_tok) + # User2 joins the room + self.helper.join(room_id, user2_id, tok=user2_tok) + + # Both users are joined to the room + sliding_sync_membership_snapshots_results_before_membership_changes = ( + self._get_sliding_sync_membership_snapshots() + ) + self.assertIncludes( + set( + sliding_sync_membership_snapshots_results_before_membership_changes.keys() + ), + { + (room_id, user1_id), + (room_id, user2_id), + }, + exact=True, + ) + + # User2 leaves the room + self.helper.leave(room_id, user2_id, tok=user2_tok) + + # Make sure all of the background updates have finished before we start the + # catch-up. Even though it should work fine if the other background update is + # still running, we want to see the catch-up routine restore the progress + # correctly. + # + # We also don't want the normal background update messing with our results so we + # run this before we do our manual database clean-up to simulate new events + # being sent while Synapse was downgraded. + self.wait_for_background_updates() + + # Rollback the `sliding_sync_membership_snapshots` table as if the user2 + # membership never made it into the table. This is to simulate a membership + # change while Synapse was downgraded. + self.get_success( + self.store.db_pool.simple_update( + table="sliding_sync_membership_snapshots", + keyvalues={"room_id": room_id, "user_id": user2_id}, + updatevalues={ + # Reset everything back to the value before user2 left the room + "membership": sliding_sync_membership_snapshots_results_before_membership_changes[ + (room_id, user2_id) + ].membership, + "membership_event_id": sliding_sync_membership_snapshots_results_before_membership_changes[ + (room_id, user2_id) + ].membership_event_id, + "event_stream_ordering": sliding_sync_membership_snapshots_results_before_membership_changes[ + (room_id, user2_id) + ].event_stream_ordering, + }, + desc="simulate membership change while Synapse was downgraded", + ) + ) + + # We should see user2 still joined to the room because we made that change in + # preparation for the test. + sliding_sync_membership_snapshots_results = ( + self._get_sliding_sync_membership_snapshots() + ) + self.assertIncludes( + set(sliding_sync_membership_snapshots_results.keys()), + { + (room_id, user1_id), + (room_id, user2_id), + }, + exact=True, + ) + self.assertEqual( + sliding_sync_membership_snapshots_results.get((room_id, user1_id)), + sliding_sync_membership_snapshots_results_before_membership_changes[ + (room_id, user1_id) + ], + ) + self.assertEqual( + sliding_sync_membership_snapshots_results.get((room_id, user2_id)), + sliding_sync_membership_snapshots_results_before_membership_changes[ + (room_id, user2_id) + ], + ) + + # The function under test. It should clear out stale data and start the + # background update to catch-up on the missing data. + self.get_success( + self.store.db_pool.runInteraction( + "_resolve_stale_data_in_sliding_sync_membership_snapshots_table", + _resolve_stale_data_in_sliding_sync_membership_snapshots_table, + ) + ) + + # Ensure that the stale data is deleted from the table + sliding_sync_membership_snapshots_results = ( + self._get_sliding_sync_membership_snapshots() + ) + self.assertIncludes( + set(sliding_sync_membership_snapshots_results.keys()), + { + (room_id, user1_id), + }, + exact=True, + ) + + # Wait for the catch-up background update to finish + self.store.db_pool.updates._all_done = False + self.wait_for_background_updates() + + # Ensure that the table is populated correctly after the catch-up background + # update finishes + sliding_sync_membership_snapshots_results = ( + self._get_sliding_sync_membership_snapshots() + ) + self.assertIncludes( + set(sliding_sync_membership_snapshots_results.keys()), + { + (room_id, user1_id), + (room_id, user2_id), + }, + exact=True, + ) def test_membership_snapshots_background_update_catch_up_no_membership( self, From 56a4c0ba6ea7c8fc0851b2192f8fb6c7fa752953 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 27 Aug 2024 19:34:16 -0500 Subject: [PATCH 17/28] Round out tests --- tests/storage/test_sliding_sync_tables.py | 102 +++++++++++++++++++--- 1 file changed, 89 insertions(+), 13 deletions(-) diff --git a/tests/storage/test_sliding_sync_tables.py b/tests/storage/test_sliding_sync_tables.py index e1b5c1325e..2654decb0c 100644 --- a/tests/storage/test_sliding_sync_tables.py +++ b/tests/storage/test_sliding_sync_tables.py @@ -4373,16 +4373,6 @@ class SlidingSyncTablesCatchUpBackgroundUpdatesTestCase(SlidingSyncTablesTestCas # `SlidingSyncTablesBackgroundUpdatesTestCase` to cover that logic. room_id = self.helper.create_room_as(user1_id, tok=user1_tok) - # Get a snapshot of the `sliding_sync_joined_rooms` table before we add some state - sliding_sync_joined_rooms_results_before_state = ( - self._get_sliding_sync_joined_rooms() - ) - self.assertIncludes( - set(sliding_sync_joined_rooms_results_before_state.keys()), - {room_id}, - exact=True, - ) - # Make sure all of the background updates have finished before we start the # catch-up. Even though it should work fine if the other background update is # still running, we want to see the catch-up routine restore the progress @@ -4561,7 +4551,6 @@ class SlidingSyncTablesCatchUpBackgroundUpdatesTestCase(SlidingSyncTablesTestCas `sliding_sync_membership_snapshots` stale) will be caught when Synapse is upgraded and the catch-up routine is run. """ - user1_id = self.register_user("user1", "pass") user1_tok = self.login(user1_id, "pass") user2_id = self.register_user("user2", "pass") @@ -4696,6 +4685,93 @@ class SlidingSyncTablesCatchUpBackgroundUpdatesTestCase(SlidingSyncTablesTestCas self, ) -> None: """ - TODO + Test that if you start your homeserver with no rooms on a Synapse version that + supports the sliding sync tables and the historical background update completes + (because no rooms/membership to process), then Synapse is downgraded and new + rooms are created/joined; when Synapse is upgraded, the rooms will be processed + catch-up routine is run. """ - TODO + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + user2_id = self.register_user("user2", "pass") + user2_tok = self.login(user2_id, "pass") + + # Instead of testing with various levels of room state that should appear in the + # table, we're only using one room to keep this test simple. Because the + # underlying background update to populate these tables is the same as this + # catch-up routine, we are going to rely on + # `SlidingSyncTablesBackgroundUpdatesTestCase` to cover that logic. + room_id = self.helper.create_room_as(user1_id, tok=user1_tok) + # User2 joins the room + self.helper.join(room_id, user2_id, tok=user2_tok) + + # Make sure all of the background updates have finished before we start the + # catch-up. Even though it should work fine if the other background update is + # still running, we want to see the catch-up routine restore the progress + # correctly. + # + # We also don't want the normal background update messing with our results so we + # run this before we do our manual database clean-up to simulate new events + # being sent while Synapse was downgraded. + self.wait_for_background_updates() + + # Rollback the `sliding_sync_membership_snapshots` table as if the user2 + # membership never made it into the table. This is to simulate a membership + # change while Synapse was downgraded. + self.get_success( + self.store.db_pool.simple_delete_many( + table="sliding_sync_membership_snapshots", + column="room_id", + iterable=(room_id,), + keyvalues={}, + desc="simulate room being created while Synapse was downgraded", + ) + ) + + # We shouldn't find anything in the table because we just deleted them in + # preparation for the test. + sliding_sync_membership_snapshots_results = ( + self._get_sliding_sync_membership_snapshots() + ) + self.assertIncludes( + set(sliding_sync_membership_snapshots_results.keys()), + set(), + exact=True, + ) + + # The function under test. It should clear out stale data and start the + # background update to catch-up on the missing data. + self.get_success( + self.store.db_pool.runInteraction( + "_resolve_stale_data_in_sliding_sync_membership_snapshots_table", + _resolve_stale_data_in_sliding_sync_membership_snapshots_table, + ) + ) + + # We still shouldn't find any data yet + sliding_sync_membership_snapshots_results = ( + self._get_sliding_sync_membership_snapshots() + ) + self.assertIncludes( + set(sliding_sync_membership_snapshots_results.keys()), + set(), + exact=True, + ) + + # Wait for the catch-up background update to finish + self.store.db_pool.updates._all_done = False + self.wait_for_background_updates() + + # Ensure that the table is populated correctly after the catch-up background + # update finishes + sliding_sync_membership_snapshots_results = ( + self._get_sliding_sync_membership_snapshots() + ) + self.assertIncludes( + set(sliding_sync_membership_snapshots_results.keys()), + { + (room_id, user1_id), + (room_id, user2_id), + }, + exact=True, + ) From 9d08bc21577ab6fdcd9c948ed02bf71c47138507 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 27 Aug 2024 19:35:05 -0500 Subject: [PATCH 18/28] Remove debug logs --- synapse/storage/prepare_database.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index ccdce90908..034e6f6ccd 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -782,7 +782,6 @@ def _resolve_stale_data_in_sliding_sync_membership_snapshots_table( max_stream_ordering_sliding_sync_membership_snapshots_table ) - logger.info("asdf insert catch-up bg update progress_json %s", progress_json) DatabasePool.simple_upsert_txn_native_upsert( txn, table="background_updates", From a507f152c907681431c96e8eb4ff8cce177177e1 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 27 Aug 2024 19:45:50 -0500 Subject: [PATCH 19/28] Use `stream_id` of some point before we fetch the current state This is simpler and some rooms are so old that they don't have `current_state_delta_stream` yet. It's easier if we just get a general max `stream_id` of the whole table than the max `stream_id` for the specific room anyway. Thanks @erikjohnston --- synapse/storage/databases/main/events.py | 21 ++------------ .../databases/main/events_bg_updates.py | 29 ++++++++----------- 2 files changed, 14 insertions(+), 36 deletions(-) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index fa41d33920..f8d176d133 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -1849,7 +1849,7 @@ class PersistEventsStore: @classmethod def _get_relevant_sliding_sync_current_state_event_ids_txn( cls, txn: LoggingTransaction, room_id: str - ) -> Tuple[MutableStateMap[str], int]: + ) -> MutableStateMap[str]: """ Fetch the current state event IDs for the relevant (to the `sliding_sync_joined_rooms` table) state types for the given room. @@ -1888,24 +1888,7 @@ class PersistEventsStore: (event_type, state_key): event_id for event_id, event_type, state_key in txn } - txn.execute( - """ - SELECT stream_id - FROM current_state_delta_stream - WHERE - room_id = ? - ORDER BY stream_id DESC - LIMIT 1 - """, - (room_id,), - ) - row = txn.fetchone() - # If we're able to fetch the `current_state_events` above, we should have rows - # in `current_state_delta_stream` as well. - assert row, "Failed to fetch the `last_current_state_delta_stream_id`" - last_current_state_delta_stream_id = row[0] - - return current_state_map, last_current_state_delta_stream_id + return current_state_map @classmethod def _get_sliding_sync_insert_values_from_state_map( diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 52b4450bbc..38a786c001 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -123,10 +123,6 @@ class _JoinedRoomStreamOrderingUpdate: most_recent_event_stream_ordering: int # The most recent event `bump_stamp` for the room most_recent_bump_stamp: Optional[int] - # The `stream_ordering` in the `current_state_delta_stream` that we got the state - # values from. We can use this to check if the current state has been updated since - # we last checked. - last_current_state_delta_stream_id: int class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseStore): @@ -1622,7 +1618,7 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS # Map from room_id to insert/update state values in the `sliding_sync_joined_rooms` table. joined_room_updates: Dict[str, SlidingSyncStateInsertValues] = {} - # Map from room_id to stream_ordering/bump_stamp/last_current_state_delta_stream_id values + # Map from room_id to stream_ordering/bump_stamp, etc values joined_room_stream_ordering_updates: Dict[ str, _JoinedRoomStreamOrderingUpdate ] = {} @@ -1632,15 +1628,18 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS # `event_stream_ordering` order *ascending* to save our progress position # correctly if we need to exit early. room_id_to_progress_marker_map: OrderedDict[str, int] = OrderedDict() + # As long as we get this value before we fetch the current state, we can use it + # to check if something has changed since that point. + most_recent_current_state_delta_stream_id = ( + await self.get_max_stream_id_in_current_state_deltas() + ) for room_id, progress_event_stream_ordering in rooms_to_update: room_id_to_progress_marker_map[room_id] = progress_event_stream_ordering - current_state_ids_map, last_current_state_delta_stream_id = ( - await self.db_pool.runInteraction( - "_sliding_sync_joined_rooms_bg_update._get_relevant_sliding_sync_current_state_event_ids_txn", - PersistEventsStore._get_relevant_sliding_sync_current_state_event_ids_txn, - room_id, - ) + current_state_ids_map = await self.db_pool.runInteraction( + "_sliding_sync_joined_rooms_bg_update._get_relevant_sliding_sync_current_state_event_ids_txn", + PersistEventsStore._get_relevant_sliding_sync_current_state_event_ids_txn, + room_id, ) # We're iterating over rooms pulled from the current_state_events table # so we should have some current state for each room @@ -1694,7 +1693,6 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS _JoinedRoomStreamOrderingUpdate( most_recent_event_stream_ordering=most_recent_event_stream_ordering, most_recent_bump_stamp=most_recent_bump_stamp, - last_current_state_delta_stream_id=last_current_state_delta_stream_id, ) ) @@ -1718,9 +1716,6 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS joined_room_update.most_recent_event_stream_ordering ) bump_stamp = joined_room_update.most_recent_bump_stamp - last_current_state_delta_stream_id = ( - joined_room_update.last_current_state_delta_stream_id - ) # Check if the current state has been updated since we gathered it state_deltas_since_we_gathered_current_state = ( @@ -1728,7 +1723,7 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS txn, room_id, from_token=RoomStreamToken( - stream=last_current_state_delta_stream_id + stream=most_recent_current_state_delta_stream_id ), to_token=None, ) @@ -1763,7 +1758,7 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS # Since we partially update the `sliding_sync_joined_rooms` as new state # is sent, we need to update the state fields `ON CONFLICT`. We just # have to be careful we're not overwriting it with stale data (see - # `last_current_state_delta_stream_id` check above). + # `most_recent_current_state_delta_stream_id` check above). # self.db_pool.simple_upsert_txn( txn, From 94e1a5468783d4ba2b94793206deac047fe3c236 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 27 Aug 2024 20:01:55 -0500 Subject: [PATCH 20/28] `get_events(...)` will omit events from unknown room versions Thanks @erikjohnston --- .../databases/main/events_bg_updates.py | 24 +++++++++++++++++++ .../storage/databases/main/events_worker.py | 6 +++++ 2 files changed, 30 insertions(+) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 38a786c001..566589ed07 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1650,8 +1650,16 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS current_state_map: StateMap[EventBase] = { state_key: fetched_events[event_id] for state_key, event_id in current_state_ids_map.items() + # `get_events(...)` will filter out events for unknown room versions + if event_id in fetched_events } + # Can happen for unknown room versions (old room versions that aren't known + # anymore) since `get_events(...)` will filter out events for unknown room + # versions + if not current_state_map: + continue + state_insert_values = ( PersistEventsStore._get_sliding_sync_insert_values_from_state_map( current_state_map @@ -1929,8 +1937,16 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS current_state_map: StateMap[EventBase] = { state_key: fetched_events[event_id] for state_key, event_id in current_state_ids_map.items() + # `get_events(...)` will filter out events for unknown room versions + if event_id in fetched_events } + # Can happen for unknown room versions (old room versions that aren't known + # anymore) since `get_events(...)` will filter out events for unknown room + # versions + if not current_state_map: + continue + state_insert_values = ( PersistEventsStore._get_sliding_sync_insert_values_from_state_map( current_state_map @@ -2006,8 +2022,16 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS state_map: StateMap[EventBase] = { state_key: fetched_events[event_id] for state_key, event_id in state_ids_map.items() + # `get_events(...)` will filter out events for unknown room versions + if event_id in fetched_events } + # Can happen for unknown room versions (old room versions that aren't known + # anymore) since `get_events(...)` will filter out events for unknown room + # versions + if not state_map: + continue + state_insert_values = ( PersistEventsStore._get_sliding_sync_insert_values_from_state_map( state_map diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index cf24d84554..6079cc4a52 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -457,6 +457,8 @@ class EventsWorkerStore(SQLBaseStore): ) -> Optional[EventBase]: """Get an event from the database by event_id. + Events for unknown room versions will also be filtered out. + Args: event_id: The event_id of the event to fetch @@ -513,6 +515,8 @@ class EventsWorkerStore(SQLBaseStore): Unknown events will be omitted from the response. + Events for unknown room versions will also be filtered out. + Args: event_ids: The event_ids of the events to fetch @@ -555,6 +559,8 @@ class EventsWorkerStore(SQLBaseStore): Unknown events will be omitted from the response. + Events for unknown room versions will also be filtered out. + Args: event_ids: The event_ids of the events to fetch From 53b7309f6c1d882003d34a5555e6c1f12c61c854 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 27 Aug 2024 20:48:56 -0500 Subject: [PATCH 21/28] Add `sliding_sync_joined_rooms_to_recalculate` table --- .../databases/main/events_bg_updates.py | 47 +++++++++++++++++++ .../delta/87/01_sliding_sync_memberships.sql | 31 +++++++++++- 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 566589ed07..c520faa9e7 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -89,6 +89,12 @@ class _BackgroundUpdates: EVENTS_JUMP_TO_DATE_INDEX = "events_jump_to_date_index" + SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE = ( + "sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update" + ) + SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE = ( + "sliding_sync_index_joined_rooms_to_recalculate_table_bg_update" + ) SLIDING_SYNC_JOINED_ROOMS_BG_UPDATE = "sliding_sync_joined_rooms_bg_update" SLIDING_SYNC_MEMBERSHIP_SNAPSHOTS_BG_UPDATE = ( "sliding_sync_membership_snapshots_bg_update" @@ -307,6 +313,19 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS where_clause="NOT outlier", ) + # Handle background updates for Sliding Sync tables + # + self.db_pool.updates.register_background_update_handler( + _BackgroundUpdates.SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, + self._sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update, + ) + self.db_pool.updates.register_background_index_update( + _BackgroundUpdates.SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, + index_name="sliding_sync_joined_rooms_to_recalculate_room_id_idx", + table="sliding_sync_joined_rooms", + columns=["room_id"], + unique=True, + ) # Add some background updates to populate the sliding sync tables self.db_pool.updates.register_background_update_handler( _BackgroundUpdates.SLIDING_SYNC_JOINED_ROOMS_BG_UPDATE, @@ -1555,6 +1574,34 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS return batch_size + async def _sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update( + self, _progress: JsonDict, _batch_size: int + ) -> int: + """ + Prefill `sliding_sync_joined_rooms_to_recalculate` table with all rooms we know about already. + """ + + def _txn(txn: LoggingTransaction) -> None: + # We do this as one big bulk insert. This has been tested on a bigger + # homeserver with ~10M rooms and took 11s. There is potential for this to + # starve disk usage while this goes on. + txn.execute( + """ + INSERT INTO sliding_sync_joined_rooms_to_recalculate (room_id) SELECT room_id FROM rooms; + """, + ) + + await self.db_pool.runInteraction( + "_sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update", + _txn, + ) + + # Background update is done. + await self.db_pool.updates._end_background_update( + _BackgroundUpdates.SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE + ) + return 0 + async def _sliding_sync_joined_rooms_bg_update( self, progress: JsonDict, batch_size: int ) -> int: diff --git a/synapse/storage/schema/main/delta/87/01_sliding_sync_memberships.sql b/synapse/storage/schema/main/delta/87/01_sliding_sync_memberships.sql index 8d7607c15f..6ab1897230 100644 --- a/synapse/storage/schema/main/delta/87/01_sliding_sync_memberships.sql +++ b/synapse/storage/schema/main/delta/87/01_sliding_sync_memberships.sql @@ -11,6 +11,18 @@ -- See the GNU Affero General Public License for more details: -- . +-- This table is a list/queue used to keep track of which rooms need to be inserted into +-- `sliding_sync_joined_rooms`. We do this to avoid reading from `current_state_events` +-- during the background update to populate `sliding_sync_joined_rooms` which works but +-- it takes a lot of work for the database to grab `DISTINCT` room_ids given how many +-- state events there are for each room. +-- +-- This table doesn't have any indexes at this point. We add the indexes in a separate +-- step to avoid the extra calculations during the bulk one-shot prefill insert. +CREATE TABLE IF NOT EXISTS sliding_sync_joined_rooms_to_recalculate( + room_id TEXT NOT NULL REFERENCES rooms(room_id) +); + -- A table for storing room meta data (current state relevant to sliding sync) that the -- local server is still participating in (someone local is joined to the room). -- @@ -127,8 +139,23 @@ CREATE INDEX IF NOT EXISTS sliding_sync_membership_snapshots_user_id ON sliding_ CREATE UNIQUE INDEX IF NOT EXISTS sliding_sync_membership_snapshots_event_stream_ordering ON sliding_sync_membership_snapshots(event_stream_ordering); --- Add some background updates to populate the new tables +-- Add a series of background updates to populate the new `sliding_sync_joined_rooms` table: +-- +-- 1. Add a background update to prefill `sliding_sync_joined_rooms_to_recalculate`. +-- We do a one-shot bulk insert from the `rooms` table to prefill. +-- 2. Add a background update to add indexes to the +-- `sliding_sync_joined_rooms_to_recalculate` table after the one-shot bulk insert. +-- We add the index in a separate step after to avoid the extra calculations during +-- the one-shot bulk insert. +-- 3. Add a background update to populate the new `sliding_sync_joined_rooms` table +-- INSERT INTO background_updates (ordering, update_name, progress_json) VALUES - (8701, 'sliding_sync_joined_rooms_bg_update', '{}'); + (8701, 'sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update', '{}'); +INSERT INTO background_updates (ordering, update_name, progress_json, depends_on) VALUES + (8701, 'sliding_sync_index_joined_rooms_to_recalculate_table_bg_update', '{}', 'sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update'); +INSERT INTO background_updates (ordering, update_name, progress_json, depends_on) VALUES + (8701, 'sliding_sync_joined_rooms_bg_update', '{}', 'sliding_sync_index_joined_rooms_to_calculate_table_bg_update'); + +-- Add a background updates to populate the new `sliding_sync_membership_snapshots` table INSERT INTO background_updates (ordering, update_name, progress_json) VALUES (8701, 'sliding_sync_membership_snapshots_bg_update', '{}'); From 8468401a97ea05380295ec123d151c392014cb43 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 28 Aug 2024 00:42:14 -0500 Subject: [PATCH 22/28] Adapt to using `sliding_sync_joined_rooms_to_recalculate` table --- synapse/storage/database.py | 2 +- .../databases/main/events_bg_updates.py | 137 ++++++++---------- synapse/storage/prepare_database.py | 52 ++++++- .../delta/87/01_sliding_sync_memberships.sql | 2 +- tests/storage/test__base.py | 18 +++ tests/storage/test_sliding_sync_tables.py | 42 +++++- 6 files changed, 165 insertions(+), 88 deletions(-) diff --git a/synapse/storage/database.py b/synapse/storage/database.py index da50fd7f83..d666039120 100644 --- a/synapse/storage/database.py +++ b/synapse/storage/database.py @@ -1536,8 +1536,8 @@ class DatabasePool: self.simple_upsert_txn_emulated(txn, table, _keys, _vals, lock=False) + @staticmethod def simple_upsert_many_txn_native_upsert( - self, txn: LoggingTransaction, table: str, key_names: Collection[str], diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index c520faa9e7..85014719ae 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -20,7 +20,6 @@ # import logging -from collections import OrderedDict from typing import TYPE_CHECKING, Dict, List, Optional, Set, Tuple, cast import attr @@ -322,7 +321,7 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS self.db_pool.updates.register_background_index_update( _BackgroundUpdates.SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, index_name="sliding_sync_joined_rooms_to_recalculate_room_id_idx", - table="sliding_sync_joined_rooms", + table="sliding_sync_joined_rooms_to_recalculate", columns=["room_id"], unique=True, ) @@ -1575,21 +1574,43 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS return batch_size async def _sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update( - self, _progress: JsonDict, _batch_size: int + self, progress: JsonDict, _batch_size: int ) -> int: """ Prefill `sliding_sync_joined_rooms_to_recalculate` table with all rooms we know about already. """ + initial_insert = progress.get("initial_insert", False) def _txn(txn: LoggingTransaction) -> None: # We do this as one big bulk insert. This has been tested on a bigger # homeserver with ~10M rooms and took 11s. There is potential for this to # starve disk usage while this goes on. - txn.execute( - """ - INSERT INTO sliding_sync_joined_rooms_to_recalculate (room_id) SELECT room_id FROM rooms; - """, - ) + if initial_insert: + txn.execute( + """ + INSERT INTO sliding_sync_joined_rooms_to_recalculate + (room_id) + SELECT room_id FROM rooms; + """, + ) + else: + # We can only upsert once the unique index has been added to the table + # (see + # `_BackgroundUpdates.SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE`) + # + # We upsert in case we have to run this multiple times. + # + # The `WHERE TRUE` clause is to avoid "Parsing Ambiguity" + txn.execute( + """ + INSERT INTO sliding_sync_joined_rooms_to_recalculate + (room_id) + SELECT room_id FROM rooms WHERE ? + ON CONFLICT (room_id) + DO NOTHING; + """, + (True,), + ) await self.db_pool.runInteraction( "_sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update", @@ -1608,11 +1629,10 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS """ Background update to populate the `sliding_sync_joined_rooms` table. """ - last_event_stream_ordering = progress.get( - "last_event_stream_ordering", -(1 << 31) - ) + # We don't need to fetch any progress state because we just grab the next N + # events in `sliding_sync_joined_rooms_to_recalculate` - def _get_rooms_to_update_txn(txn: LoggingTransaction) -> List[Tuple[str, int]]: + def _get_rooms_to_update_txn(txn: LoggingTransaction) -> List[Tuple[str]]: """ Returns: A list of room ID's to update along with the progress value @@ -1625,30 +1645,16 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS # server is still participating in the room because if we're # `no_longer_in_room`, this table would be cleared out for the given # `room_id`. - # - # Because we're using `event_stream_ordering` as the progress marker, we're - # going to be pulling out the same rooms over and over again but we can - # at-least re-use this background update for the catch-up background - # process as well (see `_resolve_stale_data_in_sliding_sync_tables()`). - # - # It's important to sort by `event_stream_ordering` *ascending* (oldest to - # newest) so that if we see that this background update in progress and want - # to start the catch-up process, we can safely assume that it will - # eventually get to the rooms we want to catch-up on anyway (see - # `_resolve_stale_data_in_sliding_sync_tables()`). txn.execute( """ - SELECT room_id, MAX(event_stream_ordering) - FROM current_state_events - WHERE event_stream_ordering > ? - GROUP BY room_id - ORDER BY MAX(event_stream_ordering) ASC + SELECT room_id + FROM sliding_sync_joined_rooms_to_recalculate LIMIT ? """, - (last_event_stream_ordering, batch_size), + (batch_size,), ) - rooms_to_update_rows = cast(List[Tuple[str, int]], txn.fetchall()) + rooms_to_update_rows = cast(List[Tuple[str]], txn.fetchall()) return rooms_to_update_rows @@ -1669,28 +1675,23 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS joined_room_stream_ordering_updates: Dict[ str, _JoinedRoomStreamOrderingUpdate ] = {} - # Map from room_id to the progress value (event_stream_ordering) - # - # This needs to be an `OrderedDict` because we need to process things in - # `event_stream_ordering` order *ascending* to save our progress position - # correctly if we need to exit early. - room_id_to_progress_marker_map: OrderedDict[str, int] = OrderedDict() # As long as we get this value before we fetch the current state, we can use it # to check if something has changed since that point. most_recent_current_state_delta_stream_id = ( await self.get_max_stream_id_in_current_state_deltas() ) - for room_id, progress_event_stream_ordering in rooms_to_update: - room_id_to_progress_marker_map[room_id] = progress_event_stream_ordering - + for (room_id,) in rooms_to_update: current_state_ids_map = await self.db_pool.runInteraction( "_sliding_sync_joined_rooms_bg_update._get_relevant_sliding_sync_current_state_event_ids_txn", PersistEventsStore._get_relevant_sliding_sync_current_state_event_ids_txn, room_id, ) - # We're iterating over rooms pulled from the current_state_events table - # so we should have some current state for each room - assert current_state_ids_map + + # If we're not joined to the room a) it doesn't belong in the + # `sliding_sync_joined_rooms` table so we should skip and b) we won't have + # any `current_state_events` for the room. + if not current_state_ids_map: + continue fetched_events = await self.get_events(current_state_ids_map.values()) @@ -1701,9 +1702,9 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS if event_id in fetched_events } - # Can happen for unknown room versions (old room versions that aren't known - # anymore) since `get_events(...)` will filter out events for unknown room - # versions + # Even if we are joined to the room, this can happen for unknown room + # versions (old room versions that aren't known anymore) since + # `get_events(...)` will filter out events for unknown room versions if not current_state_map: continue @@ -1754,23 +1755,17 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS def _fill_table_txn(txn: LoggingTransaction) -> None: # Handle updating the `sliding_sync_joined_rooms` table # - last_successful_room_id: Optional[str] = None - # Process the rooms in `event_stream_ordering` order *ascending* so we can - # save our position correctly if we need to exit early. - # `progress_event_stream_ordering` is an `OrderedDict` which remembers - # insertion order (and we inserted in the correct order) so this should be - # the correct thing to do. for ( room_id, - progress_event_stream_ordering, - ) in room_id_to_progress_marker_map.items(): - update_map = joined_room_updates[room_id] - - joined_room_update = joined_room_stream_ordering_updates[room_id] - event_stream_ordering = ( - joined_room_update.most_recent_event_stream_ordering + update_map, + ) in joined_room_updates.items(): + joined_room_stream_ordering_update = ( + joined_room_stream_ordering_updates[room_id] ) - bump_stamp = joined_room_update.most_recent_bump_stamp + event_stream_ordering = ( + joined_room_stream_ordering_update.most_recent_event_stream_ordering + ) + bump_stamp = joined_room_stream_ordering_update.most_recent_bump_stamp # Check if the current state has been updated since we gathered it state_deltas_since_we_gathered_current_state = ( @@ -1790,15 +1785,6 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS state_delta.event_type, state_delta.state_key, ) in SLIDING_SYNC_RELEVANT_STATE_SET: - # Save our progress before we exit early - if last_successful_room_id is not None: - self.db_pool.updates._background_update_progress_txn( - txn, - _BackgroundUpdates.SLIDING_SYNC_JOINED_ROOMS_BG_UPDATE, - { - "last_event_stream_ordering": progress_event_stream_ordering - }, - ) # Raising exception so we can just exit and try again. It would # be hard to resolve this within the transaction because we need # to get full events out that take redactions into account. We @@ -1829,20 +1815,17 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS }, ) - # Keep track of the last successful room_id - last_successful_room_id = room_id + # Now that we've processed the room, we can remove it from the queue + self.db_pool.simple_delete_txn( + txn, + table="sliding_sync_joined_rooms_to_recalculate", + keyvalues={"room_id": room_id}, + ) await self.db_pool.runInteraction( "sliding_sync_joined_rooms_bg_update", _fill_table_txn ) - # Update the progress - _ = room_id_to_progress_marker_map.values() - await self.db_pool.updates._background_update_progress( - _BackgroundUpdates.SLIDING_SYNC_JOINED_ROOMS_BG_UPDATE, - {"last_event_stream_ordering": rooms_to_update[-1][1]}, - ) - return len(rooms_to_update) async def _sliding_sync_membership_snapshots_bg_update( diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index 034e6f6ccd..9e9c27e3b1 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -642,6 +642,7 @@ def _resolve_stale_data_in_sliding_sync_joined_rooms_table( # nothing to clean up row = cast(Optional[Tuple[int]], txn.fetchone()) max_stream_ordering_sliding_sync_joined_rooms_table = None + depends_on = None if row is not None: (max_stream_ordering_sliding_sync_joined_rooms_table,) = row @@ -668,6 +669,7 @@ def _resolve_stale_data_in_sliding_sync_joined_rooms_table( for chunk in batch_iter(room_rows, 1000): # Handle updating the `sliding_sync_joined_rooms` table # + # Clear out the stale data DatabasePool.simple_delete_many_batch_txn( txn, table="sliding_sync_joined_rooms", @@ -675,6 +677,44 @@ def _resolve_stale_data_in_sliding_sync_joined_rooms_table( values=chunk, ) + # Update the `sliding_sync_joined_rooms_to_recalculate` table with the rooms + # that went stale and now need to be recalculated. + # + # FIXME: There is potentially a race where the unique index (added via + # `_BackgroundUpdates.SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE`) + # hasn't been added at this point so we won't be able to upsert + DatabasePool.simple_upsert_many_txn_native_upsert( + txn, + table="sliding_sync_joined_rooms_to_recalculate", + key_names=("room_id",), + key_values=chunk, + value_names=(), + # No value columns, therefore make a blank list so that the following + # zip() works correctly. + value_values=[() for x in range(len(chunk))], + ) + else: + # Re-run the `sliding_sync_joined_rooms_to_recalculate` prefill if there is + # nothing in the `sliding_sync_joined_rooms` table + DatabasePool.simple_upsert_txn_native_upsert( + txn, + table="background_updates", + keyvalues={ + "update_name": _BackgroundUpdates.SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE + }, + values={}, + # Only insert the row if it doesn't already exist. If it already exists, + # we're already working on it + insertion_values={ + "progress_json": "{}", + # Since we're going to upsert, we need to make sure the unique index is in place + "depends_on": _BackgroundUpdates.SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, + }, + ) + depends_on = ( + _BackgroundUpdates.SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE + ) + # Now kick-off the background update to catch-up with what we missed while Synapse # was downgraded. # @@ -682,13 +722,6 @@ def _resolve_stale_data_in_sliding_sync_joined_rooms_table( # `sliding_sync_joined_rooms` table yet. This could happen if someone had zero rooms # on their server (so the normal background update completes), downgrade Synapse # versions, join and create some new rooms, and upgrade again. - # - progress_json: JsonDict = {} - if max_stream_ordering_sliding_sync_joined_rooms_table is not None: - progress_json["last_event_stream_ordering"] = ( - max_stream_ordering_sliding_sync_joined_rooms_table - ) - DatabasePool.simple_upsert_txn_native_upsert( txn, table="background_updates", @@ -699,7 +732,10 @@ def _resolve_stale_data_in_sliding_sync_joined_rooms_table( # Only insert the row if it doesn't already exist. If it already exists, we will # eventually fill in the rows we're trying to populate. insertion_values={ - "progress_json": json_encoder.encode(progress_json), + # Empty progress is expected since it's not used for this background update. + "progress_json": "{}", + # Wait for the prefill to finish + "depends_on": depends_on, }, ) diff --git a/synapse/storage/schema/main/delta/87/01_sliding_sync_memberships.sql b/synapse/storage/schema/main/delta/87/01_sliding_sync_memberships.sql index 6ab1897230..11fb2c4d64 100644 --- a/synapse/storage/schema/main/delta/87/01_sliding_sync_memberships.sql +++ b/synapse/storage/schema/main/delta/87/01_sliding_sync_memberships.sql @@ -150,7 +150,7 @@ CREATE UNIQUE INDEX IF NOT EXISTS sliding_sync_membership_snapshots_event_stream -- 3. Add a background update to populate the new `sliding_sync_joined_rooms` table -- INSERT INTO background_updates (ordering, update_name, progress_json) VALUES - (8701, 'sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update', '{}'); + (8701, 'sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update', '{ "initial_insert": true }'); INSERT INTO background_updates (ordering, update_name, progress_json, depends_on) VALUES (8701, 'sliding_sync_index_joined_rooms_to_recalculate_table_bg_update', '{}', 'sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update'); INSERT INTO background_updates (ordering, update_name, progress_json, depends_on) VALUES diff --git a/tests/storage/test__base.py b/tests/storage/test__base.py index 506d981ce6..49dc973a36 100644 --- a/tests/storage/test__base.py +++ b/tests/storage/test__base.py @@ -112,6 +112,24 @@ class UpdateUpsertManyTests(unittest.HomeserverTestCase): {(1, "user1", "hello"), (2, "user2", "bleb")}, ) + self.get_success( + self.storage.db_pool.runInteraction( + "test", + self.storage.db_pool.simple_upsert_many_txn, + self.table_name, + key_names=key_names, + key_values=[[2, "user2"]], + value_names=[], + value_values=[], + ) + ) + + # Check results are what we expect + self.assertEqual( + set(self._dump_table_to_tuple()), + {(1, "user1", "hello"), (2, "user2", "bleb")}, + ) + def test_simple_update_many(self) -> None: """ simple_update_many performs many updates at once. diff --git a/tests/storage/test_sliding_sync_tables.py b/tests/storage/test_sliding_sync_tables.py index 2654decb0c..300ccd664e 100644 --- a/tests/storage/test_sliding_sync_tables.py +++ b/tests/storage/test_sliding_sync_tables.py @@ -2659,13 +2659,33 @@ class SlidingSyncTablesBackgroundUpdatesTestCase(SlidingSyncTablesTestCaseBase): exact=True, ) - # Insert and run the background update. + # Insert and run the background updates. + self.get_success( + self.store.db_pool.simple_insert( + "background_updates", + { + "update_name": _BackgroundUpdates.SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, + "progress_json": "{}", + }, + ) + ) + self.get_success( + self.store.db_pool.simple_insert( + "background_updates", + { + "update_name": _BackgroundUpdates.SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, + "progress_json": "{}", + "depends_on": _BackgroundUpdates.SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, + }, + ) + ) self.get_success( self.store.db_pool.simple_insert( "background_updates", { "update_name": _BackgroundUpdates.SLIDING_SYNC_JOINED_ROOMS_BG_UPDATE, "progress_json": "{}", + "depends_on": _BackgroundUpdates.SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, }, ) ) @@ -2807,12 +2827,32 @@ class SlidingSyncTablesBackgroundUpdatesTestCase(SlidingSyncTablesTestCaseBase): ) # Insert and run the background update. + self.get_success( + self.store.db_pool.simple_insert( + "background_updates", + { + "update_name": _BackgroundUpdates.SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, + "progress_json": "{}", + }, + ) + ) + self.get_success( + self.store.db_pool.simple_insert( + "background_updates", + { + "update_name": _BackgroundUpdates.SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, + "progress_json": "{}", + "depends_on": _BackgroundUpdates.SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, + }, + ) + ) self.get_success( self.store.db_pool.simple_insert( "background_updates", { "update_name": _BackgroundUpdates.SLIDING_SYNC_JOINED_ROOMS_BG_UPDATE, "progress_json": "{}", + "depends_on": _BackgroundUpdates.SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, }, ) ) From da463fb102709e12caa3abb5ce2ad3d41bf94401 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 28 Aug 2024 00:50:33 -0500 Subject: [PATCH 23/28] Add unique index right away for `sliding_sync_joined_rooms_to_recalculate` This makes it so we can always `upsert` to avoid duplicates otherwise I'm not sure of how to not insert duplicates in certain situations (see FIXME in the diff) which would cause problems down the line for the unique index being added later. --- .../databases/main/events_bg_updates.py | 51 +++++-------------- synapse/storage/prepare_database.py | 6 --- .../delta/87/01_sliding_sync_memberships.sql | 18 ++----- tests/storage/test_sliding_sync_tables.py | 24 +-------- 4 files changed, 21 insertions(+), 78 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 85014719ae..95244a4804 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -91,9 +91,6 @@ class _BackgroundUpdates: SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE = ( "sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update" ) - SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE = ( - "sliding_sync_index_joined_rooms_to_recalculate_table_bg_update" - ) SLIDING_SYNC_JOINED_ROOMS_BG_UPDATE = "sliding_sync_joined_rooms_bg_update" SLIDING_SYNC_MEMBERSHIP_SNAPSHOTS_BG_UPDATE = ( "sliding_sync_membership_snapshots_bg_update" @@ -318,13 +315,6 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS _BackgroundUpdates.SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, self._sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update, ) - self.db_pool.updates.register_background_index_update( - _BackgroundUpdates.SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, - index_name="sliding_sync_joined_rooms_to_recalculate_room_id_idx", - table="sliding_sync_joined_rooms_to_recalculate", - columns=["room_id"], - unique=True, - ) # Add some background updates to populate the sliding sync tables self.db_pool.updates.register_background_update_handler( _BackgroundUpdates.SLIDING_SYNC_JOINED_ROOMS_BG_UPDATE, @@ -1579,38 +1569,25 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS """ Prefill `sliding_sync_joined_rooms_to_recalculate` table with all rooms we know about already. """ - initial_insert = progress.get("initial_insert", False) def _txn(txn: LoggingTransaction) -> None: # We do this as one big bulk insert. This has been tested on a bigger # homeserver with ~10M rooms and took 11s. There is potential for this to # starve disk usage while this goes on. - if initial_insert: - txn.execute( - """ - INSERT INTO sliding_sync_joined_rooms_to_recalculate - (room_id) - SELECT room_id FROM rooms; - """, - ) - else: - # We can only upsert once the unique index has been added to the table - # (see - # `_BackgroundUpdates.SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE`) - # - # We upsert in case we have to run this multiple times. - # - # The `WHERE TRUE` clause is to avoid "Parsing Ambiguity" - txn.execute( - """ - INSERT INTO sliding_sync_joined_rooms_to_recalculate - (room_id) - SELECT room_id FROM rooms WHERE ? - ON CONFLICT (room_id) - DO NOTHING; - """, - (True,), - ) + # + # We upsert in case we have to run this multiple times. + # + # The `WHERE TRUE` clause is to avoid "Parsing Ambiguity" + txn.execute( + """ + INSERT INTO sliding_sync_joined_rooms_to_recalculate + (room_id) + SELECT room_id FROM rooms WHERE ? + ON CONFLICT (room_id) + DO NOTHING; + """, + (True,), + ) await self.db_pool.runInteraction( "_sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update", diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index 9e9c27e3b1..0c171b380b 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -679,10 +679,6 @@ def _resolve_stale_data_in_sliding_sync_joined_rooms_table( # Update the `sliding_sync_joined_rooms_to_recalculate` table with the rooms # that went stale and now need to be recalculated. - # - # FIXME: There is potentially a race where the unique index (added via - # `_BackgroundUpdates.SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE`) - # hasn't been added at this point so we won't be able to upsert DatabasePool.simple_upsert_many_txn_native_upsert( txn, table="sliding_sync_joined_rooms_to_recalculate", @@ -707,8 +703,6 @@ def _resolve_stale_data_in_sliding_sync_joined_rooms_table( # we're already working on it insertion_values={ "progress_json": "{}", - # Since we're going to upsert, we need to make sure the unique index is in place - "depends_on": _BackgroundUpdates.SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, }, ) depends_on = ( diff --git a/synapse/storage/schema/main/delta/87/01_sliding_sync_memberships.sql b/synapse/storage/schema/main/delta/87/01_sliding_sync_memberships.sql index 11fb2c4d64..71539e6bd7 100644 --- a/synapse/storage/schema/main/delta/87/01_sliding_sync_memberships.sql +++ b/synapse/storage/schema/main/delta/87/01_sliding_sync_memberships.sql @@ -16,11 +16,9 @@ -- during the background update to populate `sliding_sync_joined_rooms` which works but -- it takes a lot of work for the database to grab `DISTINCT` room_ids given how many -- state events there are for each room. --- --- This table doesn't have any indexes at this point. We add the indexes in a separate --- step to avoid the extra calculations during the bulk one-shot prefill insert. CREATE TABLE IF NOT EXISTS sliding_sync_joined_rooms_to_recalculate( - room_id TEXT NOT NULL REFERENCES rooms(room_id) + room_id TEXT NOT NULL REFERENCES rooms(room_id), + PRIMARY KEY (room_id) ); -- A table for storing room meta data (current state relevant to sliding sync) that the @@ -143,18 +141,12 @@ CREATE UNIQUE INDEX IF NOT EXISTS sliding_sync_membership_snapshots_event_stream -- -- 1. Add a background update to prefill `sliding_sync_joined_rooms_to_recalculate`. -- We do a one-shot bulk insert from the `rooms` table to prefill. --- 2. Add a background update to add indexes to the --- `sliding_sync_joined_rooms_to_recalculate` table after the one-shot bulk insert. --- We add the index in a separate step after to avoid the extra calculations during --- the one-shot bulk insert. --- 3. Add a background update to populate the new `sliding_sync_joined_rooms` table +-- 2. Add a background update to populate the new `sliding_sync_joined_rooms` table -- INSERT INTO background_updates (ordering, update_name, progress_json) VALUES - (8701, 'sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update', '{ "initial_insert": true }'); + (8701, 'sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update', '{}'); INSERT INTO background_updates (ordering, update_name, progress_json, depends_on) VALUES - (8701, 'sliding_sync_index_joined_rooms_to_recalculate_table_bg_update', '{}', 'sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update'); -INSERT INTO background_updates (ordering, update_name, progress_json, depends_on) VALUES - (8701, 'sliding_sync_joined_rooms_bg_update', '{}', 'sliding_sync_index_joined_rooms_to_calculate_table_bg_update'); + (8701, 'sliding_sync_joined_rooms_bg_update', '{}', 'sliding_sync_prefill_joined_rooms_to_recalculate_table_bg_update'); -- Add a background updates to populate the new `sliding_sync_membership_snapshots` table INSERT INTO background_updates (ordering, update_name, progress_json) VALUES diff --git a/tests/storage/test_sliding_sync_tables.py b/tests/storage/test_sliding_sync_tables.py index 300ccd664e..0770ea5e33 100644 --- a/tests/storage/test_sliding_sync_tables.py +++ b/tests/storage/test_sliding_sync_tables.py @@ -2669,23 +2669,13 @@ class SlidingSyncTablesBackgroundUpdatesTestCase(SlidingSyncTablesTestCaseBase): }, ) ) - self.get_success( - self.store.db_pool.simple_insert( - "background_updates", - { - "update_name": _BackgroundUpdates.SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, - "progress_json": "{}", - "depends_on": _BackgroundUpdates.SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, - }, - ) - ) self.get_success( self.store.db_pool.simple_insert( "background_updates", { "update_name": _BackgroundUpdates.SLIDING_SYNC_JOINED_ROOMS_BG_UPDATE, "progress_json": "{}", - "depends_on": _BackgroundUpdates.SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, + "depends_on": _BackgroundUpdates.SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, }, ) ) @@ -2836,23 +2826,13 @@ class SlidingSyncTablesBackgroundUpdatesTestCase(SlidingSyncTablesTestCaseBase): }, ) ) - self.get_success( - self.store.db_pool.simple_insert( - "background_updates", - { - "update_name": _BackgroundUpdates.SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, - "progress_json": "{}", - "depends_on": _BackgroundUpdates.SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, - }, - ) - ) self.get_success( self.store.db_pool.simple_insert( "background_updates", { "update_name": _BackgroundUpdates.SLIDING_SYNC_JOINED_ROOMS_BG_UPDATE, "progress_json": "{}", - "depends_on": _BackgroundUpdates.SLIDING_SYNC_INDEX_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, + "depends_on": _BackgroundUpdates.SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE, }, ) ) From 7c9c62051ce010e21304283a4fbcb78d3ad90360 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 28 Aug 2024 11:24:20 +0100 Subject: [PATCH 24/28] Remove all rooms pulled out from the queue --- .../databases/main/events_bg_updates.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 95244a4804..ff99a7b0b6 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1792,12 +1792,18 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS }, ) - # Now that we've processed the room, we can remove it from the queue - self.db_pool.simple_delete_txn( - txn, - table="sliding_sync_joined_rooms_to_recalculate", - keyvalues={"room_id": room_id}, - ) + # Now that we've processed all the room, we can remove them from the + # queue. + # + # Note: we need to remove all the rooms from the queue we pulled out + # from the DB, not just the ones we've processed above. Otherwise + # we'll simply keep pulling out the same rooms over and over again. + self.db_pool.simple_delete_many_batch_txn( + txn, + table="sliding_sync_joined_rooms_to_recalculate", + keys=("room_id",), + values=rooms_to_update, + ) await self.db_pool.runInteraction( "sliding_sync_joined_rooms_bg_update", _fill_table_txn From bb905cd02c93a7ef309f40f5dc61c6c4899e75fd Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 28 Aug 2024 11:44:56 +0100 Subject: [PATCH 25/28] Only run the sliding sync background updates on the main database --- .../databases/main/events_bg_updates.py | 253 +++++++++++++++++ synapse/storage/prepare_database.py | 261 +----------------- tests/storage/test_sliding_sync_tables.py | 4 +- 3 files changed, 256 insertions(+), 262 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index ff99a7b0b6..3603f46678 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -47,6 +47,8 @@ from synapse.storage.types import Cursor from synapse.types import JsonDict, RoomStreamToken, StateMap, StrCollection from synapse.types.handlers import SLIDING_SYNC_DEFAULT_BUMP_EVENT_TYPES from synapse.types.state import StateFilter +from synapse.util import json_encoder +from synapse.util.iterutils import batch_iter if TYPE_CHECKING: from synapse.server import HomeServer @@ -325,6 +327,15 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS self._sliding_sync_membership_snapshots_bg_update, ) + # FIXME: This can be removed once we bump `SCHEMA_COMPAT_VERSION` and run the + # foreground update for + # `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` (tracked by + # https://github.com/element-hq/synapse/issues/TODO) + with db_conn.cursor(txn_name="resolve_sliding_sync") as txn: + _resolve_stale_data_in_sliding_sync_tables( + txn=txn, + ) + async def _background_reindex_fields_sender( self, progress: JsonDict, batch_size: int ) -> int: @@ -2147,3 +2158,245 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS ) return len(memberships_to_update_rows) + + +def _resolve_stale_data_in_sliding_sync_tables( + txn: LoggingTransaction, +) -> None: + """ + Clears stale/out-of-date entries from the + `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` tables. + + This accounts for when someone downgrades their Synapse version and then upgrades it + again. This will ensure that we don't have any stale/out-of-date data in the + `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` tables since any new + events sent in rooms would have also needed to be written to the sliding sync + tables. For example a new event needs to bump `event_stream_ordering` in + `sliding_sync_joined_rooms` table or some state in the room changing (like the room + name). Or another example of someone's membership changing in a room affecting + `sliding_sync_membership_snapshots`. + + This way, if a row exists in the sliding sync tables, we are able to rely on it + (accurate data). And if a row doesn't exist, we use a fallback to get the same info + until the background updates fill in the rows or a new event comes in triggering it + to be fully inserted. + + FIXME: This can be removed once we bump `SCHEMA_COMPAT_VERSION` and run the + foreground update for + `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` (tracked by + https://github.com/element-hq/synapse/issues/TODO) + """ + + _resolve_stale_data_in_sliding_sync_joined_rooms_table(txn) + _resolve_stale_data_in_sliding_sync_membership_snapshots_table(txn) + + +def _resolve_stale_data_in_sliding_sync_joined_rooms_table( + txn: LoggingTransaction, +) -> None: + """ + Clears stale/out-of-date entries from the `sliding_sync_joined_rooms` table and + kicks-off the background update to catch-up with what we missed while Synapse was + downgraded. + + See `_resolve_stale_data_in_sliding_sync_tables()` description above for more + context. + """ + + # Find the point when we stopped writing to the `sliding_sync_joined_rooms` table + txn.execute( + """ + SELECT event_stream_ordering + FROM sliding_sync_joined_rooms + ORDER BY event_stream_ordering DESC + LIMIT 1 + """, + ) + + # If we have nothing written to the `sliding_sync_joined_rooms` table, there is + # nothing to clean up + row = cast(Optional[Tuple[int]], txn.fetchone()) + max_stream_ordering_sliding_sync_joined_rooms_table = None + depends_on = None + if row is not None: + (max_stream_ordering_sliding_sync_joined_rooms_table,) = row + + txn.execute( + """ + SELECT room_id + FROM events + WHERE stream_ordering > ? + GROUP BY room_id + ORDER BY MAX(stream_ordering) ASC + """, + (max_stream_ordering_sliding_sync_joined_rooms_table,), + ) + + room_rows = txn.fetchall() + # No new events have been written to the `events` table since the last time we wrote + # to the `sliding_sync_joined_rooms` table so there is nothing to clean up. This is + # the expected normal scenario for people who have not downgraded their Synapse + # version. + if not room_rows: + return + + # 1000 is an arbitrary batch size with no testing + for chunk in batch_iter(room_rows, 1000): + # Handle updating the `sliding_sync_joined_rooms` table + # + # Clear out the stale data + DatabasePool.simple_delete_many_batch_txn( + txn, + table="sliding_sync_joined_rooms", + keys=("room_id",), + values=chunk, + ) + + # Update the `sliding_sync_joined_rooms_to_recalculate` table with the rooms + # that went stale and now need to be recalculated. + DatabasePool.simple_upsert_many_txn_native_upsert( + txn, + table="sliding_sync_joined_rooms_to_recalculate", + key_names=("room_id",), + key_values=chunk, + value_names=(), + # No value columns, therefore make a blank list so that the following + # zip() works correctly. + value_values=[() for x in range(len(chunk))], + ) + else: + # Re-run the `sliding_sync_joined_rooms_to_recalculate` prefill if there is + # nothing in the `sliding_sync_joined_rooms` table + DatabasePool.simple_upsert_txn_native_upsert( + txn, + table="background_updates", + keyvalues={ + "update_name": _BackgroundUpdates.SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE + }, + values={}, + # Only insert the row if it doesn't already exist. If it already exists, + # we're already working on it + insertion_values={ + "progress_json": "{}", + }, + ) + depends_on = ( + _BackgroundUpdates.SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE + ) + + # Now kick-off the background update to catch-up with what we missed while Synapse + # was downgraded. + # + # We may need to catch-up on everything if we have nothing written to the + # `sliding_sync_joined_rooms` table yet. This could happen if someone had zero rooms + # on their server (so the normal background update completes), downgrade Synapse + # versions, join and create some new rooms, and upgrade again. + DatabasePool.simple_upsert_txn_native_upsert( + txn, + table="background_updates", + keyvalues={ + "update_name": _BackgroundUpdates.SLIDING_SYNC_JOINED_ROOMS_BG_UPDATE + }, + values={}, + # Only insert the row if it doesn't already exist. If it already exists, we will + # eventually fill in the rows we're trying to populate. + insertion_values={ + # Empty progress is expected since it's not used for this background update. + "progress_json": "{}", + # Wait for the prefill to finish + "depends_on": depends_on, + }, + ) + + +def _resolve_stale_data_in_sliding_sync_membership_snapshots_table( + txn: LoggingTransaction, +) -> None: + """ + Clears stale/out-of-date entries from the `sliding_sync_membership_snapshots` table + and kicks-off the background update to catch-up with what we missed while Synapse + was downgraded. + + See `_resolve_stale_data_in_sliding_sync_tables()` description above for more + context. + """ + + # Find the point when we stopped writing to the `sliding_sync_membership_snapshots` table + txn.execute( + """ + SELECT event_stream_ordering + FROM sliding_sync_membership_snapshots + ORDER BY event_stream_ordering DESC + LIMIT 1 + """, + ) + + # If we have nothing written to the `sliding_sync_membership_snapshots` table, + # there is nothing to clean up + row = cast(Optional[Tuple[int]], txn.fetchone()) + max_stream_ordering_sliding_sync_membership_snapshots_table = None + if row is not None: + (max_stream_ordering_sliding_sync_membership_snapshots_table,) = row + + # XXX: Since `forgotten` is simply a flag on the `room_memberships` table that is + # set out-of-band, there is no way to tell whether it was set while Synapse was + # downgraded. The only thing the user can do is `/forget` again if they run into + # this. + # + # This only picks up changes to memberships. + txn.execute( + """ + SELECT user_id, room_id + FROM local_current_membership + WHERE event_stream_ordering > ? + ORDER BY event_stream_ordering ASC + """, + (max_stream_ordering_sliding_sync_membership_snapshots_table,), + ) + + membership_rows = txn.fetchall() + # No new events have been written to the `events` table since the last time we wrote + # to the `sliding_sync_membership_snapshots` table so there is nothing to clean up. + # This is the expected normal scenario for people who have not downgraded their + # Synapse version. + if not membership_rows: + return + + # 1000 is an arbitrary batch size with no testing + for chunk in batch_iter(membership_rows, 1000): + # Handle updating the `sliding_sync_membership_snapshots` table + # + DatabasePool.simple_delete_many_batch_txn( + txn, + table="sliding_sync_membership_snapshots", + keys=("user_id", "room_id"), + values=chunk, + ) + + # Now kick-off the background update to catch-up with what we missed while Synapse + # was downgraded. + # + # We may need to catch-up on everything if we have nothing written to the + # `sliding_sync_membership_snapshots` table yet. This could happen if someone had + # zero rooms on their server (so the normal background update completes), downgrade + # Synapse versions, join and create some new rooms, and upgrade again. + # + progress_json: JsonDict = {} + if max_stream_ordering_sliding_sync_membership_snapshots_table is not None: + progress_json["last_event_stream_ordering"] = ( + max_stream_ordering_sliding_sync_membership_snapshots_table + ) + + DatabasePool.simple_upsert_txn_native_upsert( + txn, + table="background_updates", + keyvalues={ + "update_name": _BackgroundUpdates.SLIDING_SYNC_MEMBERSHIP_SNAPSHOTS_BG_UPDATE + }, + values={}, + # Only insert the row if it doesn't already exist. If it already exists, we will + # eventually fill in the rows we're trying to populate. + insertion_values={ + "progress_json": json_encoder.encode(progress_json), + }, + ) diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index 0c171b380b..aaffe5ecc9 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -32,24 +32,15 @@ from typing import ( Optional, TextIO, Tuple, - cast, ) import attr from synapse.config.homeserver import HomeServerConfig -from synapse.storage.database import ( - DatabasePool, - LoggingDatabaseConnection, - LoggingTransaction, -) -from synapse.storage.databases.main.events_bg_updates import _BackgroundUpdates +from synapse.storage.database import LoggingDatabaseConnection, LoggingTransaction from synapse.storage.engines import BaseDatabaseEngine, PostgresEngine, Sqlite3Engine from synapse.storage.schema import SCHEMA_COMPAT_VERSION, SCHEMA_VERSION from synapse.storage.types import Cursor -from synapse.types import JsonDict -from synapse.util import json_encoder -from synapse.util.iterutils import batch_iter logger = logging.getLogger(__name__) @@ -576,256 +567,6 @@ def _upgrade_existing_database( logger.info("Schema now up to date") - # FIXME: This can be removed once we bump `SCHEMA_COMPAT_VERSION` and run the - # foreground update for - # `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` (tracked by - # https://github.com/element-hq/synapse/issues/TODO) - _resolve_stale_data_in_sliding_sync_tables( - txn=cur, - ) - - -def _resolve_stale_data_in_sliding_sync_tables( - txn: LoggingTransaction, -) -> None: - """ - Clears stale/out-of-date entries from the - `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` tables. - - This accounts for when someone downgrades their Synapse version and then upgrades it - again. This will ensure that we don't have any stale/out-of-date data in the - `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` tables since any new - events sent in rooms would have also needed to be written to the sliding sync - tables. For example a new event needs to bump `event_stream_ordering` in - `sliding_sync_joined_rooms` table or some state in the room changing (like the room - name). Or another example of someone's membership changing in a room affecting - `sliding_sync_membership_snapshots`. - - This way, if a row exists in the sliding sync tables, we are able to rely on it - (accurate data). And if a row doesn't exist, we use a fallback to get the same info - until the background updates fill in the rows or a new event comes in triggering it - to be fully inserted. - - FIXME: This can be removed once we bump `SCHEMA_COMPAT_VERSION` and run the - foreground update for - `sliding_sync_joined_rooms`/`sliding_sync_membership_snapshots` (tracked by - https://github.com/element-hq/synapse/issues/TODO) - """ - - _resolve_stale_data_in_sliding_sync_joined_rooms_table(txn) - _resolve_stale_data_in_sliding_sync_membership_snapshots_table(txn) - - -def _resolve_stale_data_in_sliding_sync_joined_rooms_table( - txn: LoggingTransaction, -) -> None: - """ - Clears stale/out-of-date entries from the `sliding_sync_joined_rooms` table and - kicks-off the background update to catch-up with what we missed while Synapse was - downgraded. - - See `_resolve_stale_data_in_sliding_sync_tables()` description above for more - context. - """ - - # Find the point when we stopped writing to the `sliding_sync_joined_rooms` table - txn.execute( - """ - SELECT event_stream_ordering - FROM sliding_sync_joined_rooms - ORDER BY event_stream_ordering DESC - LIMIT 1 - """, - ) - - # If we have nothing written to the `sliding_sync_joined_rooms` table, there is - # nothing to clean up - row = cast(Optional[Tuple[int]], txn.fetchone()) - max_stream_ordering_sliding_sync_joined_rooms_table = None - depends_on = None - if row is not None: - (max_stream_ordering_sliding_sync_joined_rooms_table,) = row - - txn.execute( - """ - SELECT room_id - FROM events - WHERE stream_ordering > ? - GROUP BY room_id - ORDER BY MAX(stream_ordering) ASC - """, - (max_stream_ordering_sliding_sync_joined_rooms_table,), - ) - - room_rows = txn.fetchall() - # No new events have been written to the `events` table since the last time we wrote - # to the `sliding_sync_joined_rooms` table so there is nothing to clean up. This is - # the expected normal scenario for people who have not downgraded their Synapse - # version. - if not room_rows: - return - - # 1000 is an arbitrary batch size with no testing - for chunk in batch_iter(room_rows, 1000): - # Handle updating the `sliding_sync_joined_rooms` table - # - # Clear out the stale data - DatabasePool.simple_delete_many_batch_txn( - txn, - table="sliding_sync_joined_rooms", - keys=("room_id",), - values=chunk, - ) - - # Update the `sliding_sync_joined_rooms_to_recalculate` table with the rooms - # that went stale and now need to be recalculated. - DatabasePool.simple_upsert_many_txn_native_upsert( - txn, - table="sliding_sync_joined_rooms_to_recalculate", - key_names=("room_id",), - key_values=chunk, - value_names=(), - # No value columns, therefore make a blank list so that the following - # zip() works correctly. - value_values=[() for x in range(len(chunk))], - ) - else: - # Re-run the `sliding_sync_joined_rooms_to_recalculate` prefill if there is - # nothing in the `sliding_sync_joined_rooms` table - DatabasePool.simple_upsert_txn_native_upsert( - txn, - table="background_updates", - keyvalues={ - "update_name": _BackgroundUpdates.SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE - }, - values={}, - # Only insert the row if it doesn't already exist. If it already exists, - # we're already working on it - insertion_values={ - "progress_json": "{}", - }, - ) - depends_on = ( - _BackgroundUpdates.SLIDING_SYNC_PREFILL_JOINED_ROOMS_TO_RECALCULATE_TABLE_BG_UPDATE - ) - - # Now kick-off the background update to catch-up with what we missed while Synapse - # was downgraded. - # - # We may need to catch-up on everything if we have nothing written to the - # `sliding_sync_joined_rooms` table yet. This could happen if someone had zero rooms - # on their server (so the normal background update completes), downgrade Synapse - # versions, join and create some new rooms, and upgrade again. - DatabasePool.simple_upsert_txn_native_upsert( - txn, - table="background_updates", - keyvalues={ - "update_name": _BackgroundUpdates.SLIDING_SYNC_JOINED_ROOMS_BG_UPDATE - }, - values={}, - # Only insert the row if it doesn't already exist. If it already exists, we will - # eventually fill in the rows we're trying to populate. - insertion_values={ - # Empty progress is expected since it's not used for this background update. - "progress_json": "{}", - # Wait for the prefill to finish - "depends_on": depends_on, - }, - ) - - -def _resolve_stale_data_in_sliding_sync_membership_snapshots_table( - txn: LoggingTransaction, -) -> None: - """ - Clears stale/out-of-date entries from the `sliding_sync_membership_snapshots` table - and kicks-off the background update to catch-up with what we missed while Synapse - was downgraded. - - See `_resolve_stale_data_in_sliding_sync_tables()` description above for more - context. - """ - - # Find the point when we stopped writing to the `sliding_sync_membership_snapshots` table - txn.execute( - """ - SELECT event_stream_ordering - FROM sliding_sync_membership_snapshots - ORDER BY event_stream_ordering DESC - LIMIT 1 - """, - ) - - # If we have nothing written to the `sliding_sync_membership_snapshots` table, - # there is nothing to clean up - row = cast(Optional[Tuple[int]], txn.fetchone()) - max_stream_ordering_sliding_sync_membership_snapshots_table = None - if row is not None: - (max_stream_ordering_sliding_sync_membership_snapshots_table,) = row - - # XXX: Since `forgotten` is simply a flag on the `room_memberships` table that is - # set out-of-band, there is no way to tell whether it was set while Synapse was - # downgraded. The only thing the user can do is `/forget` again if they run into - # this. - # - # This only picks up changes to memberships. - txn.execute( - """ - SELECT user_id, room_id - FROM local_current_membership - WHERE event_stream_ordering > ? - ORDER BY event_stream_ordering ASC - """, - (max_stream_ordering_sliding_sync_membership_snapshots_table,), - ) - - membership_rows = txn.fetchall() - # No new events have been written to the `events` table since the last time we wrote - # to the `sliding_sync_membership_snapshots` table so there is nothing to clean up. - # This is the expected normal scenario for people who have not downgraded their - # Synapse version. - if not membership_rows: - return - - # 1000 is an arbitrary batch size with no testing - for chunk in batch_iter(membership_rows, 1000): - # Handle updating the `sliding_sync_membership_snapshots` table - # - DatabasePool.simple_delete_many_batch_txn( - txn, - table="sliding_sync_membership_snapshots", - keys=("user_id", "room_id"), - values=chunk, - ) - - # Now kick-off the background update to catch-up with what we missed while Synapse - # was downgraded. - # - # We may need to catch-up on everything if we have nothing written to the - # `sliding_sync_membership_snapshots` table yet. This could happen if someone had - # zero rooms on their server (so the normal background update completes), downgrade - # Synapse versions, join and create some new rooms, and upgrade again. - # - progress_json: JsonDict = {} - if max_stream_ordering_sliding_sync_membership_snapshots_table is not None: - progress_json["last_event_stream_ordering"] = ( - max_stream_ordering_sliding_sync_membership_snapshots_table - ) - - DatabasePool.simple_upsert_txn_native_upsert( - txn, - table="background_updates", - keyvalues={ - "update_name": _BackgroundUpdates.SLIDING_SYNC_MEMBERSHIP_SNAPSHOTS_BG_UPDATE - }, - values={}, - # Only insert the row if it doesn't already exist. If it already exists, we will - # eventually fill in the rows we're trying to populate. - insertion_values={ - "progress_json": json_encoder.encode(progress_json), - }, - ) - def _apply_module_schemas( txn: Cursor, database_engine: BaseDatabaseEngine, config: HomeServerConfig diff --git a/tests/storage/test_sliding_sync_tables.py b/tests/storage/test_sliding_sync_tables.py index 0770ea5e33..f6a6796e7b 100644 --- a/tests/storage/test_sliding_sync_tables.py +++ b/tests/storage/test_sliding_sync_tables.py @@ -33,8 +33,8 @@ from synapse.rest import admin from synapse.rest.client import login, room from synapse.server import HomeServer from synapse.storage.databases.main.events import DeltaState -from synapse.storage.databases.main.events_bg_updates import _BackgroundUpdates -from synapse.storage.prepare_database import ( +from synapse.storage.databases.main.events_bg_updates import ( + _BackgroundUpdates, _resolve_stale_data_in_sliding_sync_joined_rooms_table, _resolve_stale_data_in_sliding_sync_membership_snapshots_table, ) From 6f9932d146fda288688da4b7a75352364fa9d26b Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 28 Aug 2024 14:13:52 +0100 Subject: [PATCH 26/28] Handle old rows with null event_stream_ordering column --- .../databases/main/events_bg_updates.py | 132 +++++++++++++----- 1 file changed, 99 insertions(+), 33 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 3603f46678..1f8905fefa 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1828,38 +1828,84 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS """ Background update to populate the `sliding_sync_membership_snapshots` table. """ - last_event_stream_ordering = progress.get( - "last_event_stream_ordering", -(1 << 31) - ) + # We do this in two phases: a) the initial phase where we go through all + # room memberships, and then b) a second phase where we look at new + # memberships (this is to handle the case where we downgrade and then + # upgrade again). + # + # We have to do this as two phases (rather than just the second phase + # where we iterate on event_stream_ordering), as the + # `event_stream_ordering` column may have null values for old rows. + # Therefore we first do the set of historic rooms and *then* look at any + # new rows (which will have a non-null `event_stream_ordering`). + initial_phase = progress.get("initial_phase") + if initial_phase is None: + # If this is the first run, store the current max stream position. + # We know we will go through all memberships less than the current + # max in the initial phase. + progress = { + "initial_phase": True, + "last_event_stream_ordering": self.get_room_max_stream_ordering(), + } + await self.db_pool.updates._background_update_progress( + _BackgroundUpdates.SLIDING_SYNC_MEMBERSHIP_SNAPSHOTS_BG_UPDATE, + progress, + ) + initial_phase = True + + last_room_id = progress.get("last_room_id", "") + last_event_stream_ordering = progress["last_event_stream_ordering"] def _find_memberships_to_update_txn( txn: LoggingTransaction, ) -> List[Tuple[str, str, str, str, str, int, bool]]: # Fetch the set of event IDs that we want to update - # - # It's important to sort by `event_stream_ordering` *ascending* (oldest to - # newest) so that if we see that this background update in progress and want - # to start the catch-up process, we can safely assume that it will - # eventually get to the rooms we want to catch-up on anyway (see - # `_resolve_stale_data_in_sliding_sync_tables()`). - txn.execute( - """ - SELECT - c.room_id, - c.user_id, - e.sender, - c.event_id, - c.membership, - c.event_stream_ordering, - e.outlier - FROM local_current_membership as c - INNER JOIN events AS e USING (event_id) - WHERE event_stream_ordering > ? - ORDER BY event_stream_ordering ASC - LIMIT ? - """, - (last_event_stream_ordering, batch_size), - ) + + if initial_phase: + txn.execute( + """ + SELECT + c.room_id, + c.user_id, + e.sender, + c.event_id, + c.membership, + e.stream_ordering, + e.outlier + FROM local_current_membership as c + INNER JOIN events AS e USING (event_id) + WHERE c.room_id > ? + ORDER BY c.room_id ASC + LIMIT ? + """, + (last_room_id, batch_size), + ) + elif last_event_stream_ordering is not None: + # It's important to sort by `event_stream_ordering` *ascending* (oldest to + # newest) so that if we see that this background update in progress and want + # to start the catch-up process, we can safely assume that it will + # eventually get to the rooms we want to catch-up on anyway (see + # `_resolve_stale_data_in_sliding_sync_tables()`). + txn.execute( + """ + SELECT + c.room_id, + c.user_id, + e.sender, + c.event_id, + c.membership, + c.event_stream_ordering, + e.outlier + FROM local_current_membership as c + INNER JOIN events AS e USING (event_id) + WHERE event_stream_ordering > ? + ORDER BY event_stream_ordering ASC + LIMIT ? + """, + (last_event_stream_ordering, batch_size), + ) + else: + raise Exception("last_event_stream_ordering should not be None") memberships_to_update_rows = cast( List[Tuple[str, str, str, str, str, int, bool]], txn.fetchall() @@ -1873,10 +1919,22 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS ) if not memberships_to_update_rows: - await self.db_pool.updates._end_background_update( - _BackgroundUpdates.SLIDING_SYNC_MEMBERSHIP_SNAPSHOTS_BG_UPDATE - ) - return 0 + if initial_phase: + # Move onto the next phase. + await self.db_pool.updates._background_update_progress( + _BackgroundUpdates.SLIDING_SYNC_MEMBERSHIP_SNAPSHOTS_BG_UPDATE, + { + "initial_phase": False, + "last_event_stream_ordering": last_event_stream_ordering, + }, + ) + return 0 + else: + # We've finished both phases, we're done. + await self.db_pool.updates._end_background_update( + _BackgroundUpdates.SLIDING_SYNC_MEMBERSHIP_SNAPSHOTS_BG_UPDATE + ) + return 0 def _find_previous_membership_txn( txn: LoggingTransaction, room_id: str, user_id: str, stream_ordering: int @@ -2144,7 +2202,7 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS # Update the progress ( - _room_id, + room_id, _user_id, _sender, _membership_event_id, @@ -2152,9 +2210,16 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS membership_event_stream_ordering, _is_outlier, ) = memberships_to_update_rows[-1] + + progress = { + "initial_phase": initial_phase, + "last_room_id": room_id, + "last_event_stream_ordering": membership_event_stream_ordering, + } + await self.db_pool.updates._background_update_progress( _BackgroundUpdates.SLIDING_SYNC_MEMBERSHIP_SNAPSHOTS_BG_UPDATE, - {"last_event_stream_ordering": membership_event_stream_ordering}, + progress, ) return len(memberships_to_update_rows) @@ -2383,6 +2448,7 @@ def _resolve_stale_data_in_sliding_sync_membership_snapshots_table( # progress_json: JsonDict = {} if max_stream_ordering_sliding_sync_membership_snapshots_table is not None: + progress_json["initial_phase"] = False progress_json["last_event_stream_ordering"] = ( max_stream_ordering_sliding_sync_membership_snapshots_table ) From ab414f2ab8a294fbffb417003eeea0f14bbd6588 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 28 Aug 2024 14:23:19 +0100 Subject: [PATCH 27/28] Use event_auth table to get previous membership --- .../databases/main/events_bg_updates.py | 31 +++++++------------ tests/storage/test_sliding_sync_tables.py | 10 +++--- 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 1f8905fefa..946d5ec65b 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1937,33 +1937,27 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS return 0 def _find_previous_membership_txn( - txn: LoggingTransaction, room_id: str, user_id: str, stream_ordering: int + txn: LoggingTransaction, event_id: str, user_id: str ) -> Tuple[str, str]: - # Find the previous invite/knock event before the leave event + # Find the previous invite/knock event before the leave event. This + # is done by looking at the auth events of the invite/knock and + # finding the corresponding membership event. txn.execute( """ - SELECT event_id, membership - FROM room_memberships - WHERE - room_id = ? - AND user_id = ? - AND event_stream_ordering < ? - ORDER BY event_stream_ordering DESC - LIMIT 1 + SELECT m.event_id, m.membership + FROM event_auth AS a + INNER JOIN room_memberships AS m ON (a.auth_id = m.event_id) + WHERE a.event_id = ? AND m.user_id = ? """, - ( - room_id, - user_id, - stream_ordering, - ), + (event_id, user_id), ) row = txn.fetchone() # We should see a corresponding previous invite/knock event assert row is not None - event_id, membership = row + previous_event_id, membership = row - return event_id, membership + return previous_event_id, membership # Map from (room_id, user_id) to ... to_insert_membership_snapshots: Dict[ @@ -2057,9 +2051,8 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS await self.db_pool.runInteraction( "sliding_sync_membership_snapshots_bg_update._find_previous_membership", _find_previous_membership_txn, - room_id, + membership_event_id, user_id, - membership_event_stream_ordering, ) ) diff --git a/tests/storage/test_sliding_sync_tables.py b/tests/storage/test_sliding_sync_tables.py index f6a6796e7b..569863ab13 100644 --- a/tests/storage/test_sliding_sync_tables.py +++ b/tests/storage/test_sliding_sync_tables.py @@ -270,9 +270,7 @@ class SlidingSyncTablesTestCaseBase(HomeserverTestCase): return invite_room_id, persisted_event def _retract_remote_invite_for_user( - self, - user_id: str, - remote_room_id: str, + self, user_id: str, remote_room_id: str, invite_event_id: str ) -> EventBase: """ Create a fake invite retraction for a remote room and persist it. @@ -285,6 +283,7 @@ class SlidingSyncTablesTestCaseBase(HomeserverTestCase): user_id: The person who was invited and we're going to retract the invite for. remote_room_id: The room ID that the invite was for. + invite_event_id: The event ID of the invite Returns: The persisted leave (kick) event. @@ -298,7 +297,7 @@ class SlidingSyncTablesTestCaseBase(HomeserverTestCase): "origin_server_ts": 1, "type": EventTypes.Member, "content": {"membership": Membership.LEAVE}, - "auth_events": [], + "auth_events": [invite_event_id], "prev_events": [], } @@ -2202,6 +2201,7 @@ class SlidingSyncTablesTestCase(SlidingSyncTablesTestCaseBase): remote_invite_retraction_event = self._retract_remote_invite_for_user( user_id=user1_id, remote_room_id=remote_invite_room_id, + invite_event_id=remote_invite_event.event_id, ) # No one local is joined to the remote room @@ -3549,6 +3549,7 @@ class SlidingSyncTablesBackgroundUpdatesTestCase(SlidingSyncTablesTestCaseBase): room_id_no_info_leave_event = self._retract_remote_invite_for_user( user_id=user1_id, remote_room_id=room_id_no_info, + invite_event_id=room_id_no_info_invite_event.event_id, ) room_id_with_info_leave_event_response = self.helper.leave( room_id_with_info, user1_id, tok=user1_tok @@ -3556,6 +3557,7 @@ class SlidingSyncTablesBackgroundUpdatesTestCase(SlidingSyncTablesTestCaseBase): space_room_id_leave_event = self._retract_remote_invite_for_user( user_id=user1_id, remote_room_id=space_room_id, + invite_event_id=space_room_id_invite_event.event_id, ) # Clean-up the `sliding_sync_membership_snapshots` table as if the inserts did not From 90d0e035dd872150c5a99a42659f900f0a00949d Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 28 Aug 2024 14:14:09 +0100 Subject: [PATCH 28/28] Fix port script tests by handling empty DBs correctly --- synapse/storage/databases/main/events_bg_updates.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 946d5ec65b..6b080f7678 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -2323,6 +2323,12 @@ def _resolve_stale_data_in_sliding_sync_joined_rooms_table( value_values=[() for x in range(len(chunk))], ) else: + txn.execute("SELECT 1 FROM local_current_membership LIMIT 1") + row = txn.fetchone() + if row is None: + # There are no rooms, so don't schedule the bg update. + return + # Re-run the `sliding_sync_joined_rooms_to_recalculate` prefill if there is # nothing in the `sliding_sync_joined_rooms` table DatabasePool.simple_upsert_txn_native_upsert( @@ -2430,6 +2436,12 @@ def _resolve_stale_data_in_sliding_sync_membership_snapshots_table( keys=("user_id", "room_id"), values=chunk, ) + else: + txn.execute("SELECT 1 FROM local_current_membership LIMIT 1") + row = txn.fetchone() + if row is None: + # There are no rooms, so don't schedule the bg update. + return # Now kick-off the background update to catch-up with what we missed while Synapse # was downgraded.