element-hq / dendrite

Dendrite is a second-generation Matrix homeserver written in Go!
https://element-hq.github.io/dendrite/
GNU Affero General Public License v3.0
37 stars 6 forks source link

Expire ephemeral events #1319

Open matrixbot opened 3 weeks ago

matrixbot commented 3 weeks ago

This issue was originally created by @kegsay at https://github.com/matrix-org/dendrite/issues/1319.

Sytests:

    × Ephemeral messages received from servers are correctly expired
    × Ephemeral messages received from clients are correctly expired
matrixbot commented 3 weeks ago

This comment was originally posted by @DAlperin at https://github.com/matrix-org/dendrite/issues/1319#issuecomment-892085455.

I can try this. I have one architectural questions about the implementation: Dendrite currently has no system of scheduled workers that I know of (which is how synapse implements this). I can build a naive database backed worker implementation but let me know if anyone has some ideas of how to best do this.

matrixbot commented 3 weeks ago

This comment was originally posted by @kegsay at https://github.com/matrix-org/dendrite/issues/1319#issuecomment-892472178.

Does this need a database backend at all though? Ephemeral are just that: short lived. We just want to sometimes drop them instead of always passing them through to the client I think?

matrixbot commented 3 weeks ago

This comment was originally posted by @DAlperin at https://github.com/matrix-org/dendrite/issues/1319#issuecomment-892725680.

As I understand it from reading synapse, ephemeral events are just events that have a set time by which they will be redacted. Without backing it with a database we would have to check every time we send events to the client whether any of them have expired and then redact them which feels expensive. Besides we also need to inform clients that the event has expired (which to the client just means a normal redaction)

matrixbot commented 3 weeks ago

This comment was originally posted by @DAlperin at https://github.com/matrix-org/dendrite/issues/1319#issuecomment-894876511.

Ah got it, this is a bit more involved than I expected. Here this the relevant section from the doc:

When a client sends a message with m.self_destruct information, the servers participating in a room should start monitoring the room for read receipts for the event in question.

Once a given server has received a read receipt for this message from a member in the room (other than the sender), then the message's self-destruct timer should be started for that user. Once the timer is complete, the server should redact the event from that member's perspective, and send the user a synthetic m.redaction event in the room to the reader's clients on behalf of the sender.

The synthetic redaction event should contain an m.synthetic: true flag on the reaction's content to show the client that it is synthetic and used for implementing self-destruction rather than actually sent from the claimed client.

For m.self_destruct_after, the server should redact the event and send a synthetic redaction once the server's localtime overtakes the timestamp given by m.self_destruct_after. The server should only perform the redaction once.

I think implementing just m.self_destruct_after like synapse does might be a good starting place: https://github.com/matrix-org/synapse/commit/54dd5dc12b0ac5c48303144c4a73ce3822209488. I am gonna try that this week.

matrixbot commented 3 weeks ago

This comment was originally posted by @kegsay at https://github.com/matrix-org/dendrite/issues/1319#issuecomment-895909548.

Pleas gate any support for this behind a config flag for MSC2228.

matrixbot commented 3 weeks ago

This comment was originally posted by @DAlperin at https://github.com/matrix-org/dendrite/issues/1319#issuecomment-897993171.

@kegsay Architecturally, do you think it's better to add an expires_at field to the roomserver_events table or is it better to create an entirely new table to keep track of events that we need to expire? I'm personally leaning towards the first option but I understand the urge to leave a core table like roomserver_events alone.

matrixbot commented 3 weeks ago

This comment was originally posted by @kegsay at https://github.com/matrix-org/dendrite/issues/1319#issuecomment-898314324.

Add an additional table please. Don't pollute the core tables with MSC extensions. The vast majority of events will not have an expires_at field anyway, so it doesn't really make sense to do that when you can just have event_id | expires_at in a different table. This also lets you add additional columns for performance optimisations very easily.

Bear in mind the following when implementing this:

matrixbot commented 3 weeks ago

This comment was originally posted by @kegsay at https://github.com/matrix-org/dendrite/issues/1319#issuecomment-1134765270.

Still failing as of today.