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

Use READ COMMITTED isolation level when inserting read receipts #12957

Merged
merged 3 commits into from
Jun 9, 2022
Merged

Use READ COMMITTED isolation level when inserting read receipts #12957

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
89b4551
Use READ COMMITTED isolation level when inserting read receipts
Fizzadar Jun 4, 2022
a7cc1b8
Add changelog file
Fizzadar Jun 4, 2022
b5976c1
Fix import
Fizzadar Jun 4, 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/12957.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Use lower isolation level when inserting read receipts to avoid serialization errors. Contributed by Nick @ Beeper.
5 changes: 5 additions & 0 deletions synapse/storage/databases/main/receipts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
LoggingTransaction,
)
from synapse.storage.engines import PostgresEngine
from synapse.storage.engines._base import IsolationLevel
from synapse.storage.util.id_generators import (
AbstractStreamIdTracker,
MultiWriterIdGenerator,
Expand Down Expand Up @@ -764,6 +765,10 @@ async def insert_receipt(
linearized_event_id,
data,
stream_id=stream_id,
# Read committed is actually beneficial here because we check for a receipt with
# greater stream order, and checking the very latest data at select time is better
# than the data at transaction start time.
isolation_level=IsolationLevel.READ_COMMITTED,
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit confused as to why anything is changing during this transaction TBH. Are we ending up inserting multiple receipts for the same user/room at the same time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just checked our logs and we are indeed sending multiple requests for the same client/room within the same second, I believe this is for a very small number of extremely high event rate rooms.

)

# If the receipt was older than the currently persisted one, nothing to do.
Expand Down