MSC4074: Server side annotation aggregation#4074
Conversation
turt2live
changed the title
|
@dihydrogenmonoxide16 when you get a chance, please sign off on the changes for this MSC to be eligible for FCP acceptance. Also, lines should be limited to approximately 120 characters. |
turt2live
added
proposal
addressed |
|
@turt2live , I added changes to make the behavior backwards-compatible with older clients as we discussed before |
turt2live
left a comment
turt2live
left a comment
There was a problem hiding this comment.
overall this seems like a great concept. I'm not sure on the client synchronization stuff myself, but others should be. A listed client implementation would help quite a bit in this understanding, I suspect.
| Servers tried to provide clients with server side generated | ||
| annotation aggregates before, but these attempts were not successful | ||
| mainly because both sides (server and client) tried to | ||
| aggregate and the same time. That used to lead to the situation when |
| aggregated by the server. The only exception to this rule is E2E | ||
| encrypted events, which should be solely aggregated by the client. |
There was a problem hiding this comment.
Does the encryption condition only apply once reactions are encrypted?
There was a problem hiding this comment.
rephrased to make it clearer
| For server-server API, homeserver should provide aggregates, | ||
| including only local events (events created and signed by self). |
There was a problem hiding this comment.
I'm not following this bit. Servers distribute the events as-is without consideration for the underlying feature, meaning each server should be able to determine how many of each event there is. Reactions can also be used as a form of "DAG healing" when rooms get complex. This is because they are normally higher traffic than regular messages, giving more opportunity for the DAG to heal extremities. The delay associated with needing to process these complicated event trees is also (typically) less critical than message traffic.
There was a problem hiding this comment.
extended the comment to clarify the idea: severs do not filter aggregated relations (no issue with the problem you described), but attach aggregates with counters to parent events they deliver (e.g servers A and B federate room R. server A users liked event E 100 times. server B users liked event E 20 times. when server A delivers E event PDU, it comes with counter 100, server B knows that there are additional 20 'local' likes and can deduct/merge 100+20 = 120.);
If this point seems arguable, we can exclude the paragraph completely
| Servers are allowed to limit maximum number of different "m.annotation" | ||
| keys they aggregate to a single event by a reasonable value (that | ||
| corresponds to a limited number of different reaction kinds, e.g. "👍", | ||
| "👎"). This is needed to avoid the situation when malicious users may | ||
| attack server creating jumbo-events (events with arbitrary high count of | ||
| different reaction kinds). Servers may configure this threshold on their | ||
| discretion, but this number should not be lower than 16. When annotation | ||
| key limit is reached, no new keys should be added to the aggregate, but | ||
| older keys would continue to be aggregated (change their count). | ||
| Annotations beyond that threshold should still be available via | ||
| [`relations`](https://spec.matrix.org/v1.8/client-server-api/#relationships-api) endpoints. |
There was a problem hiding this comment.
How would a server communicate that it hit the limit?
There was a problem hiding this comment.
I completely removed this clause. Potentially this problem already exists now, but only on the client side. I think it would better get addressed separately in order not to overcomplicate this change proposal (already big enough and technically complex)
| No new identifiers are proposed; it is proposed that servers implementing | ||
| this | ||
| proposal simply do so on the existing endpoints. |
There was a problem hiding this comment.
filter_server_aggregated_relation_types and the m.reaction EDU appear new, as do a few other identifiers. They will need to be namespaced.
There was a problem hiding this comment.
prefixed with msc4074., also renamed filter_server_aggregated_relation_types to msc4072.not_aggregated_relations to better match other filter params naming
There was a problem hiding this comment.
Implementation requirements:
- Server supporting filter opt-in behaviour
- Client using the filter behaviour
There was a problem hiding this comment.
What is the expectation here?
- to add this comment to the MXC inself
- some pseudocode
- working client/server implementation somewhere
- arch doc
?
Or is it just a ref comment for other contributors?
There was a problem hiding this comment.
The third option, a working client/server implementation somewhere. MSCs must be proven with implementations before they can be accepted: https://spec.matrix.org/proposals/#implementing-a-proposal
It generally doesn't matter who implements it or where, so implementation requirements are commented as a reference for everyone
@turt2live , I'm doing a follow-up work to answer your comments and extend/adjust the proposal. Will post updates in ~1 week due to high work load. |
|
No worries! Take the time you need :) |
…ot to complicate client gap resolution logic
@turt2live, thank you for your feedback and collaboration! I answered your comments and addressed the points you mentioned. Added few improvements that turned out to be necessary for a few corner cases (gap resolution, older event updates). Asked few questions there also. |
3 participants
Proposal describes the way how clients and servers could interact in a more efficient and scalable way working with annotation (reaction) - heavy rooms.
rendered
Signed-off-by: Vladimir Aseev vladimir.aseev@reddit.com