element-hq / element-hq/synapse

Profile updates stream further improvements

Open
#19,981 7 comments 0 reactions 1 assignee Claimed by @jaywink View on GitHub
A-Profiles A-Sync
Dominant language
Python
Stars
4.6k
Forks
600
Avg merge
5d 22h
Merged PRs (30d)
51

Description

PR https://github.com/element-hq/synapse/pull/19556 will add support for profile updates over legacy sync [MSC4429](https://github.com/matrix-org/matrix-spec-proposals/pull/4429), and an incoming pr will add [MSC4262](https://github.com/matrix-org/matrix-spec-proposals/pull/4262) for Sliding Sync. During the work on this feature, several points of concern have been raised on scalability, reliability and future proofing, especially regarding edge cases. In the interest of getting something usable ready, which also gives us data on usage, we're choosing to document and iterate on these points as follow-up.

This issue tracks solving those, in addition to tracking several items that were scoped out during the work on the initial profile updates delivery.

### Concerns

While the implementation of https://github.com/element-hq/synapse/pull/19556 has tried to ensure the data we push to clients is reliable, there are edge cases identified that we believe might a substantial rework of the Synapse implementation. This includes for example:

* [x] The lazy loading cache to avoid sending too many profiles to clients (https://github.com/element-hq/synapse/pull/19556/files/edc8624a64591bf014729f09189f4e2d8b1a6b8f#diff-70c84622edff898486bdc02d7a579db4e52a3e0b26c14020bed0ca1fb8ea2018)
* [ ] Various edge cases where a user would be removed from a room that we may miss https://github.com/element-hq/synapse/pull/19556#discussion_r3603767711, https://github.com/element-hq/synapse/pull/19556#discussion_r3588557733)
* [ ] We probably want to stop piggybacking on the `sliding_sync_table_changes` calculated changes for figuring out if memberships have changed or not (https://github.com/element-hq/synapse/pull/19556/changes#r3603767711)
* [ ] We shouldn't be generating `joined_room` profile update rows for display name changes, which currently happens as we're piggybacking on the `sliding_sync_table_changes` data.
* [ ] Performance related to the amount of write-heavy operations needed to write to the per user stream tables (which needs to pull in room memberships), especially regarding the future profile updates over federation.

### Scoped out items

* [ ] We will need to add purging of profiles updates stream rows from the database, to avoid endless growth there. This was initially in the pull request but later extracted out (see https://github.com/element-hq/synapse/pull/19556#discussion_r3534331707) to iron out complexity there
* [ ] We will need to adjust to match [MSC4429](https://github.com/matrix-org/matrix-spec-proposals/pull/4429) regarding deleted profile fields, see https://github.com/matrix-org/matrix-spec-proposals/pull/4429#pullrequestreview-4616853235
* A branch has some [wip](https://github.com/element-hq/synapse/tree/jaywink/msc4429-removed-fields) code regarding this
* [ ] We may want to add sync filtering by user ID, which was scoped out of the initial pull request
* [ ] We may want to include profiles of rooms the user joins in the sync responses, as per MSC4429:
* _To bridge this gap, homeservers MAY communicate fields from a user's profiles to a client using the users..profile_updates /sync entry; even if those fields have not been updated recently. This may be useful if, for instance, a user has just joined a room with users that it hasn't seen before. The client should be able to display appropriate profile field information immediately without having to query the homeserver for each new user's profile._
* [ ] Connection tracking of sent fields over a sliding sync connection was pulled out in the initial sliding sync pr due to performance concerns. The old branch is [here](https://github.com/element-hq/synapse/commits/jaywink/msc4262-field-tracking/), specifically from [this commit](https://github.com/element-hq/synapse/commit/9250caf5d8377f114fc348c342841e65005b1d14) onwards.

### Other gotchas

Currently only local users get profile updates pushed into the profile updates stream. Federated users are tracked in [MSC4259](https://github.com/matrix-org/matrix-spec-proposals/pull/4259). We may want to consider not having support for federated users a blocker for enabling profile updates over sync on Matrix.org, for example.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.