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

Skip handling of push actions for outlier events #10780

Merged
merged 3 commits into from
Sep 8, 2021
Merged
Show file tree
Hide file tree
Changes from all 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/10780.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Minor speed ups when joining large rooms over federation.
21 changes: 17 additions & 4 deletions synapse/storage/databases/main/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -1990,6 +1990,15 @@ def _set_push_actions_for_event_and_users_txn(
events_and_context.
"""

# Only non outlier events will have push actions associated with them,
# so let's filter them out. (This makes joining large rooms faster, as
# these queries took seconds to process all the state events).
Comment on lines +1993 to +1995
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's see if I can reason through this.

  • An event in the DAG is an outlier if we don't have the state at that point in the DAG
  • "Don't have the state" meaning "hat state could always be computed if needed, but we don't have easy access to it"?

Only non outlier events will have push actions associated with them

Not sure why this follows. What's a push action?

Copy link
Member Author

Choose a reason for hiding this comment

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

Push actions is the output of running a user's push rules on an event and state and basically says if and how the user should be notified about the event (e.g. should they be bushed? Should it be highlighted? etc).

An outlier really means that its not part of the "timeline", i.e. the connected chunk(s) of the rooms graph that makes up the history/timeline we send down to the clients. Think timeline events as being a sequence of messages, whereas outliers being state that was set (e.g. room name) before you joined the room. You're right that we track state for events in the timeline and not for outliers, but the reason we won't have push actions for outliers is that we only calculate them for timeline events, as we only notify for events that have just been added to the timeline rather than for old state events.

non_outlier_events = [
event
for event, _ in events_and_contexts
if not event.internal_metadata.is_outlier()
]

sql = """
INSERT INTO event_push_actions (
room_id, event_id, user_id, actions, stream_ordering,
Expand All @@ -2000,7 +2009,7 @@ def _set_push_actions_for_event_and_users_txn(
WHERE event_id = ?
"""

if events_and_contexts:
if non_outlier_events:
txn.execute_batch(
sql,
(
Expand All @@ -2010,12 +2019,12 @@ def _set_push_actions_for_event_and_users_txn(
event.depth,
event.event_id,
)
for event, _ in events_and_contexts
for event in non_outlier_events
),
)

room_to_event_ids: Dict[str, List[str]] = {}
for e, _ in events_and_contexts:
for e in non_outlier_events:
room_to_event_ids.setdefault(e.room_id, []).append(e.event_id)

for room_id, event_ids in room_to_event_ids.items():
Expand All @@ -2040,7 +2049,11 @@ def _set_push_actions_for_event_and_users_txn(
# persisted.
txn.execute_batch(
"DELETE FROM event_push_actions_staging WHERE event_id = ?",
((event.event_id,) for event, _ in all_events_and_contexts),
(
(event.event_id,)
for event, _ in all_events_and_contexts
if not event.internal_metadata.is_outlier()
),
)

def _remove_push_actions_for_event_id_txn(self, txn, room_id, event_id):
Expand Down
1 change: 1 addition & 0 deletions tests/storage/test_event_push_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ def _inject_actions(stream, action):
event.room_id = room_id
event.event_id = "$test:example.com"
event.internal_metadata.stream_ordering = stream
event.internal_metadata.is_outlier.return_value = False
event.depth = stream

self.get_success(
Expand Down