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

Use new device_lists_changes_in_room table for fetching device lists in /sync #12388

Closed
@erikjohnston

Description

We added a table that tracks device list changes in a room in #12321. We can use this in /sync (and /keys/changes) to more efficiently calculate the device list changes without having to calculate the full set of users who share a room with the requester.

c.f.

users_who_share_room = (
await self.store.get_users_who_share_room_with_user(user_id)
)
# Always tell the user about their own devices. We check as the user
# ID is almost certainly already included (unless they're not in any
# rooms) and taking a copy of the set is relatively expensive.
if user_id not in users_who_share_room:
users_who_share_room = set(users_who_share_room)
users_who_share_room.add(user_id)
tracked_users = users_who_share_room
users_that_have_changed = (
await self.store.get_users_whose_devices_changed(
since_token.device_list_key, tracked_users
)
)

and

# First we check if any devices have changed for users that we share
# rooms with.
users_who_share_room = await self.store.get_users_who_share_room_with_user(
user_id
)

The tricky thing is that since the table has been added recently we need to a) only use it once our minimum schema is 69 and b) only for device list stream IDs that have happened after a.

Activity

added
T-EnhancementNew features, changes in functionality, improvements in performance, or user-facing enhancements.
on Apr 6, 2022
chagai95

chagai95 commented on Apr 13, 2022

@chagai95
Contributor

Is this ready for testing? I could merge the PR on a fork and try it out. I have an account which can't log in to new devices...

erikjohnston

erikjohnston commented on Apr 13, 2022

@erikjohnston
MemberAuthor

Is this ready for testing? I could merge the PR on a fork and try it out. I have an account which can't log in to new devices...

Can you try running latest develop, #12321 and #12365 should hopefully fix logging in and I'd be interested in how they perform in real world use cases.

chagai95

chagai95 commented on Apr 13, 2022

@chagai95
Contributor

Sorry I'm having some trouble with building the docker image so I won't be able to test this soon...

erikjohnston

erikjohnston commented on Apr 13, 2022

@erikjohnston
MemberAuthor

Sorry I'm having some trouble with building the docker image so I won't be able to test this soon...

If you're running 1.56 (edit:) 1.57 then there is also a hidden config option that you can set to enable the same functionality: use_new_device_lists_changes_in_room: true, which will be enabled in 1.57 (edit:) 1.58 by default. Note that if you do enable it then you must not downgrade your synapse to a prior version

chagai95

chagai95 commented on Apr 15, 2022

@chagai95
Contributor

Is this a hidden config in the release and also in the tag? I'm not using the official releases, I have a fork and I merged...

erikjohnston

erikjohnston commented on Apr 15, 2022

@erikjohnston
MemberAuthor

Is this a hidden config in the release and also in the tag? I'm not using the official releases, I have a fork and I merged...

Oh sorry, I'm wrong, its not in 1.56. I'm getting the timeline wrong. It will be in 1.57 and then enabled by default in 1.58.

The commit you need is 5c9e39e, which is in the tag v1.57.0rc1.

chagai95

chagai95 commented on Apr 18, 2022

@chagai95
Contributor

Hey @erikjohnston I managed to build a docker image and test but it seems like I still can't log in... Maybe I need to higher the cache factor? I'm using the defaults...

That's what @squahtx suggested here: #12429 (comment)

Here are the logs:
synapse.log

I found:

Apr 18 03:50:59 localhost.localdomain matrix-synapse[19576]: 2022-04-18 01:50:59,075 - synapse.http.server - 690 - WARNING - POST-12397 - Not sending response to request <XForwardedForRequest at 0x7fd634d36820 method='POST' uri='/_matrix/client/r0/login' clientproto='HTTP/1.0' site='8008'>, already disconnected.

in line 3425 after about an hour and so since clicking the login.

richvdh

richvdh commented on May 5, 2022

@richvdh
Member

I think this was fixed by #12365. @erikjohnston: if there is still outstanding work here, please can you clarify what it is.

erikjohnston

erikjohnston commented on May 5, 2022

@erikjohnston
MemberAuthor

#12365 only enabled using that table while registering a new device. This issue is about using the new table in a couple of other places where we do similar things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    T-EnhancementNew features, changes in functionality, improvements in performance, or user-facing enhancements.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Use new `device_lists_changes_in_room` table for fetching device lists in `/sync` · Issue #12388 · matrix-org/synapse