Check appservice user interest against the local users instead of all users (get_users_in_room mis-use)

changelog.d/13958.bugfix

@@ -0,0 +1 @@
+Fix performance bottleneck with heavy appservice and bridged users in Synapse by checking appservice user interest against the local users instead of all users.

synapse/appservice/__init__.py

@@ -172,7 +172,18 @@ async def _matches_user_in_member_list(
         Returns:
             True if this service would like to know about this room.
         """
-        member_list = await store.get_users_in_room(
+        # We can use `get_local_users_in_room(...)` here because an application
+        # service can only act on behalf of users of the server it's on.
+        #
+        # In the future, we can consider re-using
+        # `store.get_app_service_users_in_room` which is very similar to this
+        # function but has a slightly worse performance than this because we
+        # have an early escape-hatch if we find a single user that the
+        # appservice is interested in. The juice would be worth the squeeze if
+        # `store.get_app_service_users_in_room` was used in more places besides
+        # an experimental MSC. But for now we can avoid doing more work and
+        # barely using it later.
+        member_list = await store.get_local_users_in_room(
             room_id, on_invalidate=cache_context.invalidate
         )
 

synapse/storage/databases/main/appservice.py

@@ -157,7 +157,19 @@ async def get_app_service_users_in_room(
         app_service: "ApplicationService",
         cache_context: _CacheContext,
     ) -> List[str]:
-        users_in_room = await self.get_users_in_room(
+        """
+        Get all users in a room that the appservice controls.
+
+        Args:
+            room_id: The room to check in.
+            app_service: The application service to check interest/control against
+
+        Returns:
+            List of user IDs that the appservice controls.
+        """
+        # We can use `get_local_users_in_room(...)` here because an application
+        # service can only act on behalf of users of the server it's on.
+        users_in_room = await self.get_local_users_in_room(
             room_id, on_invalidate=cache_context.invalidate
         )
         return list(filter(app_service.is_interested_in_user, users_in_room))

synapse/storage/databases/main/roommember.py

@@ -152,13 +152,18 @@ async def get_users_in_room(self, room_id: str) -> List[str]:
         `get_current_hosts_in_room()` and so we can re-use the cache but it's
         not horrible to have here either.
 
-        Uses `m.room.member`s in the room state at the current forward extremities to
-        determine which users are in the room.
-
-        Will return inaccurate results for rooms with partial state, since the state for
-        the forward extremities of those rooms will exclude most members. We may also
-        calculate room state incorrectly for such rooms and believe that a member is or
-        is not in the room when the opposite is true.
+        Uses `m.room.member`s in the room state at the current forward
+        extremities to determine which users are in the room.
+
+        Will return inaccurate results for rooms with partial state, since the
+        state for the forward extremities of those rooms will exclude most
+        members. We may also calculate room state incorrectly for such rooms and
+        believe that a member is or is not in the room when the opposite is
+        true.
+
+        Note: If you only care about users in the room local to the homeserver,
+        use `get_local_users_in_room(...)` instead which will be more
+        performant.
         """
         return await self.db_pool.runInteraction(
             "get_users_in_room", self.get_users_in_room_txn, room_id