From 563c19ee3eadb4391ae9cf8b4b45e8f9c85549ec Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 9 Sep 2024 17:06:49 +0100 Subject: [PATCH 1/5] Pull out _get_bump_stamp --- synapse/handlers/sliding_sync/__init__.py | 121 ++++++++++++---------- 1 file changed, 67 insertions(+), 54 deletions(-) diff --git a/synapse/handlers/sliding_sync/__init__.py b/synapse/handlers/sliding_sync/__init__.py index 7340c6ec053..1ccf3c41ff5 100644 --- a/synapse/handlers/sliding_sync/__init__.py +++ b/synapse/handlers/sliding_sync/__init__.py @@ -1047,60 +1047,11 @@ async def get_room_sync_data( # If we're joined to the room, we need to find the last bump event before the # `to_token` if room_membership_for_user_at_to_token.membership == Membership.JOIN: - # We can quickly query for the latest bump event in the room using the - # sliding sync tables. - latest_room_bump_stamp = await self.store.get_latest_bump_stamp_for_room( - room_id - ) - - min_to_token_position = to_token.room_key.stream - - # If we can rely on the new sliding sync tables and the `bump_stamp` is - # `None`, just fallback to the membership event position. This can happen - # when we've just joined a remote room and all the events are backfilled. - if ( - # FIXME: The background job check 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/17623) - await self.store.have_finished_sliding_sync_background_jobs() - and latest_room_bump_stamp is None - ): - pass - - # The `bump_stamp` stored in the database might be ahead of our token. Since - # `bump_stamp` is only a `stream_ordering` position, we can't be 100% sure - # that's before the `to_token` in all scenarios. The only scenario we can be - # sure of is if the `bump_stamp` is totally before the minimum position from - # the token. - # - # We don't need to check if the background update has finished, as if the - # returned bump stamp is not None then it must be up to date. - elif ( - latest_room_bump_stamp is not None - and latest_room_bump_stamp < min_to_token_position - ): - bump_stamp = latest_room_bump_stamp - - # Otherwise, if it's within or after the `to_token`, we need to find the - # last bump event before the `to_token`. - else: - last_bump_event_result = ( - await self.store.get_last_event_pos_in_room_before_stream_ordering( - room_id, - to_token.room_key, - event_types=SLIDING_SYNC_DEFAULT_BUMP_EVENT_TYPES, - ) - ) - if last_bump_event_result is not None: - _, new_bump_event_pos = last_bump_event_result - - # If we've just joined a remote room, then the last bump event may - # have been backfilled (and so have a negative stream ordering). - # These negative stream orderings can't sensibly be compared, so - # instead we use the membership event position. - if new_bump_event_pos.stream > 0: - bump_stamp = new_bump_event_pos.stream + # Try and get a bump stamp, if not we just fall back to the + # membership token. + new_bump_stamp = await self._get_bump_stamp(room_id, to_token) + if new_bump_stamp is not None: + bump_stamp = new_bump_stamp unstable_expanded_timeline = False prev_room_sync_config = previous_connection_state.room_configs.get(room_id) @@ -1190,3 +1141,65 @@ async def get_room_sync_data( notification_count=0, highlight_count=0, ) + + async def _get_bump_stamp( + self, room_id: str, to_token: StreamToken + ) -> Optional[int]: + """Get a bump stamp for the room, if we have a bump event""" + + # We can quickly query for the latest bump event in the room using the + # sliding sync tables. + latest_room_bump_stamp = await self.store.get_latest_bump_stamp_for_room( + room_id + ) + + min_to_token_position = to_token.room_key.stream + + # If we can rely on the new sliding sync tables and the `bump_stamp` is + # `None`, just fallback to the membership event position. This can happen + # when we've just joined a remote room and all the events are backfilled. + if ( + # FIXME: The background job check 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/17623) + await self.store.have_finished_sliding_sync_background_jobs() + and latest_room_bump_stamp is None + ): + return None + + # The `bump_stamp` stored in the database might be ahead of our token. Since + # `bump_stamp` is only a `stream_ordering` position, we can't be 100% sure + # that's before the `to_token` in all scenarios. The only scenario we can be + # sure of is if the `bump_stamp` is totally before the minimum position from + # the token. + # + # We don't need to check if the background update has finished, as if the + # returned bump stamp is not None then it must be up to date. + elif ( + latest_room_bump_stamp is not None + and latest_room_bump_stamp < min_to_token_position + ): + return latest_room_bump_stamp + + # Otherwise, if it's within or after the `to_token`, we need to find the + # last bump event before the `to_token`. + else: + last_bump_event_result = ( + await self.store.get_last_event_pos_in_room_before_stream_ordering( + room_id, + to_token.room_key, + event_types=SLIDING_SYNC_DEFAULT_BUMP_EVENT_TYPES, + ) + ) + if last_bump_event_result is not None: + _, new_bump_event_pos = last_bump_event_result + + # If we've just joined a remote room, then the last bump event may + # have been backfilled (and so have a negative stream ordering). + # These negative stream orderings can't sensibly be compared, so + # instead we use the membership event position. + if new_bump_event_pos.stream > 0: + return new_bump_event_pos.stream + + return None From 1bcf7d7d495d365a90bab31b917eada5b5bdfd91 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 9 Sep 2024 17:15:01 +0100 Subject: [PATCH 2/5] Look for bump stamp in the room timeline This allows us to skip checking the database a lot of the time. --- synapse/handlers/sliding_sync/__init__.py | 31 ++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/sliding_sync/__init__.py b/synapse/handlers/sliding_sync/__init__.py index 1ccf3c41ff5..7aa3f39fa13 100644 --- a/synapse/handlers/sliding_sync/__init__.py +++ b/synapse/handlers/sliding_sync/__init__.py @@ -1049,7 +1049,9 @@ async def get_room_sync_data( if room_membership_for_user_at_to_token.membership == Membership.JOIN: # Try and get a bump stamp, if not we just fall back to the # membership token. - new_bump_stamp = await self._get_bump_stamp(room_id, to_token) + new_bump_stamp = await self._get_bump_stamp( + room_id, to_token, timeline_events + ) if new_bump_stamp is not None: bump_stamp = new_bump_stamp @@ -1143,9 +1145,32 @@ async def get_room_sync_data( ) async def _get_bump_stamp( - self, room_id: str, to_token: StreamToken + self, room_id: str, to_token: StreamToken, timeline: List[EventBase] ) -> Optional[int]: - """Get a bump stamp for the room, if we have a bump event""" + """Get a bump stamp for the room, if we have a bump event + + Args: + room_id + to_token: The upper bound of token to return + timeline: The list of events we have fetched. + """ + + # First check the timeline events we're returning to see if one of + # those matches. We iterate backwards and take the stream ordering + # of the first event that matches the bump event types. + for timeline_event in reversed(timeline): + if timeline_event.type in SLIDING_SYNC_DEFAULT_BUMP_EVENT_TYPES: + new_bump_stamp = timeline_event.internal_metadata.stream_ordering + + # All persisted events have a stream ordering + assert new_bump_stamp is not None + + # If we've just joined a remote room, then the last bump event may + # have been backfilled (and so have a negative stream ordering). + # These negative stream orderings can't sensibly be compared, so + # instead we use the membership event position. + if new_bump_stamp > 0: + return new_bump_stamp # We can quickly query for the latest bump event in the room using the # sliding sync tables. From 667a556dea1aa116d68eee1d7962a9e0e50ec7f9 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 9 Sep 2024 12:00:57 +0100 Subject: [PATCH 3/5] Newsfile --- changelog.d/17684.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/17684.misc diff --git a/changelog.d/17684.misc b/changelog.d/17684.misc new file mode 100644 index 00000000000..ecfb040a5fb --- /dev/null +++ b/changelog.d/17684.misc @@ -0,0 +1 @@ +Speed up sliding sync by reducing number of database calls. From 6ed910d5002305ac451066154d84c46844538087 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 10 Sep 2024 09:52:57 +0100 Subject: [PATCH 4/5] Update synapse/handlers/sliding_sync/__init__.py Co-authored-by: Eric Eastwood --- synapse/handlers/sliding_sync/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/handlers/sliding_sync/__init__.py b/synapse/handlers/sliding_sync/__init__.py index 7aa3f39fa13..5209f3df80e 100644 --- a/synapse/handlers/sliding_sync/__init__.py +++ b/synapse/handlers/sliding_sync/__init__.py @@ -1144,6 +1144,7 @@ async def get_room_sync_data( highlight_count=0, ) + @trace async def _get_bump_stamp( self, room_id: str, to_token: StreamToken, timeline: List[EventBase] ) -> Optional[int]: From d99b8f36924344385a3ed5568b27ef8552abbbca Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 10 Sep 2024 09:54:38 +0100 Subject: [PATCH 5/5] Always check bump stamp is above 0 --- synapse/handlers/sliding_sync/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/sliding_sync/__init__.py b/synapse/handlers/sliding_sync/__init__.py index 5209f3df80e..6af6d041660 100644 --- a/synapse/handlers/sliding_sync/__init__.py +++ b/synapse/handlers/sliding_sync/__init__.py @@ -1206,7 +1206,10 @@ async def _get_bump_stamp( latest_room_bump_stamp is not None and latest_room_bump_stamp < min_to_token_position ): - return latest_room_bump_stamp + if latest_room_bump_stamp > 0: + return latest_room_bump_stamp + else: + return None # Otherwise, if it's within or after the `to_token`, we need to find the # last bump event before the `to_token`.