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

Consider using the "ignored_users" database table when filtering client events #11964

Closed
anoadragon453 opened this issue Feb 11, 2022 · 2 comments · Fixed by #12225
Closed

Consider using the "ignored_users" database table when filtering client events #11964

anoadragon453 opened this issue Feb 11, 2022 · 2 comments · Fixed by #12225
Assignees
Labels
T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.

Comments

@anoadragon453
Copy link
Member

When ignoring a user, clients modify a m.ignored_user_list key in their account data (spec). Synapse will detect if this is happening, and modify an equivalent database table named ignored_users. The idea is that the database table is faster to read from than digging the value out of account data.

The table is used when calculating bulk push rules:

# If the event is not a state event check if any users ignore the sender.
if not event.is_state():
ignorers = await self.store.ignored_by(event.sender)
else:
ignorers = set()

@cached(max_entries=5000, iterable=True)
async def ignored_by(self, user_id: str) -> Set[str]:
"""
Get users which ignore the given user.
Params:
user_id: The user ID which might be ignored.
Return:
The user IDs which ignore the given user.
"""
return set(
await self.db_pool.simple_select_onecol(
table="ignored_users",
keyvalues={"ignored_user_id": user_id},
retcol="ignorer_user_id",
desc="ignored_by",
)
)

Yet it is not used when ignored users are considered while filtering events for clients (i.e. for /sync):

async def _get_ignored_users(self, user_id: str) -> FrozenSet[str]:
"""Retrieve the users ignored by the given user from their global account_data.
Returns an empty set if
- there is no global account_data entry for ignored_users
- there is such an entry, but it's not a JSON object.
"""
# TODO: Can we `SELECT ignored_user_id FROM ignored_users WHERE ignorer_user_id=?;` instead?
ignored_account_data = (
await self.store.get_global_account_data_by_type_for_user(
user_id=user_id, data_type=AccountDataTypes.IGNORED_USER_LIST
)
)
# If there is ignored users account data and it matches the proper type,
# then use it.
ignored_users: FrozenSet[str] = frozenset()
if ignored_account_data:
ignored_users_data = ignored_account_data.get("ignored_users", {})
if isinstance(ignored_users_data, dict):
ignored_users = frozenset(ignored_users_data.keys())
return ignored_users

ignore_dict_content = await storage.main.get_global_account_data_by_type_for_user(
user_id, AccountDataTypes.IGNORED_USER_LIST
)

I'm not sure how much of an optimisation this would really be (as get_global_account_data_by_type_for_user is cached on key (user_id, account_data_type)). In addition, we'd need an index of the ignorer_user column, which doesn't currently exist.

On the flip side - it is a nice idea to consolidate where we pull this information from.

@anoadragon453 anoadragon453 added the T-Other Questions, user support, anything else. label Feb 11, 2022
@babolivier babolivier added T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. and removed T-Other Questions, user support, anything else. labels Feb 11, 2022
@clokep
Copy link
Member

clokep commented Feb 11, 2022

On the flip side - it is a nice idea to consolidate where we pull this information from.

I think another nice benefit is that you don't need to do the error checking (of ensuring it is a dict, etc.)

@clokep
Copy link
Member

clokep commented Mar 14, 2022

I'm going to look at this in light of #11753 poking a bit at ignored users too.

@clokep clokep self-assigned this Mar 14, 2022
@clokep clokep added T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. and removed T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. labels Mar 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants