From 90433c596d0b4e697d38e0fe5633532f6d2f7e80 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 17 Aug 2022 21:32:38 -0500 Subject: [PATCH 01/17] Initial gappy timelines MSC --- proposals/0000-gappy-timelines.md | 125 ++++++++++++++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 proposals/0000-gappy-timelines.md diff --git a/proposals/0000-gappy-timelines.md b/proposals/0000-gappy-timelines.md new file mode 100644 index 00000000..b951497d --- /dev/null +++ b/proposals/0000-gappy-timelines.md @@ -0,0 +1,125 @@ +# MSC0000: 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 expectation(TODO: better word) 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 promise(TODO: match +word above) sometimes when it doesn't have the event and is unable to get the +event from anyone else and the client is unaware. 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` +(Matrix HQ), almost every place you ask for has gaps in it (thousands of +backwards extremities) and lots 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 `m.timeline.gap` indicator, that can be used in the `chunk` list of events +from a `GET /_matrix/client/v3/rooms/{roomId}/messages` response. 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=` | yes + +Pagination tokens are positions between events. This already an established +concept but to illustrate this better, see the following diagram: +``` + pagination_token + | + [0]<--[1] [gap_start_event_id]▼<--[4]<--[5]<--[6] +``` + +`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 +so it should not be mistaken as a real event in the room. + +A full example of the `m.timeline.gap` indicator: + +```json5 +{ + "type": "m.timeline.gap", + "content": { + "gap_start_event_id": "$12345", + "pagination_token": "t47409-4357353_219380_26003_2265", + } +}, +``` + +`/messages` response example with a gap: + +```json5 +{ + "chunk": [ + { + "type": "m.room.message", + "content": { + "body": "foo", + } + }, + { + "type": "m.timeline.gap", + "content": { + "gap_start_event": "$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 + +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 + +The `m.timeline.gap` indicator can be used in the `org.matrix.mscXXXX` room version. + + From 889366170fe6f7f68081c013aeac5daca6bdb5ba Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 17 Aug 2022 21:36:09 -0500 Subject: [PATCH 02/17] Add MSC number --- .../{0000-gappy-timelines.md => 3871-gappy-timelines.md} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename proposals/{0000-gappy-timelines.md => 3871-gappy-timelines.md} (98%) diff --git a/proposals/0000-gappy-timelines.md b/proposals/3871-gappy-timelines.md similarity index 98% rename from proposals/0000-gappy-timelines.md rename to proposals/3871-gappy-timelines.md index b951497d..e6cf39e6 100644 --- a/proposals/0000-gappy-timelines.md +++ b/proposals/3871-gappy-timelines.md @@ -1,4 +1,4 @@ -# MSC0000: Gappy timeline +# 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 @@ -120,6 +120,6 @@ edge case where the gap is trying to get you to fetch spam or something. ## Unstable prefix -The `m.timeline.gap` indicator can be used in the `org.matrix.mscXXXX` room version. +The `m.timeline.gap` indicator can be used in the `org.matrix.msc3871` room version. From fbcc872a694e2e3a4b6be296f866ed50c12367a8 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 17 Aug 2022 21:48:33 -0500 Subject: [PATCH 03/17] Some cleanup --- proposals/3871-gappy-timelines.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/proposals/3871-gappy-timelines.md b/proposals/3871-gappy-timelines.md index e6cf39e6..ceeb25ad 100644 --- a/proposals/3871-gappy-timelines.md +++ b/proposals/3871-gappy-timelines.md @@ -10,11 +10,11 @@ Currently, there is an unwritten expectation(TODO: better word) 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 promise(TODO: match word above) sometimes when it doesn't have the event and is unable to get the -event from anyone else and the client is unaware. This MSC aims to change 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 +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." @@ -22,11 +22,11 @@ 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` (Matrix HQ), almost every place you ask for has gaps in it (thousands of -backwards extremities) and lots 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. +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 @@ -57,19 +57,19 @@ so it should not be mistaken as a real event in the room. A full example of the `m.timeline.gap` indicator: -```json5 +```json { "type": "m.timeline.gap", "content": { "gap_start_event_id": "$12345", "pagination_token": "t47409-4357353_219380_26003_2265", } -}, +} ``` `/messages` response example with a gap: -```json5 +```json { "chunk": [ { From b84ef7736c8de396355e80e283458037c146e7ce Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 18 Aug 2022 10:35:58 -0500 Subject: [PATCH 04/17] Fix wrong field name --- proposals/3871-gappy-timelines.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/3871-gappy-timelines.md b/proposals/3871-gappy-timelines.md index ceeb25ad..6ed85b6d 100644 --- a/proposals/3871-gappy-timelines.md +++ b/proposals/3871-gappy-timelines.md @@ -81,7 +81,7 @@ A full example of the `m.timeline.gap` indicator: { "type": "m.timeline.gap", "content": { - "gap_start_event": "$12345", + "gap_start_event_id": "$12345", "pagination_token": "t47409-4357353_219380_26003_2265", } }, From 19eb6ddbe771c8efaae9c0f28717d9acfa23700c Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 19 Aug 2022 14:27:04 -0500 Subject: [PATCH 05/17] Unwritten rule See https://github.com/matrix-org/matrix-spec-proposals/pull/3871#discussion_r948598275 --- proposals/3871-gappy-timelines.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/proposals/3871-gappy-timelines.md b/proposals/3871-gappy-timelines.md index 6ed85b6d..75a4be7a 100644 --- a/proposals/3871-gappy-timelines.md +++ b/proposals/3871-gappy-timelines.md @@ -6,11 +6,10 @@ 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 expectation(TODO: better word) 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 promise(TODO: match -word above) sometimes when it doesn't have the event and is unable to get the -event from anyone else. This MSC aims to change the +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. From d142d056c73494f3add540ed2e0939ed0a9322a7 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 19 Aug 2022 14:35:16 -0500 Subject: [PATCH 06/17] Control behavior via query parameter See https://github.com/matrix-org/matrix-spec-proposals/pull/3871#discussion_r949933365 --- proposals/3871-gappy-timelines.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/proposals/3871-gappy-timelines.md b/proposals/3871-gappy-timelines.md index 75a4be7a..d0b2765d 100644 --- a/proposals/3871-gappy-timelines.md +++ b/proposals/3871-gappy-timelines.md @@ -30,9 +30,10 @@ the client about the gap that they can retry fetching a little later. ## Proposal -Add a `m.timeline.gap` indicator, that can be used in the `chunk` list of events -from a `GET /_matrix/client/v3/rooms/{roomId}/messages` response. There can be -multiple gaps per response. +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` @@ -119,6 +120,7 @@ edge case where the gap is trying to get you to fetch spam or something. ## Unstable prefix -The `m.timeline.gap` indicator can be used in the `org.matrix.msc3871` room version. +While this feature is in development, it can be used as `GET +/_matrix/client/unstable/org.matrix.msc3871/rooms/{roomId}/messages?gaps_allowed=true` From 0fae14d3e7ebc585c2aeb4b77a2dd439adb9a8d1 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 22 Aug 2022 16:44:49 -0500 Subject: [PATCH 07/17] Advertise unstable support --- proposals/3871-gappy-timelines.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/proposals/3871-gappy-timelines.md b/proposals/3871-gappy-timelines.md index d0b2765d..33e7cb77 100644 --- a/proposals/3871-gappy-timelines.md +++ b/proposals/3871-gappy-timelines.md @@ -120,6 +120,10 @@ 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` From 84acfc69bfaaa77bb883d1be7d7066b8b6986878 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 7 Oct 2022 16:33:41 -0500 Subject: [PATCH 08/17] Switch away from synthetic events As suggested by @benkuly, https://github.com/matrix-org/matrix-spec-proposals/pull/3871#discussion_r960928194 The synthetic event approach is still listed as an alternative --- proposals/3871-gappy-timelines.md | 130 +++++++++++++++++++++++------- 1 file changed, 99 insertions(+), 31 deletions(-) diff --git a/proposals/3871-gappy-timelines.md b/proposals/3871-gappy-timelines.md index 33e7cb77..a03c2255 100644 --- a/proposals/3871-gappy-timelines.md +++ b/proposals/3871-gappy-timelines.md @@ -30,17 +30,27 @@ 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. +Add a `gaps` field to the response of [`GET +/_matrix/client/v3/rooms/{roomId}/messages`](https://spec.matrix.org/v1.1/client-server-api/#get_matrixclientv3roomsroomidmessages). +This field is an array of `GapEntry` indicating where missing events in the +timeline are as defined below. -### `m.timeline.gap` +### 200 response + +Thid describes the new `gaps` response field being added to the `200 response` +of `/messages`: + +Name | Type | Description | required +--- | --- | --- | --- +`gaps` | `[GapEntry]` | A list of gaps indicating where events are missing in the `chunk` | no + + +#### `GapEntry` 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 +`next_to_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=` | yes Pagination tokens are positions between events. This already an established @@ -51,46 +61,68 @@ concept but to illustrate this better, see the following diagram: [0]<--[1] [gap_start_event_id]▼<--[4]<--[5]<--[6] ``` -`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 -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 examples -`/messages` response example with a gap: +`/messages?dir=f` response example with a gap (`chunk` has events in +chronoligcal order): -```json +```json5 { "chunk": [ { + "event_id": "$foo", "type": "m.room.message", "content": { "body": "foo", } }, + // { - "type": "m.timeline.gap", + "event_id": "$baz", + "type": "m.room.message", "content": { - "gap_start_event_id": "$12345", - "pagination_token": "t47409-4357353_219380_26003_2265", + "body": "baz", } }, + ] + "gaps": [ + { + "next_to_event_id": "$foo", + "pagination_token": "t47402-4357353_219380_26003_2265", + } + ] +} +``` + + +`/messages?dir=b` response example with a gap (`chunk` has events in +reverse-chronoligcal order): + +```json5 +{ + "chunk": [ { + "event_id": "$baz", "type": "m.room.message", "content": { "body": "baz", } }, + // + { + "event_id": "$foo", + "type": "m.room.message", + "content": { + "body": "foo", + } + } + ] + "gaps": [ + { + "next_to_event_id": "$baz", + "pagination_token": "t47403-4357353_219380_26003_2265", + } ] } ``` @@ -110,21 +142,57 @@ about the occasional missing events. People seem not to notice any missing messages anyway but they do probably see our slow `/messages` pagination. +### Synthetic `m.timeline.gap` event alternative + +Another alternative is using synthetic events (thing that looks like an event +without an `event_id`) which the server inserts alongside other events in the +`chunk` to indicate where the gap is. But this has detractors since it's harder +to implement in strongly typed SDK's and easy for a client to naively display +every "event" in the `chunk`. + +`/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", + } + }, + ] +} +``` + ## 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. +Only your own homeserver controls whether a gap is added to the `/messages` +response so there shouldn't be any weird edge case where someone else can +control whether you to fetch something. ## Unstable prefix -Servers will indicate support for the new endpoint via a true value for feature +Servers will indicate support for this MSC 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` +While this feature is in development, the `gaps` field can be used as +`org.matrix.msc3871.gaps` From 8b3f419a133404f459faf544942a8c397f6520d1 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 7 Oct 2022 16:36:08 -0500 Subject: [PATCH 09/17] Fix typo --- proposals/3871-gappy-timelines.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/3871-gappy-timelines.md b/proposals/3871-gappy-timelines.md index a03c2255..2f0155b9 100644 --- a/proposals/3871-gappy-timelines.md +++ b/proposals/3871-gappy-timelines.md @@ -38,7 +38,7 @@ timeline are as defined below. ### 200 response -Thid describes the new `gaps` response field being added to the `200 response` +This describes the new `gaps` response field being added to the `200 response` of `/messages`: Name | Type | Description | required From 5f7bb0ecb09cc0ac4cc13cf1a7bec2a3758ffcd1 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 7 Oct 2022 16:46:02 -0500 Subject: [PATCH 10/17] Update pagination token graphs --- proposals/3871-gappy-timelines.md | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/proposals/3871-gappy-timelines.md b/proposals/3871-gappy-timelines.md index 2f0155b9..1e8790c0 100644 --- a/proposals/3871-gappy-timelines.md +++ b/proposals/3871-gappy-timelines.md @@ -56,14 +56,19 @@ key | type | value | description | required Pagination tokens are positions between events. This already an established concept but to illustrate this better, see the following diagram: ``` - pagination_token - | - [0]<--[1] [gap_start_event_id]▼<--[4]<--[5]<--[6] + pagination_token + | + [0]<--[1]<-- ▼ <--[4 (next_to_event_id)]<--[5]<--[6] ``` +The idea is to be able to keep paginating from `pagination_token` in the same +direction of the request to fill in the gap. + ### `/messages` response examples +#### `/messages?dir=f` + `/messages?dir=f` response example with a gap (`chunk` has events in chronoligcal order): @@ -95,6 +100,15 @@ chronoligcal order): } ``` +``` + pagination_token + | + [foo (next_to_event_id)]<-- ▼ <--[baz] +``` + + +#### `/messages?dir=b` + `/messages?dir=b` response example with a gap (`chunk` has events in reverse-chronoligcal order): @@ -127,6 +141,13 @@ reverse-chronoligcal order): } ``` +``` + pagination_token + | + [foo]<-- ▼ <--[baz (next_to_event_id)] +``` + + ## Potential issues From 55444ecfa38b1500747c269c28792f75b23d4201 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 7 Oct 2022 16:48:15 -0500 Subject: [PATCH 11/17] Swap which direction example comes first to match the pagination token explanation just above --- proposals/3871-gappy-timelines.md | 54 +++++++++++++++---------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/proposals/3871-gappy-timelines.md b/proposals/3871-gappy-timelines.md index 1e8790c0..b778ab50 100644 --- a/proposals/3871-gappy-timelines.md +++ b/proposals/3871-gappy-timelines.md @@ -67,84 +67,84 @@ direction of the request to fill in the gap. ### `/messages` response examples -#### `/messages?dir=f` +#### `/messages?dir=b` -`/messages?dir=f` response example with a gap (`chunk` has events in -chronoligcal order): + +`/messages?dir=b` response example with a gap (`chunk` has events in +reverse-chronoligcal order): ```json5 { "chunk": [ { - "event_id": "$foo", + "event_id": "$baz", "type": "m.room.message", "content": { - "body": "foo", + "body": "baz", } }, // { - "event_id": "$baz", + "event_id": "$foo", "type": "m.room.message", "content": { - "body": "baz", + "body": "foo", } - }, + } ] "gaps": [ { - "next_to_event_id": "$foo", - "pagination_token": "t47402-4357353_219380_26003_2265", + "next_to_event_id": "$baz", + "pagination_token": "t47403-4357353_219380_26003_2265", } ] } ``` ``` - pagination_token - | - [foo (next_to_event_id)]<-- ▼ <--[baz] + pagination_token + | + [foo]<-- ▼ <--[baz (next_to_event_id)] ``` -#### `/messages?dir=b` - +#### `/messages?dir=f` -`/messages?dir=b` response example with a gap (`chunk` has events in -reverse-chronoligcal order): +`/messages?dir=f` response example with a gap (`chunk` has events in +chronoligcal order): ```json5 { "chunk": [ { - "event_id": "$baz", + "event_id": "$foo", "type": "m.room.message", "content": { - "body": "baz", + "body": "foo", } }, // { - "event_id": "$foo", + "event_id": "$baz", "type": "m.room.message", "content": { - "body": "foo", + "body": "baz", } - } + }, ] "gaps": [ { - "next_to_event_id": "$baz", - "pagination_token": "t47403-4357353_219380_26003_2265", + "next_to_event_id": "$foo", + "pagination_token": "t47402-4357353_219380_26003_2265", } ] } ``` ``` - pagination_token - | - [foo]<-- ▼ <--[baz (next_to_event_id)] + pagination_token + | + [foo (next_to_event_id)]<-- ▼ <--[baz] ``` From c1caf2eb95b6883619b884f4abd1130c8538ba85 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 25 Oct 2022 18:11:46 -0500 Subject: [PATCH 12/17] Better example and fix flaw with needing to indicate gap at start of response See https://github.com/matrix-org/matrix-spec-proposals/pull/3871#discussion_r1005038309 --- proposals/3871-gappy-timelines.md | 95 +++++++++++++++++++------------ 1 file changed, 60 insertions(+), 35 deletions(-) diff --git a/proposals/3871-gappy-timelines.md b/proposals/3871-gappy-timelines.md index b778ab50..65e39834 100644 --- a/proposals/3871-gappy-timelines.md +++ b/proposals/3871-gappy-timelines.md @@ -50,32 +50,49 @@ Name | Type | Description | required key | type | value | description | required --- | --- | --- | --- | --- -`next_to_event_id` | string | Event ID | The event ID that the homeserver is missing where the gap begins | yes +`next_to_event_id` | string | Event ID | The event ID indicating the position in the `/messages` `"chunk"` response where the gap starts after that position. This field can be `null` or completely omitted to indicate that the gap is at the start of the `/messages` `"chunk"` | no `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=` | yes -Pagination tokens are positions between events. This already an established -concept but to illustrate this better, see the following diagram: -``` - pagination_token - | - [0]<--[1]<-- ▼ <--[4 (next_to_event_id)]<--[5]<--[6] + +### `/messages` response examples + +The following mermaid diagram represents the room DAG snapshot used for the following +`/messages` responses. The slightly transparent events with no background are events +that the homeserver does not have and are in the gap. + +Pagination tokens are positions between events. This already an established concept but +to illustrate this better, see the following `tX` pagination tokens in the following +diagram. + +```mermaid +flowchart RL + after[newest events...]:::gap-event -->|t10| fred -->|t9| waldo:::gap-event -->|t8| garply -->|t7| grault:::gap-event -->|t6| corge -->|t5| qux:::gap-event -->|t4| baz -->|t3| bar:::gap-event -->|t2| foo -->|t1| before[oldest events...]:::gap-event + + classDef gap-event opacity:0.8,fill:transparent; ``` The idea is to be able to keep paginating from `pagination_token` in the same direction of the request to fill in the gap. -### `/messages` response examples - #### `/messages?dir=b` +`/messages?dir=b` response example with gaps (`chunk` has events in +reverse-chronoligcal order since we're paginating backwards): -`/messages?dir=b` response example with a gap (`chunk` has events in -reverse-chronoligcal order): - +`/messages?dir=b&from=t6` ```json5 { "chunk": [ + // there is no gap from `t6` to `$corge` as expected + { + "event_id": "$corge", + "type": "m.room.message", + "content": { + "body": "corge", + } + }, + // { "event_id": "$baz", "type": "m.room.message", @@ -83,7 +100,7 @@ reverse-chronoligcal order): "body": "baz", } }, - // + // { "event_id": "$foo", "type": "m.room.message", @@ -91,62 +108,70 @@ reverse-chronoligcal order): "body": "foo", } } + // ] "gaps": [ + { + "next_to_event_id": "$corge", + "pagination_token": "t5", + }, { "next_to_event_id": "$baz", - "pagination_token": "t47403-4357353_219380_26003_2265", + "pagination_token": "t3", + }, + { + "next_to_event_id": "$foo", + "pagination_token": "t1", } ] } ``` -``` - pagination_token - | - [foo]<-- ▼ <--[baz (next_to_event_id)] -``` - #### `/messages?dir=f` -`/messages?dir=f` response example with a gap (`chunk` has events in -chronoligcal order): +`/messages?dir=f` response example with gaps (`chunk` has events in +chronoligcal order since we're paginating forwards): +`/messages?dir=f&from=t6` ```json5 { "chunk": [ + // { - "event_id": "$foo", + "event_id": "$garply", "type": "m.room.message", "content": { - "body": "foo", + "body": "garply", } }, - // + // { - "event_id": "$baz", + "event_id": "$fred", "type": "m.room.message", "content": { - "body": "baz", + "body": "fred", } }, + // ] "gaps": [ { - "next_to_event_id": "$foo", - "pagination_token": "t47402-4357353_219380_26003_2265", + "next_to_event_id": null, + "pagination_token": "t6", + }, + { + "next_to_event_id": "$garply", + "pagination_token": "t8", + }, + { + "next_to_event_id": "$fred", + "pagination_token": "t10", } ] } ``` -``` - pagination_token - | - [foo (next_to_event_id)]<-- ▼ <--[baz] -``` - ## Potential issues From c7b09b11ad07a6af528411c4e945e23cc2f127fa Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 2 May 2023 15:11:47 -0500 Subject: [PATCH 13/17] Add alternative of exposing prev_events to client See https://github.com/matrix-org/matrix-spec-proposals/pull/3871#discussion_r958849276 --- proposals/3871-gappy-timelines.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/proposals/3871-gappy-timelines.md b/proposals/3871-gappy-timelines.md index 65e39834..642e9d13 100644 --- a/proposals/3871-gappy-timelines.md +++ b/proposals/3871-gappy-timelines.md @@ -188,6 +188,27 @@ about the occasional missing events. People seem not to notice any missing messages anyway but they do probably see our slow `/messages` pagination. +### Expose `prev_events` to the client + +One alternative is including the `prev_events` in the events that the client sees so +they can figure out the DAG chain themselves and see if there is an missing event in the +middle. + +There is an [unspecced `/messages?raw=true` query parameter in +Synapse](https://github.com/matrix-org/synapse/blob/20c76cecb9eb84dadfa7b2d25b436d3ab9218a1a/synapse/rest/client/room.py#L653) +that returns the full raw event as seen over federation which means it will include the +`prev_events`. + +You can also specify `event_format: federation` directly in that JSON `filter` parameter +of `/messages` -> +`/_matrix/client/v3/rooms/{room_id}}/messages?dir=b&filter=%7B%22event_format%22%3A%20%22federation%22%7D` + +Related to: + + - https://github.com/matrix-org/matrix-spec/issues/859 + - https://github.com/matrix-org/matrix-spec/issues/1047 + + ### Synthetic `m.timeline.gap` event alternative Another alternative is using synthetic events (thing that looks like an event From 5394865e89b3fb90a150dc98d4c4541bd4170b87 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 2 May 2023 15:56:18 -0500 Subject: [PATCH 14/17] Update with new GapEntry format (paginate from both sides) See https://github.com/matrix-org/matrix-spec-proposals/pull/3871#discussion_r1005280263 --- proposals/3871-gappy-timelines.md | 196 ++++++++++++++++++++++++++---- 1 file changed, 171 insertions(+), 25 deletions(-) diff --git a/proposals/3871-gappy-timelines.md b/proposals/3871-gappy-timelines.md index 642e9d13..226eaa43 100644 --- a/proposals/3871-gappy-timelines.md +++ b/proposals/3871-gappy-timelines.md @@ -50,8 +50,9 @@ Name | Type | Description | required key | type | value | description | required --- | --- | --- | --- | --- -`next_to_event_id` | string | Event ID | The event ID indicating the position in the `/messages` `"chunk"` response where the gap starts after that position. This field can be `null` or completely omitted to indicate that the gap is at the start of the `/messages` `"chunk"` | no -`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=` | yes +`event_id` | string | Event ID | The event ID indicating the position in the `/messages` `chunk` response | yes +`prev_pagination_token` | string | Pagination token | A pagination token that represents the spot in the DAG before the given `event_id` in the `chunk` | yes +`next_pagination_token` | string | Pagination token | A pagination token that represents the spot in the DAG after the given `event_id` in the `chunk` | yes ### `/messages` response examples @@ -71,8 +72,9 @@ flowchart RL classDef gap-event opacity:0.8,fill:transparent; ``` -The idea is to be able to keep paginating from `pagination_token` in the same -direction of the request to fill in the gap. +The idea is to be able to keep paginating from +`prev_pagination_token`/`next_pagination_token` in the respective direction to fill in +the gap. #### `/messages?dir=b` @@ -84,7 +86,6 @@ reverse-chronoligcal order since we're paginating backwards): ```json5 { "chunk": [ - // there is no gap from `t6` to `$corge` as expected { "event_id": "$corge", "type": "m.room.message", @@ -92,7 +93,6 @@ reverse-chronoligcal order since we're paginating backwards): "body": "corge", } }, - // { "event_id": "$baz", "type": "m.room.message", @@ -100,7 +100,6 @@ reverse-chronoligcal order since we're paginating backwards): "body": "baz", } }, - // { "event_id": "$foo", "type": "m.room.message", @@ -108,20 +107,22 @@ reverse-chronoligcal order since we're paginating backwards): "body": "foo", } } - // ] "gaps": [ { - "next_to_event_id": "$corge", - "pagination_token": "t5", + "prev_pagination_token": "t6", + "event_id": "$corge", + "next_pagination_token": "t5", }, { - "next_to_event_id": "$baz", - "pagination_token": "t3", + "prev_pagination_token": "t4", + "event_id": "$baz", + "next_pagination_token": "t3", }, { - "next_to_event_id": "$foo", - "pagination_token": "t1", + "prev_pagination_token": "t2", + "event_id": "$foo", + "next_pagination_token": "t1", } ] } @@ -137,7 +138,6 @@ chronoligcal order since we're paginating forwards): ```json5 { "chunk": [ - // { "event_id": "$garply", "type": "m.room.message", @@ -145,7 +145,6 @@ chronoligcal order since we're paginating forwards): "body": "garply", } }, - // { "event_id": "$fred", "type": "m.room.message", @@ -153,20 +152,17 @@ chronoligcal order since we're paginating forwards): "body": "fred", } }, - // ] "gaps": [ { - "next_to_event_id": null, - "pagination_token": "t6", + "prev_pagination_token": "t7", + "event_id": "$garply", + "next_pagination_token": "t8", }, { - "next_to_event_id": "$garply", - "pagination_token": "t8", - }, - { - "next_to_event_id": "$fred", - "pagination_token": "t10", + "prev_pagination_token": "t9", + "event_id": "$fred", + "next_pagination_token": "t10", } ] } @@ -246,6 +242,156 @@ every "event" in the `chunk`. ``` +### `GapEntry` alternative only indicating a gap `next_to_event_id` (only one side) + +Same concept as the existing `GapEntry` proposal but we only indicate the gap on one +side of an event `next_to_event_id` according to the direction that `/messages` is going +already. + +The problem with this alternative is that clients store events differently and it's +valid to want to paginate in either direction from a given event. This alternative works +fine in the Element Web case where you always paginate backwards in the scrollback and +store events as a whole timeline list but another client like the [Trixinity +SDK](https://github.com/benkuly/trixnity), where events are stored individually in a +linked list, where each event could have a gap before and after, and where a gap could +be 100's, 1000's of events wide, it would be useful to paginate from both ends to fill +the gap faster. + +
+ + Details for the GapEntry alternative only indicating a gap next_to_event_id + + +#### `GapEntry` + +key | type | value | description | required +--- | --- | --- | --- | --- +`next_to_event_id` | string | Event ID | The event ID indicating the position in the `/messages` `"chunk"` response where the gap starts after that position. This field can be `null` or completely omitted to indicate that the gap is at the start of the `/messages` `"chunk"` | no +`pagination_token` | string | Pagination token | A pagination token that represents the spot in the DAG to be able to continue paginating in the same direction as the request and fill in the gap from `next_to_event_id` to the next known event. | yes + + +### `/messages` response examples + +The following mermaid diagram represents the room DAG snapshot used for the following +`/messages` responses. The slightly transparent events with no background are events +that the homeserver does not have and are in the gap. + +Pagination tokens are positions between events. This already an established concept but +to illustrate this better, see the following `tX` pagination tokens in the following +diagram. + +```mermaid +flowchart RL + after[newest events...]:::gap-event -->|t10| fred -->|t9| waldo:::gap-event -->|t8| garply -->|t7| grault:::gap-event -->|t6| corge -->|t5| qux:::gap-event -->|t4| baz -->|t3| bar:::gap-event -->|t2| foo -->|t1| before[oldest events...]:::gap-event + + classDef gap-event opacity:0.8,fill:transparent; +``` + +The idea is to be able to keep paginating from `pagination_token` in the same +direction of the request to fill in the gap. + + +#### `/messages?dir=b` + +`/messages?dir=b` response example with gaps (`chunk` has events in +reverse-chronoligcal order since we're paginating backwards): + +`/messages?dir=b&from=t6` +```json5 +{ + "chunk": [ + // there is no gap from `t6` to `$corge` as expected + { + "event_id": "$corge", + "type": "m.room.message", + "content": { + "body": "corge", + } + }, + // + { + "event_id": "$baz", + "type": "m.room.message", + "content": { + "body": "baz", + } + }, + // + { + "event_id": "$foo", + "type": "m.room.message", + "content": { + "body": "foo", + } + } + // + ] + "gaps": [ + { + "next_to_event_id": "$corge", + "pagination_token": "t5", + }, + { + "next_to_event_id": "$baz", + "pagination_token": "t3", + }, + { + "next_to_event_id": "$foo", + "pagination_token": "t1", + } + ] +} +``` + + +#### `/messages?dir=f` + +`/messages?dir=f` response example with gaps (`chunk` has events in +chronoligcal order since we're paginating forwards): + +`/messages?dir=f&from=t6` +```json5 +{ + "chunk": [ + // + { + "event_id": "$garply", + "type": "m.room.message", + "content": { + "body": "garply", + } + }, + // + { + "event_id": "$fred", + "type": "m.room.message", + "content": { + "body": "fred", + } + }, + // + ] + "gaps": [ + { + "next_to_event_id": null, + "pagination_token": "t6", + }, + { + "next_to_event_id": "$garply", + "pagination_token": "t8", + }, + { + "next_to_event_id": "$fred", + "pagination_token": "t10", + } + ] +} +``` + +
+ + + ## Security considerations Only your own homeserver controls whether a gap is added to the `/messages` From 0a6cc0bf344f774dd52625f53f7660091312648e Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 2 May 2023 16:01:14 -0500 Subject: [PATCH 15/17] More robust unstable to stable rollout See https://github.com/matrix-org/matrix-spec-proposals/pull/3871#discussion_r1139456654 --- proposals/3871-gappy-timelines.md | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/proposals/3871-gappy-timelines.md b/proposals/3871-gappy-timelines.md index 226eaa43..0e0a5995 100644 --- a/proposals/3871-gappy-timelines.md +++ b/proposals/3871-gappy-timelines.md @@ -401,11 +401,29 @@ control whether you to fetch something. ## Unstable prefix -Servers will indicate support for this MSC 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, the `gaps` field can be used as `org.matrix.msc3871.gaps` +### While the MSC is unstable + +During this period, to detect server support clients should check for the +presence of the `org.matrix.msc3871` flag in `unstable_features` on `/versions`. +Clients are also required to use the unstable prefixes (see [unstable +prefix](#unstable-prefix)) during this time. + +### Once the MSC is merged but not in a spec version + +Once this MSC is merged, but is not yet part of the spec, clients should rely on +the presence of the `org.matrix.msc3871.stable` flag in `unstable_features` to +determine server support. If the flag is present, clients are required to use +stable prefixes (see [unstable prefix](#unstable-prefix)). + +### Once the MSC is in a spec version +Once this MSC becomes a part of a spec version, clients should rely on the +presence of the spec version, that supports the MSC, in `versions` on +`/versions`, to determine support. Servers are encouraged to keep the +`org.matrix.msc3871.stable` flag around for a reasonable amount of time +to help smooth over the transition for clients. "Reasonable" is intentionally +left as an implementation detail, however the MSC process currently recommends +*at most* 2 months from the date of spec release. From 0e1f021ae6306acd44ae1f259f96ae20efa4969d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 2 May 2023 16:04:18 -0500 Subject: [PATCH 16/17] Future consideration to add the gap treatment to /context See https://github.com/matrix-org/matrix-spec-proposals/pull/3871#discussion_r1182407978 --- proposals/3871-gappy-timelines.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/proposals/3871-gappy-timelines.md b/proposals/3871-gappy-timelines.md index 0e0a5995..2ca07245 100644 --- a/proposals/3871-gappy-timelines.md +++ b/proposals/3871-gappy-timelines.md @@ -391,6 +391,15 @@ chronoligcal order since we're paginating forwards): +## Future considerations + +In the future, we should consider adding the same `gaps` field to `/context` because +it's another endpoint that returns a linearized version of the DAG. + +It could make sense to roll this into this MSC but it might make the proposal less clear +if we have to bulk it up by specifying the same details for `/context`. Leaving it to be +follow-up MSC for now. + ## Security considerations From 02194b01aaf04381566c0166dff53ae9fd7fa34d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 2 May 2023 16:15:33 -0500 Subject: [PATCH 17/17] Not required --- proposals/3871-gappy-timelines.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/3871-gappy-timelines.md b/proposals/3871-gappy-timelines.md index 2ca07245..cde60e51 100644 --- a/proposals/3871-gappy-timelines.md +++ b/proposals/3871-gappy-timelines.md @@ -51,8 +51,8 @@ Name | Type | Description | required key | type | value | description | required --- | --- | --- | --- | --- `event_id` | string | Event ID | The event ID indicating the position in the `/messages` `chunk` response | yes -`prev_pagination_token` | string | Pagination token | A pagination token that represents the spot in the DAG before the given `event_id` in the `chunk` | yes -`next_pagination_token` | string | Pagination token | A pagination token that represents the spot in the DAG after the given `event_id` in the `chunk` | yes +`prev_pagination_token` | string | Pagination token | A pagination token that represents the spot in the DAG before the given `event_id` in the `chunk`. Omitting this field just means there is no gap on this side. | no +`next_pagination_token` | string | Pagination token | A pagination token that represents the spot in the DAG after the given `event_id` in the `chunk`. Omitting this field just means there is no gap on this side. | no ### `/messages` response examples