Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Speed up user directory rebuild for users some more... #15665

Merged
merged 6 commits into from
May 24, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 31 additions & 6 deletions synapse/storage/databases/main/user_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import unicodedata
from typing import (
TYPE_CHECKING,
Collection,
Iterable,
List,
Mapping,
Expand Down Expand Up @@ -45,7 +46,7 @@
if TYPE_CHECKING:
from synapse.server import HomeServer

from synapse.api.constants import EventTypes, HistoryVisibility, JoinRules
from synapse.api.constants import EventTypes, HistoryVisibility, JoinRules, UserTypes
from synapse.storage.database import (
DatabasePool,
LoggingDatabaseConnection,
Expand Down Expand Up @@ -397,11 +398,11 @@ def _get_next_batch(txn: LoggingTransaction) -> Optional[List[str]]:
)

# First filter down to users we want to insert into the user directory.
users_to_insert = [
user_id
for user_id in users_to_work_on
if await self.should_include_local_user_in_dir(user_id)
]
users_to_insert = await self.db_pool.runInteraction(
"populate_user_directory_process_users_filter",
self._filter_local_users_for_dir_txn,
users_to_work_on,
)

# Next fetch their profiles. Note that the `user_id` here is the
# *localpart*, and that not all users have profiles.
Expand Down Expand Up @@ -494,6 +495,30 @@ async def should_include_local_user_in_dir(self, user: str) -> bool:

return True

def _filter_local_users_for_dir_txn(
self, txn: LoggingTransaction, users: Collection[str]
) -> Collection[str]:
Comment on lines +511 to +513
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to just replace should_include_local_user_in_dir (or have it call this?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my original plan, but it turns out that the functions called in should_include_local_user_in_dir are cached. I'm really not sure how much we are relying on those caches, so decided to leave it as is 🤷

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough.

"""A batched version of `should_include_local_user_in_dir`"""
users = [
user
for user in users
if self.get_app_service_by_user_id(user) is None # type: ignore[attr-defined]
and not self.get_if_app_services_interested_in_user(user) # type: ignore[attr-defined]
]

rows = self.db_pool.simple_select_many_txn(
txn,
table="users",
column="name",
iterable=users,
keyvalues={
"deactivated": 0,
},
retcols=("name", "user_type"),
)

return [row["name"] for row in rows if row["user_type"] != UserTypes.SUPPORT]

async def is_room_world_readable_or_publicly_joinable(self, room_id: str) -> bool:
"""Check if the room is either world_readable or publically joinable"""

Expand Down