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

Speed up calculating push actions in large rooms #13973

Merged
merged 4 commits into from
Sep 30, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
1 change: 1 addition & 0 deletions changelog.d/13973.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Speed up calculating push actions in large rooms.
25 changes: 15 additions & 10 deletions synapse/push/bulk_push_rule_evaluator.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,20 +303,10 @@ async def action_for_event_by_user(
event.room_id, users
)

# This is a check for the case where user joins a room without being
# allowed to see history, and then the server receives a delayed event
# from before the user joined, which they should not be pushed for
uids_with_visibility = await filter_event_for_clients_with_state(
self.store, users, event, context
)

for uid, rules in rules_by_user.items():
if event.sender == uid:
continue

if uid not in uids_with_visibility:
continue

display_name = None
profile = profiles.get(uid)
if profile:
Expand All @@ -342,6 +332,21 @@ async def action_for_event_by_user(
# Push rules say we should notify the user of this event
actions_by_user[uid] = actions

# This is a check for the case where user joins a room without being
# allowed to see history, and then the server receives a delayed event
# from before the user joined, which they should not be pushed for
#
# We do this *after* calculating the push actions as a) its unlikely
# that we'll filter anyone out and b) for large rooms its likely that
# most users will have push disabled and so the set of users to check is
# much smaller.
uids_with_visibility = await filter_event_for_clients_with_state(
self.store, actions_by_user.keys(), event, context
)

for user_id in set(uids_with_visibility).difference(uids_with_visibility):
Copy link
Member

Choose a reason for hiding this comment

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

set(uids_with_visibility).difference(uids_with_visibility) is the empty set?

Copy link
Member Author

Choose a reason for hiding this comment

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

AAAAAAAAAAAAAAAAAAAAAAAARGH

Copy link
Member Author

Choose a reason for hiding this comment

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

This requires me to write a test doesn't it

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll write the test after lunch.

actions_by_user.pop(user_id, None)

# Mark in the DB staging area the push actions for users who should be
# notified for this event. (This will then get handled when we persist
# the event)
Expand Down