-
Notifications
You must be signed in to change notification settings - Fork 412
MSC3871: Gappy timelines #3871
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
Open
MadLittleMods
wants to merge
19
commits into
main
Choose a base branch
from
madlittlemods/gappy-timelines
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
MSC3871: Gappy timelines #3871
Changes from 7 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
90433c5
Initial gappy timelines MSC
MadLittleMods 8893661
Add MSC number
MadLittleMods fbcc872
Some cleanup
MadLittleMods b84ef77
Fix wrong field name
MadLittleMods 19eb6dd
Unwritten rule
MadLittleMods d142d05
Control behavior via query parameter
MadLittleMods 0fae14d
Advertise unstable support
MadLittleMods 84acfc6
Switch away from synthetic events
MadLittleMods 8b3f419
Fix typo
MadLittleMods 5f7bb0e
Update pagination token graphs
MadLittleMods 55444ec
Swap which direction example comes first to match the pagination toke…
MadLittleMods c1caf2e
Better example and fix flaw with needing to indicate gap at start of …
MadLittleMods c7b09b1
Add alternative of exposing prev_events to client
MadLittleMods 5394865
Update with new GapEntry format (paginate from both sides)
MadLittleMods 0a6cc0b
More robust unstable to stable rollout
MadLittleMods 0e1f021
Future consideration to add the gap treatment to /context
MadLittleMods 5af4c7f
Merge branch 'main' into madlittlemods/gappy-timelines
MadLittleMods 02194b0
Not required
MadLittleMods 3990314
Merge branch 'main' into madlittlemods/gappy-timelines
MadLittleMods File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,130 @@ | ||
# MSC3871: Gappy timeline | ||
|
||
`/messages` returns a linearized version of the event DAG. From any given | ||
homeservers perspective of the room, the DAG can have gaps where they're missing | ||
events. This could be because the homeserver hasn't fetched them yet or because | ||
it failed to fetch the events because those homeservers are unreachable and no | ||
one else knows about the event. | ||
|
||
Currently, there is an unwritten rule between the server and client that the | ||
server will always return all contiguous events in that part of the timeline. | ||
But the server has to break this rule sometimes when it doesn't have the event | ||
and is unable to get the event from anyone else. This MSC aims to change the | ||
dynamic so the server can give the client feedback and an indication of where | ||
the gaps are. | ||
|
||
This way, clients know where they are missing events and can even retry fetching | ||
by perhaps adding some UI to the timeline like "We failed to get some messages | ||
in this gap, try again." | ||
|
||
This can also make servers faster to respond to `/messages`. For example, | ||
currently, Synapse always tries to backfill and fill in the gap (even when it | ||
has enough messages locally to respond). In big rooms like `#matrix:matrix.org` | ||
kegsay marked this conversation as resolved.
Show resolved
Hide resolved
|
||
(Matrix HQ), almost every place you ask for has gaps in it (thousands of | ||
backwards extremities) and lots of those events are unreachable so we try the | ||
same thing over and over hoping the response will be different this time but | ||
instead, we just make the `/messages` response time slow. With this MSC, we can | ||
instead be more intelligent about backfilling in the background and just tell | ||
the client about the gap that they can retry fetching a little later. | ||
|
||
|
||
## Proposal | ||
|
||
Add a `?gaps_allowed=true` query parameter flag to `GET | ||
/_matrix/client/v3/rooms/{roomId}/messages` which allows usage of a | ||
`m.timeline.gap` indicator, that can be used in the `response` `chunk` list of | ||
events. There can be multiple gaps per response. | ||
|
||
|
||
### `m.timeline.gap` | ||
|
||
key | type | value | description | required | ||
--- | --- | --- | --- | --- | ||
`gap_start_event_id` | string | Event ID | The event ID that the homeserver is missing where the gap begins | yes | ||
`pagination_token` | string | Pagination token | A pagination token that represents the spot in the DAG after the missing `gap_start_event_id`. Useful when retrying to fetch the missing part of the timeline again via `/messages?dir=b&from=<pagination_token>` | yes | ||
|
||
Pagination tokens are positions between events. This already an established | ||
concept but to illustrate this better, see the following diagram: | ||
``` | ||
pagination_token | ||
| | ||
<oldest-in-time> [0]<--[1] <gap> [gap_start_event_id]▼<--[4]<--[5]<--[6] <newest-in-time> | ||
``` | ||
|
||
`m.timeline.gap` has a similar shape to a normal event so it's still easy to | ||
iterate over the `/messages` response and process but has no `event_id` itself | ||
MadLittleMods marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
so it should not be mistaken as a real event in the room. | ||
|
||
A full example of the `m.timeline.gap` indicator: | ||
|
||
```json | ||
{ | ||
"type": "m.timeline.gap", | ||
"content": { | ||
"gap_start_event_id": "$12345", | ||
"pagination_token": "t47409-4357353_219380_26003_2265", | ||
} | ||
} | ||
``` | ||
|
||
`/messages` response example with a gap: | ||
|
||
```json | ||
{ | ||
"chunk": [ | ||
{ | ||
"type": "m.room.message", | ||
"content": { | ||
"body": "foo", | ||
} | ||
}, | ||
{ | ||
"type": "m.timeline.gap", | ||
"content": { | ||
"gap_start_event_id": "$12345", | ||
"pagination_token": "t47409-4357353_219380_26003_2265", | ||
} | ||
}, | ||
{ | ||
"type": "m.room.message", | ||
"content": { | ||
"body": "baz", | ||
} | ||
}, | ||
] | ||
} | ||
``` | ||
|
||
|
||
## Potential issues | ||
|
||
Lots of gaps/extremities are generated when a spam attack occurs and federation | ||
falls behind. If clients start showing gaps with retry links, we might just be | ||
exposing the spam more. | ||
|
||
|
||
## Alternatives | ||
MadLittleMods marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
As an alternative, we can continue to do nothing as we do today and not worry | ||
about the occasional missing events. People seem not to notice any missing | ||
messages anyway but they do probably see our slow `/messages` pagination. | ||
|
||
|
||
|
||
## Security considerations | ||
|
||
Only your own homeserver controls whether a `m.timeline.gap` indicator is added to the | ||
message response and it isn't an event of the room so there shouldn't be any weird | ||
edge case where the gap is trying to get you to fetch spam or something. | ||
|
||
|
||
## Unstable prefix | ||
|
||
Servers will indicate support for the new endpoint via a true value for feature | ||
flag `org.matrix.msc3871` in `unstable_features` in the response to `GET | ||
/_matrix/client/versions`. | ||
|
||
While this feature is in development, it can be used as `GET | ||
/_matrix/client/unstable/org.matrix.msc3871/rooms/{roomId}/messages?gaps_allowed=true` | ||
|
||
MadLittleMods marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.