-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Mark remote device list updates as already handled #12557
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Reduce unnecessary work when handling remote device list updates. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1748,7 +1748,8 @@ def _add_device_outbound_room_poke_txn( | |
device_id, | ||
room_id, | ||
stream_id, | ||
False, | ||
# We only need to calculate outbound pokes for local users | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because we never need to know (indeed, cannot know) the full set of hosts that a remote user shares via their rooms? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's more that we would never send out device list updates for remote users, so there is no need to calculate where to send the update to |
||
not self.hs.is_mine_id(user_id), | ||
encoded_context, | ||
) | ||
for room_id in room_ids | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,7 @@ def add_device_change(self, user_id, device_ids, host): | |
for device_id in device_ids: | ||
stream_id = self.get_success( | ||
self.store.add_device_change_to_streams( | ||
"user_id", [device_id], ["!some:room"] | ||
user_id, [device_id], ["!some:room"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was this just plain broken before? (Why didn't we notice?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this unit test actually only cares about getting a |
||
) | ||
) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't fully understand the machinery here, but the docstring of
_handle_new_device_update_async
says:So that seems reasonable.