Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSC2775: Lazy loading over federation #2775

Closed
wants to merge 8 commits into from
Closed
Changes from 5 commits
Commits
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
155 changes: 155 additions & 0 deletions proposals/2775-lazy-loading-over-federation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
# Lazy loading room membership over federation

## Problem

Joining remote rooms for the first time from your homeserver can be very slow.
This is particularly painful for the first time user experience of a new
homeserver owner.

Causes include:
* Room state can be big. For instance, a /send_join response for Matrix HQ is
currently 24MB of JSON covering 28,188 events, and could easily take tens of
Copy link
Member

Choose a reason for hiding this comment

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

now 115M and 144K events, for the record.

Choose a reason for hiding this comment

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

I think there is some duplication we can get rid of. Some state events are also mentioned in the auth chain. Maybe this can be fixed by only sending it as one list of event jsons and one list of only the event ids that are in the state

Choose a reason for hiding this comment

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

Ah, this was mentioned already in #2775 (comment)

Choose a reason for hiding this comment

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

We can also think about how we can improve the auth chain size. Member events don't have to mention the previous member event in most cases and can instead mention an old member event or none at all in public rooms

seconds to calculate and send (especially on lower-end hardware).
* All these events have to be verified by the receiving server.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* All these events have to be verified by the receiving server.
* All these events have to be verified and persisted by the receiving server.

Our testing shows the main problem is writing the events to the database.

Choose a reason for hiding this comment

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

Is it possible to keep them in ram and persist them in the background while users can already use the room? Probably not because the server might crash...

* Your server may have to fetch ths signing keys for all the servers who have
sent state into the room.

Choose a reason for hiding this comment

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

This can be improved by including the public key in the event (instead of the server name?)


