Use receipts event_stream_ordering instead of joins
#17032
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This should reduce IOPs incurred by joining to the events table to lookup stream ordering, which happens in many receipt handling code paths.
|
|
|
FTR the row was added in v1.68.0 in Sep 2022 |
erikjohnston
approved these changes
Apr 12, 2024
| @@ -736,8 +735,7 @@ def _insert_linearized_receipt_txn( | |||
| thread_args = (thread_id,) | |||
|
|
|||
| sql = f""" | |||
| SELECT stream_ordering, event_id FROM events | |||
| INNER JOIN receipts_linearized AS r USING (event_id, room_id) | |||
| SELECT r.event_stream_ordering, r.event_id FROM receipts_linearized AS r | |||
| WHERE r.room_id = ? AND r.receipt_type = ? AND r.user_id = ? AND {thread_clause} | |||
There was a problem hiding this comment.
I actually had some empty stream orderings in this for remote users, and the code below assumes stream_ordering is not None. I added a WHERE event_stream_ordering IS NOT NULL, and that seemed to have fixed it. @Fizzadar I think that patch makes sense?
There was a problem hiding this comment.
Yep that makes sense, thanks for catching that!
erikjohnston
added a commit
that referenced
this pull request
Apr 17, 2024
yingziwu
added a commit
to yingziwu/synapse
that referenced
this pull request
May 3, 2024
No significant changes since 1.106.0rc1. - Send an email if the address is already bound to an user account. ([\#16819](element-hq/synapse#16819)) - Implement the rendezvous mechanism described by [MSC4108](matrix-org/matrix-spec-proposals#4108). ([\#17056](element-hq/synapse#17056)) - Support delegating the rendezvous mechanism described [MSC4108](matrix-org/matrix-spec-proposals#4108) to an external implementation. ([\#17086](element-hq/synapse#17086)) - Add validation to ensure that the `limit` parameter on `/publicRooms` is non-negative. ([\#16920](element-hq/synapse#16920)) - Return `400 M_NOT_JSON` upon receiving invalid JSON in query parameters across various client and admin endpoints, rather than an internal server error. ([\#16923](element-hq/synapse#16923)) - Make the CSAPI endpoint `/keys/device_signing/upload` idempotent. ([\#16943](element-hq/synapse#16943)) - Redact membership events if the user requested erasure upon deactivating. ([\#17076](element-hq/synapse#17076)) - Add a prompt in the contributing guide to manually configure icu4c. ([\#17069](element-hq/synapse#17069)) - Clarify what part of message retention is still experimental. ([\#17099](element-hq/synapse#17099)) - Use new receipts column to optimise receipt and push action SQL queries. Contributed by Nick @ Beeper (@Fizzadar). ([\#17032](element-hq/synapse#17032), [\#17096](element-hq/synapse#17096)) - Fix mypy with latest Twisted release. ([\#17036](element-hq/synapse#17036)) - Bump minimum supported Rust version to 1.66.0. ([\#17079](element-hq/synapse#17079)) - Add helpers to transform Twisted requests to Rust http Requests/Responses. ([\#17081](element-hq/synapse#17081)) - Fix type annotation for `visited_chains` after `mypy` upgrade. ([\#17125](element-hq/synapse#17125)) * Bump anyhow from 1.0.81 to 1.0.82. ([\#17095](element-hq/synapse#17095)) * Bump peaceiris/actions-gh-pages from 3.9.3 to 4.0.0. ([\#17087](element-hq/synapse#17087)) * Bump peaceiris/actions-mdbook from 1.2.0 to 2.0.0. ([\#17089](element-hq/synapse#17089)) * Bump pyasn1-modules from 0.3.0 to 0.4.0. ([\#17093](element-hq/synapse#17093)) * Bump pygithub from 2.2.0 to 2.3.0. ([\#17092](element-hq/synapse#17092)) * Bump ruff from 0.3.5 to 0.3.7. ([\#17094](element-hq/synapse#17094)) * Bump sigstore/cosign-installer from 3.4.0 to 3.5.0. ([\#17088](element-hq/synapse#17088)) * Bump twine from 4.0.2 to 5.0.0. ([\#17091](element-hq/synapse#17091)) * Bump types-pillow from 10.2.0.20240406 to 10.2.0.20240415. ([\#17090](element-hq/synapse#17090))
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resurrecting matrix-org/synapse#13918.
This should reduce IOPs incurred by joining to the events table to lookup stream ordering, which happens in many receipt handling code paths. Like the previous PR I believe sufficient time has passed between the original migration in DB schema 72 and now to merge this as-is. It's highly unlikely that both the migration is still ongoing AND (active) users still have any receipts prior to that date.
In the unlikely event there is a receipt without a populated
event_stream_orderingsynapse will behave just as it does now when receipts exist for events that don't (yet): for push action calculation the receipts are just ignored.I've removed the validation on event IDs as this is already covered here:
synapse/synapse/handlers/receipts.py
Lines 189 to 192 in 59ceabc
Pull Request Checklist
(run the linters)