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

device_lists_changes_in_room is inefficient. #14037

Closed
erikjohnston opened this issue Oct 4, 2022 · 0 comments · Fixed by #14516
Closed

device_lists_changes_in_room is inefficient. #14037

erikjohnston opened this issue Oct 4, 2022 · 0 comments · Fixed by #14516
Assignees
@squahtx
Labels
A-Device-List-Tracking Telling clients about other devices. Often related to E2EE. A-Performance Performance, both client-facing and admin-facing O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience S-Minor Blocks non-critical functionality, workarounds exist. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@erikjohnston
Copy link
Member

We see a lot of disk IO on the matrix.org DB go on reading from the device_lists_changes_in_room table.

I think this is due to the fact that we have a background process that processes each row, and then flips a boolean flag on the row (which is in an index). The causes the row to effectively get deleted/reinserted, meaning that a lot of the recent (ish) rows get shuffled around causing many more pages to have to be loaded from disk to service queries.

My proposal is to do one of:

  1. Have a separate table to keep track of devices that we need to process, and then delete out of that when we've processed a row. The disadvantage with this approach is that we double the amount of data we need to insert; or
  2. Remove the boolean flag and instead keep track of which stream ID we had processed up to. This has the disadvantage that we need to scan over every row of device_lists_changes_in_room table even though we may not need to handle them (i.e. we only need to process local device changes).
@squahtx squahtx added A-Device-List-Tracking Telling clients about other devices. Often related to E2EE. S-Minor Blocks non-critical functionality, workarounds exist. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience labels Oct 4, 2022
@Fizzadar Fizzadar mentioned this issue Oct 4, 2022
Closed
@squahtx squahtx self-assigned this Nov 21, 2022
@squahtx squahtx mentioned this issue Nov 22, 2022
Merged
squahtx added a commit that referenced this issue Nov 22, 2022
@squahtx
Track unconverted device list outbound pokes using a position instead (…
9cae44f 
…#14516)

When a local device list change is added to
`device_lists_changes_in_room`, the `converted_to_destinations` flag is
set to `FALSE` and the `_handle_new_device_update_async` background
process is started. This background process looks for unconverted rows
in `device_lists_changes_in_room`, copies them to
`device_lists_outbound_pokes` and updates the flag.

To update the `converted_to_destinations` flag, the database performs a
`DELETE` and `INSERT` internally, which fragments the table. To avoid
this, track unconverted rows using a `(stream ID, room ID)` position
instead of the flag.

From now on, the `converted_to_destinations` column indicates rows that
need converting to outbound pokes, but does not indicate whether the
conversion has already taken place.

Closes #14037.

Signed-off-by: Sean Quah <seanq@matrix.org>
@MadLittleMods MadLittleMods added the A-Performance Performance, both client-facing and admin-facing label Nov 23, 2022
H-Shay pushed a commit that referenced this issue Dec 13, 2022
@squahtx @H-Shay
Track unconverted device list outbound pokes using a position instead (…
9a4aea1 
…#14516)

When a local device list change is added to
`device_lists_changes_in_room`, the `converted_to_destinations` flag is
set to `FALSE` and the `_handle_new_device_update_async` background
process is started. This background process looks for unconverted rows
in `device_lists_changes_in_room`, copies them to
`device_lists_outbound_pokes` and updates the flag.

To update the `converted_to_destinations` flag, the database performs a
`DELETE` and `INSERT` internally, which fragments the table. To avoid
this, track unconverted rows using a `(stream ID, room ID)` position
instead of the flag.

From now on, the `converted_to_destinations` column indicates rows that
need converting to outbound pokes, but does not indicate whether the
conversion has already taken place.

Closes #14037.

Signed-off-by: Sean Quah <seanq@matrix.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@squahtx squahtx

Labels
A-Device-List-Tracking Telling clients about other devices. Often related to E2EE. A-Performance Performance, both client-facing and admin-facing O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience S-Minor Blocks non-critical functionality, workarounds exist. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Track unconverted device list outbound pokes using a position instead matrix-org/synapse
3 participants
@MadLittleMods @squahtx @erikjohnston