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

Faster joins: Omit device list updates from partial state rooms in /sync #15069

Merged
merged 1 commit into from
Feb 14, 2023
+6 −0
Merged

Faster joins: Omit device list updates from partial state rooms in /sync #15069

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
Faster joins: Omit device list updates from partial state rooms in /sync 
...when lazy loading of members is not enabled. It's weird to notify
a client that another user's device list has changed when the client
doesn't think that they share a room.

Note that when a room is un-partial stated, device list updates are
emitted for every member in that room over /sync.

Signed-off-by: Sean Quah <[email protected]>
Sean Quah committed Feb 14, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
commit ddd210b491fa57ac6b9ece9e89053d2639d32ad0
1 change: 1 addition & 0 deletions changelog.d/15069.misc
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Faster joins: omit device list updates originating from partial state rooms in /sync responses without lazy loading of members enabled.
5 changes: 5 additions & 0 deletions synapse/handlers/sync.py
View file
Original file line number Diff line number Diff line change
@@ -1391,6 +1391,11 @@ async def generate_sync_result(
for room_id, is_partial_state in results.items()
if is_partial_state
)
membership_change_events = [
event
for event in membership_change_events
if not results.get(event.room_id, False)
]
Comment on lines +1394 to +1398
Copy link
Contributor

Choose a reason for hiding this comment

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

Argh, sorry we missed this in #14831 and #14870.

Is there any other per-room state in this function that we should suppress for a non-lazy-loading member sync?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe mem_last_change_by_room_id, but I think that might be handled by mutable_rooms_to_exclude. Otherwise I think everything else is expressed in terms of sync_result_builder.joined_rooms, which should have the partial rooms excluded for this kind of sync.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there any other per-room state in this function that we should suppress for a non-lazy-loading member sync?

I think we've handled everything that goes into the SyncResultBuilder now.

I *did* see a similar issue with presence while debugging this, but would like to not open that can of worms right now.


# Incremental eager syncs should additionally include rooms that
# - we are joined to