MSC2675: Serverside aggregations of message relationships

[uhoreg]

Rendered

Replaces #1849 along with #2674, #2676, and #2677

FCP proposal

---

Synapse implementation:

• /relations endpoint
• Bundled aggregations

[uhoreg]

Transfer comments from 1849

[turt2live]

@revidee says:

So regarding Handling limited (gappy) syncs: I couldn't find any MSC Proposal handling this, is there any?

IMHO, this is the most important part, since for example:
If we received and event via sync, which may has annotations bundled into it and keep it up-to-date via local aggregation using all future m.relates_to events, but then go offline for a day and come back, without syncing all events in-between, we end up in an incorrect state for the local aggregations of the already received event.

If we would simply use a locally cached copy of the locally aggregated event, it would not match a refreshed version from the server. Thus, in this situation, we can't really trust any local copies of events, except we query all relational events, that happened in-between. This would make "jumping back" to the locally saved event somewhat incorrect (mismatches a fresh copy from the server). And refreshing all locally saved events would diminish any possible traffic savings gained by local caching.

Only case I see in which we could realistically save traffic is an initial sync, with filters enabled to not fetch all m.annotation / ... / events, but include then in consecutive /sync calls. Other than that, this makes locally caching/saving events, in order to not re-query them, a step harder, since we have to be really careful to keep all the local events in sync & up-to-date.

This problem is (imho) rather drastic in bigger rooms with large amounts of daily traffic. When loading older messages we may end up loading batches upon batches of events filled with relational events only until we load a real m.text message that the client could display. (e.g. if many reactions have happened) But we cannot exclude them, if we choose to locally aggregate already received events, since we might miss some otherwise.

In that light, it almost seems more logical and traffic-saving to discard local copies all together in order to take advantage of aggregations and bundling. In that case, we can always apply a filter for the events received in /sync and can be sure that the data received is up-to-date. (after loading/paginating through the /aggregations endpoint if there is a next_batch present)

Am I missing something obvious here, or am I correct that making local copies in combination with this MSC seems like a pain?

[clokep]

So regarding Handling limited (gappy) syncs: I couldn't find any MSC Proposal handling this, is there any?

Correct, there is currently not MSC which proposes this behavior.

IMHO, this is the most important part, since for example:
If we received and event via sync, which may has annotations bundled into it and keep it up-to-date via local aggregation using all future m.relates_to events, but then go offline for a day and come back, without syncing all events in-between, we end up in an incorrect state for the local aggregations of the already received event.

Correct, the local client might not know some of the related messages without filling the entire gap. (Note that I don't think this is any different than if only local aggregation was done -- so I'm unsure if this MSC really changes that behavior.)

If we would simply use a locally cached copy of the locally aggregated event, it would not match a refreshed version from the server. Thus, in this situation, we can't really trust any local copies of events, except we query all relational events, that happened in-between. This would make "jumping back" to the locally saved event somewhat incorrect (mismatches a fresh copy from the server). And refreshing all locally saved events would diminish any possible traffic savings gained by local caching.

I think this is pretty much what the gappy syncs issue in the MSC describes. I'm unsure what current clients do -- @gsouquet do you know if Element Web does anything special in this case?

Only case I see in which we could realistically save traffic is an initial sync, with filters enabled to not fetch all m.annotation / ... / events, but include then in consecutive /sync calls. Other than that, this makes locally caching/saving events, in order to not re-query them, a step harder, since we have to be really careful to keep all the local events in sync & up-to-date.

This might work, but I doubt the savings in traffic would be the tremendous, unless the majority of your events are annotations, specifically. (I think you would still want to fetch references / edits / threads since the aggregation of those somewhat assumes you're going to fetch the full event anyway).

In that light, it almost seems more logical and traffic-saving to discard local copies all together in order to take advantage of aggregations and bundling. In that case, we can always apply a filter for the events received in /sync and can be sure that the data received is up-to-date. (after loading/paginating through the /aggregations endpoint if there is a next_batch present)

Note that the /aggregations endpoint was removed from this MSC (and from Synapse). MSC3571 includes the bits split out of MSC2675.

Am I missing something obvious here, or am I correct that making local copies in combination with this MSC seems like a pain?

I don't think you're missing anything obvious, but maybe @gsouquet has some ideas of how the clients deal with this.

[germain-gg]

Thank you for raising this @revidee

Just like @clokep said, I believe you're pretty much on point and your assessment of the situation is pretty correct.

Besides threads, Element Web does not use bundled aggregation.

If we take the example of reactions, the bundled aggregations does not give you enough information to generate the user interface that Element wants to provide. To be able to do that you will need a complete list of the annotations, which is not practical and comes with quite a big network overhead

The way that threads went around this problem is by providing a set of information to render the initial event tile without having to fetch all events that belong to a thread (we're currently giving the number of replies to a thread, whether the logged in user has participated to it, and the last event of that thread). That means that client only have to fetch the root event of that thread to refresh the bundled relationship on app load.
This is more practical but comes at the cost of slightly coupling your server implementation with your UI

[turt2live]

@alphapapa says:

@ara4n @turt2live I guess it's pointless to comment here since the proposal has been merged, but I feel like chiming in, as a client developer:

I've been reluctant to implement anything not yet in a released version of the spec, but in my client I did implement support for reaction annotations, since they're so widely used in Element and other clients already. But now I'm seeing my client not render annotations in some cases, which I've found is because the server is now aggregating some of them.

I understand the motivations for this proposal, but at the same time, it increases the complexity of developing clients: rather than simply handling each reaction annotation event, a client must handle both aggregated reaction annotations and individual reaction annotation events. This means that, when an individual event comes in, I have to emulate the server-side aggregation in my client, adding it to the annotated event's unsigned data myself, so that I can still have a single path for rendering an event.

Fundamentally, this means that a single logical action (reacting to a message) may now be represented in a room's timeline in two different ways: as individual events, or as aggregations of metadata on the annotated event. So this does not seem to make it easier for clients, but rather, more complicated.

I feel like Matrix would do well to emulate the Zen of Python: there should be one, and preferably only one, way to represent a logical event in a room's timeline. This would greatly simplify implementations on both clients and servers, which would do much to help ensure Matrix's adoption and longevity.

My two cents. Thanks.

[turt2live]

Spec PR: matrix-org/matrix-spec#1062

[turt2live]

Merged 🎉