This also impacts peeking over federation
([MSC2444](https://github.com/matrix-org/matrix-doc/pull/2444)), which is even
more undesirable, given users expect peeking to have a very snappy UX, letting them
quickly check links to sample rooms etc.

For instance Gitter shows a usable peeked page for a room with 20K
members in under 2 seconds (https://gitter.im/webpack/webpack) including
launching the whole webapp. Similarly Discord loads usable state for a server
with 90K users like https://chat.vuejs.org in around 2s.

## Proposal

The vast majority of state events in Matrix today are `m.room.member` events.
For instance, 99.4% (30661 out of 30856) of Matrix HQ's state is
`m.room.member`s (see Stats section below).
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be interesting to know how many of these are actually ”membership”: “join” and not users that have left.

Certainly another optimisation would be to not bother telling homeservers about ”leave” membership events until they need to know them for some reason (which is probably when processing their next join and, even then, unless they are ”invite” or ”ban” I’m still not sure why we care about their previous ”leave” as long as join rules permit).

Copy link

Choose a reason for hiding this comment

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

Looks like it's approximately 2:1 join:leave for #matrix:matrix.org, but approximately even for #fdroid:f-droid.org. It'd be a trickier query for "not users that have left".

select json::json#>>'{content,membership}' as membership, count(*) from state_events natural join event_json where type='m.room.member' and room_id='!OGEhHVWSdvArJzumhm:matrix.org' group by membership order by count(*) desc;
 membership | count 
------------+-------
 join       | 24269
 leave      | 12029
 ban        |   307
 invite     |   145

Copy link
Member

Choose a reason for hiding this comment

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

current figures for #matrix:

 count | membership 
-------+------------
  1458 | ban
 26287 | join
 26251 | leave


Therefore, in the response to `/send_join` (or a MSC2444 `/peek`), we propose
sending only the following `m.room.member` events (if the initiating server
includes `lazy_load_members: true` in their JSON request body):
Copy link
Member

Choose a reason for hiding this comment

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

the request body for a /send_join is the membership event itself, so we'll have to put this flag elsewhere. Suggest a lazy_load_members=true|false query-param.

Copy link
Member

Choose a reason for hiding this comment

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

also, needs an unstable prefix, I guess.

Suggest org.matrix.msc2775.lazy_load_members


* the "hero" room members which are needed for clients to display
a summary of the room (based on the
[requirements of the CS API](https://github.com/matrix-org/matrix-doc/blob/1c7a6a9c7fa2b47877ce8790ea5e5c588df5fa90/api/client-server/sync.yaml#L148))
Comment on lines +37 to +39
Copy link
Member

Choose a reason for hiding this comment

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

do we really need this as well as a summary?

* any members which are in the auth chain for the state events in the response
Copy link
Member

Choose a reason for hiding this comment

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

the auth chain ends up in a separate section, so I think this is a no-op.

* any members which are power events (aka control events): bans & kicks.
Copy link
Member

Choose a reason for hiding this comment

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

why do we need kicks here?

* one joined member per server (if we want to be able to send messages while
Copy link
Contributor

Choose a reason for hiding this comment

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

I worry that this will create a potential race at the protocol level that may be exploitable by a bad actor in the room.

For example, in the situation that you join a room with two or more other users that are resident on the same server remote.com. You learn about user A but not about B, C or D.

User A detects your join and then leaves the room immediately before you are able to retrieve the rest of the room state, therefore you think you are the only occupant of the room. As you don’t know about any users from remote.com anymore, you no longer know if that server is still resident in the room and therefore you don’t know if you can ask it for room state.

The impact of this is lessened if you can include more than one membership from a given homeserver—even knowing about two or three users reduces the chance of this ever being an issue.

Copy link
Member

Choose a reason for hiding this comment

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

for context, this will be 2462 membership events for Matrix HQ (of 54194 total state) at present.

Copy link
Member

Choose a reason for hiding this comment

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

Can we just ask the server we join via to send us a list of the servers in the room? There doesn't seem to be any need to have actual membership events for them.

the room state is synchronising, otherwise we won't know where to send them
to)
* any membership events with membership `invite` (to mitigate risk of double invites)
* any members for user_ids which are referred to by the content of state events
in the response (e.g. `m.room.power_levels`) <-- TBD. These could be irrelevant,
Copy link
Contributor

Choose a reason for hiding this comment

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

This does seem irrelevant, as the power levels are still enforced even for users that we don’t know about yet. Anything that’s important for auth will already be in the auth chain.

plus we don't know where to look for user_ids in arbitrary state events.
Comment on lines +37 to +48
Copy link
Member

@richvdh richvdh Dec 22, 2021

Choose a reason for hiding this comment

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

something else I'd like to change while we're messing about with /send_join responses: we should make it explicit that events do not need to be duplicated between state and auth_chain. For example, the m.room.create is necessarily both part of state but is also on the auth chain for all the events in the response. There is no point in sending two copies of such events - servers should be able to elide them from auth_chain.

This needs to be opt-in, because existing implementations (such as Synapse) rely on at least the create event being returned in auth_chain - so this is a good time to change it (when we are adding a query param anyway).

We should do something similar to /state.


In addition, we extend the response to `/send_join` and `/peek` to include a
`summary` block, matching that of the CS `/sync` API, giving the local server
the necessary data to support MSC1227 CS API lazy loading.

The joining server can then sync in the remaining membership events by calling
`/state` as of the user's join event. To avoid retrieving duplicate data, we
propose adding a parameter of `lazy_load_members_only: true` to the JSON
request body which would then only return the missing `m.room.member` events.
Copy link
Contributor

Choose a reason for hiding this comment

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

This implies that the homeserver needs to track which membership events have been sent to which users, which feels like it might create a lot of additional complexity for homeserver implementors. It might just be better (certainly a lot simpler) to send the entire room state and deal with the duplicates.


The remote server may decide not to honour lazy_loading if a room is too small
(thus saving the additional roundtrip of calling `/state`), so the response to
`/send_join` or `/peek` must include a `lazy_load_members: true` field if the
state is partial and members need to be subsequently loaded by `/state`.

Clients which are not lazy loading members (by MSC1227) must block returning
Copy link
Member

Choose a reason for hiding this comment

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

what does it mean for a client to "block returning" an API?

the CS API `/join` or `/peek` until this `/state` has completed and been
processed.

Clients which are lazy loading members however may return the initial `/join`
or `/peek` before `/state` has completed. However, we need a way to tell
clients once the server has finished synchronising its local state. We do this
by adding an `syncing: true` field to the room's `state` block in the `/sync`
Copy link
Member

Choose a reason for hiding this comment

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

what exactly do clients do with this field? I thought that clients which do lazy-loading syncs were obliged to expect partial state blocks anyway?

Copy link
Member Author

Choose a reason for hiding this comment

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

they do expect partial state blocks, but they currently don't know that they're partial - and so then go and hit /members anyway to fill in the missing members. so this i think is fixing that thinko by giving the client a clear way to know that state is partial and they need to fill it in.

response. Once this field is missing or false, the client knows that the joining
server has fully synchronised the state for this room. Operations which are
blocked on state being fully synchronised are:

* Sending E2EE messages, otherwise some of the users will not have the keys
Copy link
Contributor

Choose a reason for hiding this comment

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

Something to consider here is that we can’t even start sending device list updates for users until we learn about those users, let alone exchanging keys, so this might create another protocol-level race when joining E2E rooms if you start sending messages into the room before you know about all the devices in the room (resulting in UTDs).

Copy link
Member

Choose a reason for hiding this comment

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

@neilalexander I'm not really following you here. You seem to be saying the same thing as the MSC (if a user sends E2E messages before they have the user list, their client will not know who to encrypt for).

to decrypt the message.
* Calling /members to get an accurate detailed list of the users in the room.
Instead clients showing a membership list should calculate it from the
members they do have, and the room summary (e.g. "these 5 heroes + 124 others")

While the joining server is busy syncing the remaining room members via
`/state`, it will also need to sync new inbound events to the user (and old
ones if the user calls `/messages`). If these events refer to members we're
not yet aware of (e.g. they're sent by a user our server hasn't lazyloaded
yet) we should separately retrieve their membership event so the server can
include it in the `/sync` response to the client. To do this, we add fields
to `/state` to let our server request a specific `type` and `state_key` from
the target server.

Matrix requires each server to track the full state rather than a partial
state in its DB for every event persisted in the DAG, in order to correctly
calculate resolved state as of that event for authorising events and servicing
/state queries etc. Loading the power events up front lets us authorise new
events (backfilled & new traffic) using partial state. However, once our
ara4n marked this conversation as resolved.
Show resolved Hide resolved
server has fully synced the state of the room at the point of the join event,
we must rollback the DAG and replay all the events we've accepted into the
room DAG in order to correctly capture the full state in the room as of that
event. This could theoretically result in some events now being
rejected/soft-failed, so it's important that "uncommitted" events in the DAG
(i.e. those which arrived since the join, but before state was fully synced)
do not have side-effects on the rest of the server (e.g. generate push) until
the room is fully synced.

XXX: what's an example of an event being failed/rejected during replay which
was previously accepted? If we could auth it correctly before, shouldn't it
still auth correctly afterwards?
ara4n marked this conversation as resolved.
Show resolved Hide resolved

## Alternatives

Rather than making this specific to membership events, we could lazy load all
state by default. However, it's challenging to know which events the server
ara4n marked this conversation as resolved.
Show resolved Hide resolved
(and clients) need up front in order to correctly handle the room - plus this
list may well change over time. For instance, do we need to know the
`uk.half-shot.bridge` event in the Stats section up front?

Rather than reactively pulling in missing membership events as needed while
the room is syncing in the background, we could require the server we're
joining via to proactively push us member events it knows we don't know about
yet, and save a roundtrip. This feels more fiddly though; we can optimise this
edge case if it's actually needed.

## Security considerations

We currently trust the server we join via to provide us with accurate room state.
This proposal doesn't make this any better or worse.
Comment on lines +152 to +153
Copy link
Member

Choose a reason for hiding this comment

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

per the above, I think it does, since we now trust lots of servers to give us accurate room state - at least while we're lazy-loading the state.


## Related

MSC1228 (and future variants) also will help speed up joining rooms
significantly, as you no longer have to query for server keys given the room
ID becomes a server's public key.

## Stats

```
matrix=> select type, count(*) from matrix.state_events where room_id='!OGEhHVWSdvArJzumhm:matrix.org' group by type order by count(*) desc;
type | count
---------------------------+-------
m.room.member | 30661
m.room.aliases | 141
m.room.server_acl | 23
m.room.join_rules | 9
m.room.guest_access | 6
m.room.power_levels | 5
m.room.history_visibility | 3
m.room.name | 1
m.room.related_groups | 1
m.room.avatar | 1
m.room.topic | 1
m.room.create | 1
uk.half-shot.bridge | 1
m.room.canonical_alias | 1
m.room.bot.options | 1
```