Peeking over federation via MSC2444 #1391
Conversation
doesn't yet compile or work. needs to actually add the peeking block into the sync response. checking in now before it gets any bigger, and to gather any initial feedback on the vague shape of it.
To use: set `DENDRITE_TRACE_SQL=1` then grep for `unsafe`
| return resErr | ||
| } | ||
|
|
||
| if !response.RoomExists { |
There was a problem hiding this comment.
We would've failed earlier as QueryRoomVersionForRoom would return a failure.
| } | ||
|
|
||
| type PerformOutboundPeekResponse struct { | ||
| LastError *gomatrix.HTTPError |
There was a problem hiding this comment.
Shouldn't we be getting gomatrixserverlib.RespPeek somewhere?
|
|
||
| // PerformInboundPeek handles peeking into matrix rooms, including over | ||
| // federation by talking to the federationsender. called when a remote server | ||
| // initiates a /peek over federation. |
There was a problem hiding this comment.
Feels bad that we don't call PerformInboundPeek when local clients call /peek like we do with all the other Perform stuff.
| // for now we just use the room ID again. In future, if we ever | ||
| // support concurrent peeks to the same room with different filters | ||
| // then we would need to disambiguate further. | ||
| peekID := roomID |
There was a problem hiding this comment.
Annoyingly the MSC forbids this because:
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.
| RoomID string | ||
| ServerName gomatrixserverlib.ServerName | ||
| CreationTimestamp int64 | ||
| RenewedTimestamp int64 |
There was a problem hiding this comment.
As N points out, it would be nicer for this to be gomatrixserverlib.Timestamp types (which are just ints)
| // it to the list of servers to try. | ||
| // handle federated peeks | ||
| // FIXME: don't create an outbound peek if we already have one going. | ||
| if domain != r.Cfg.Matrix.ServerName { |
There was a problem hiding this comment.
This won't work in cases where you create the room, other servers join then you leave the room then try to peek into it, as this will resolve to false. We shouldn't ever be switching on the domain of the room ID like this.
| * The `roomserver` will kick the `federationsender` much as it does for a federated `/join` in order to trigger a federated outbound `/peek` | ||
| * The `federationsender` tracks the existence of the outbound peek in in its federationsender_outbound_peeks table. | ||
| * The `federationsender` regularly renews the remote peek as long as there are still peeking devices syncing for it. | ||
| * TBD: how do we tell if there are no devices currently syncing for a given peeked room? The syncserver needs to tell the roomserver |
There was a problem hiding this comment.
Why do we care? We time out the peek anyway, and really just because there are no devices currently syncing doesn't mean we should stop peeking. Network interruptions are a reasonable reason, we should still be honouring the renewal period.
First wave of peeking over federation a la MSC2444 to see how it feels.
I think this is now good to be merged as an initial wave of work, to stop it bitrotting, even though all the housekeeping is missing.
{in,out}bound-peeks-tablesImplements matrix-org/matrix-spec-proposals#2444
Builds on #1370
Requires matrix-org/gomatrixserverlib#220