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

Speed up inserting event_push_actions_staging #13634

Merged
merged 3 commits into from
Aug 30, 2022
Merged

Speed up inserting event_push_actions_staging #13634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
15a11ad
Use execute_values instead of execute_batch when inserting into the e…
clokep Aug 25, 2022
ac69e5c
Use the simpler_insert_many helper.
clokep Aug 25, 2022
691bfa3
Newsfragment
clokep Aug 25, 2022
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/13634.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve performance of sending messages in rooms with thousands of local users.
28 changes: 8 additions & 20 deletions synapse/storage/databases/main/event_push_actions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,26 +700,14 @@ def _gen_entry(
int(count_as_unread), # unread column
)

def _add_push_actions_to_staging_txn(txn: LoggingTransaction) -> None:
# We don't use simple_insert_many here to avoid the overhead
# of generating lists of dicts.
Comment on lines -704 to -705
Copy link
Member Author

@clokep clokep Aug 25, 2022

Choose a reason for hiding this comment

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

This comment is out of date due to a combination of #11560 and #11742.


sql = """
INSERT INTO event_push_actions_staging
(event_id, user_id, actions, notif, highlight, unread)
VALUES (?, ?, ?, ?, ?, ?)
"""

txn.execute_batch(
sql,
(
_gen_entry(user_id, actions)
for user_id, actions in user_id_actions.items()
),
)

return await self.db_pool.runInteraction(
"add_push_actions_to_staging", _add_push_actions_to_staging_txn
await self.db_pool.simple_insert_many(
"event_push_actions_staging",
keys=("event_id", "user_id", "actions", "notif", "highlight", "unread"),
values=[
_gen_entry(user_id, actions)
for user_id, actions in user_id_actions.items()
],
Comment on lines +706 to +709
Copy link
Member Author

@clokep clokep Aug 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The method must take a Collection since it can be retried, so we change from a generate to a list here.

(I think this is actually a bug in the old code, but now is caught by type hints.)

Copy link
Member

@erikjohnston erikjohnston Aug 30, 2022

Choose a reason for hiding this comment

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

(I think this is actually a bug in the old code, but now is caught by type hints.)

Ah, I don't think so, as previously we created the generator inside the transaction, and its the entire transaction that gets retried (rather than just the individual queries)

Copy link
Member Author

@clokep clokep Aug 30, 2022

Choose a reason for hiding this comment

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

Oh, good call. Well now that we do it outside we need a list!

desc="add_push_actions_to_staging",
)

async def remove_push_actions_from_staging(self, event_id: str) -> None:
Expand Down