MSC2444: peeking over federation via /peek #2444
Conversation
Co-authored-by: Travis Ralston <travpc@gmail.com>
A first cut at an MSC to define how to massively speed up joins (and MSC #2444 peeks) by sending irrelevant events after you join rather than up front
| XXX: it might be better to split this into two operations: first fetch the | ||
| state data, then begin the peek operation by sending your idea of the forward | ||
| extremities, to bring you up to date with anything you missed. This would | ||
| reduce the chance of having to immediately cancel a peek, and would be more | ||
| efficient in the case of rapid `peek-unpeek-peek` switches. |
There was a problem hiding this comment.
I'm leaving this open for now, pending work on an implementation, which I hope will provide guidance.
| ### Joining a room | ||
|
|
||
| When the user joins the peeked room, the peeking server should just emit the | ||
| right membership event rather than calling `/make_join` or `/send_join`, to |
| of idempotency. The ID must be unique for a given `{ peeking_server, room_id, | ||
| target_server }` tuple, and should be a string consisting of the characters | ||
| `[0-9a-zA-Z.=_-]`. Its length must not exceed 8 characters and it should not be | ||
| empty. |
There was a problem hiding this comment.
Can we relax these requirements? Reasoning is that we often just want to use the room ID as the peek ID as in https://github.com/matrix-org/dendrite/pull/1391/files#r561821829
There was a problem hiding this comment.
why not use a fixed constant, given it is scoped to room id anyway?
There was a problem hiding this comment.
(I have a vague memory that I convinced myself that you needed to support more than one-peek-per-room and hence it was insufficient to use either a fixed constant or room id, but in any case I can't see the advantage of using room id rather than a fixed MY_PEEK)
| "content": { /* ... */ } | ||
| } | ||
| ], | ||
| "renewal_interval": 3600000 |
There was a problem hiding this comment.
| "renewal_interval": 3600000 | |
| "room_version": "6", | |
| "renewal_interval": 3600000 |
| `[0-9a-zA-Z.=_-]`. Its length must not exceed 8 characters and it should not be | ||
| empty. |
There was a problem hiding this comment.
8 chars is too few for large instances.
| `[0-9a-zA-Z.=_-]`. Its length must not exceed 8 characters and it should not be | |
| empty. | |
| `[0-9a-zA-Z.=_-]`. It shall not be empty. |
turt2live
moved this from Awaiting SCT input
to Temp column 001
in Spec Core Team Backlog
turt2live
added
the
needs-implementation
| * You can't use rooms as generic pubsub mechanisms for synchronising data like | ||
| profiles, groups, reputation lists, device-lists etc if you can't peek into | ||
| them remotely. | ||
| * Matrix-speaking search engines can't work if they can't peek remote rooms. |
There was a problem hiding this comment.
This is also inconvenient for restricted spaces where the space just appears as an internal room ID before you try to join.
| * listed in the `auth_events` field of any of the above events, or: | ||
| * listed in the `auth_events` of the `auth_events`, recursively. | ||
|
|
||
| * `renewal_interval`: a duration in milliseconds after which the target server |
There was a problem hiding this comment.
Wouldn't seconds make more sense here? I don't think any value under a second is going to be reasonably useful.
| * `common_state_ids`: A list of the IDs of any events which are common to the room | ||
| states after *all* of the forward extremities in the room. | ||
|
|
||
| * `events`: The bodies of any events whose IDs are: |
There was a problem hiding this comment.
Arent these, in effect, only state events then? The language makes it ambiguous as to what events, exactly, would be included here.
Also, the language makes it uncertain as to what the exact body would look like of these events, should these be stripped down state events (like StrippedState as referenced under /sync)?
| "events": [ | ||
| { | ||
| "type": "m.room.member", | ||
| "room_id": "!somewhere:example.org", |
There was a problem hiding this comment.
Wouldn't room_id be redundant here, considering we were querying the room id in the first place?
Also, see my note about the layout of events here (in another comment).
| If the room is not peekable, the target server should return a 403 error with | ||
| `M_FORBIDDEN`. |
There was a problem hiding this comment.
Under which conditions? A room has to be globally readable? (for explicitness' sake)
| If the room is not known to the target server, it should return a 404 error | ||
| with `M_NOT_FOUND`. |
There was a problem hiding this comment.
Can't a server leak metadata about having joined a room by another server trying every known room ID on all servers it knows about? Shouldn't plausible deniability (always 404) be the default response here?
| * MSC1777 offers a solution for EDU transmission which this MSC does not, | ||
| given we don't currently have any data flows for mirroring other servers' | ||
| EDUs. |
There was a problem hiding this comment.
I question how useful some EDUs (such as typing, or even read receipts) would be in a peeked room. Unless an EDU type is added which is also critical to a room's data, I don't foresee this being a problem.
| * It may be useful to allow peeking servers to "filter" the events to be | ||
| returned - for example, if you only care about particular events, or | ||
| particular servers - e.g. if load-balancing peeking via multiple servers. |
There was a problem hiding this comment.
About filters; Is it still useful to accommodate for them in a forward-compatible manner? I think that just opening a singular peek to a room, without peek IDs, would be a best way forward for now, it'd be easy to add in the /{peek_id}/ "peek_id": "{peek_id}" parts of the API later on, right now they feel like incomplete elements which a future MSC defining filters should instead be adding on.
| * A malicious server could set up multiple peeks to multiple target servers by | ||
| way of attempting a denial-of-service attack. Server implementations should | ||
| rate-limit requests to establish peeks, as well as limiting the absolute | ||
| number of active peeks each server may have, to mitigate this. |
There was a problem hiding this comment.
Some care should be taken when ratelimiting legitimate-interest rooms, such as profiles, if a large server is interested in the profiles of another large server. Maybe these ratelimits should apply to normal rooms that users peek into.
There was a problem hiding this comment.
Additionally, a server might also deny peeking into a profile room if the server shares no rooms with the other server that that requested-profile-user is also into, for extra privacy protection. This may be worth noting.
| How do we handle backpressure or rate limiting on the event stream (if at | ||
| all?) |
There was a problem hiding this comment.
(I think this is missing an XXX:)
There was a problem hiding this comment.
Also, re: the comment; I think currently it should be handled like normal federation mechanics, where delays in /send requests already sorta back-pressure other servers sending federation events. This does it on a server-to-server basis, not a room basis, but discussing solutions to that is out of scope for this MSC.
| How do we handle backpressure or rate limiting on the event stream (if at | ||
| all?) | ||
|
|
||
| ## Dependencies |
There was a problem hiding this comment.
(Shouldn't this be "dependants"? :P)
A new MSC for proposing how peeking over federation could work, hopefully replacing #1777
This one introduces the idea of a
/peekAPI in S2S which lets a peeking server subscribe to events from given server(s) so it can have a read-only peek view of peekable rooms. This is based on the review of #1777 in Oct 2019.Rendered
Obsoletes #1777