Skip to content
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

Expire ephemeral events #1319

Open
matrixbot opened this issue Oct 31, 2024 · 8 comments
Open

Expire ephemeral events #1319

matrixbot opened this issue Oct 31, 2024 · 8 comments
Labels
are-we-synapse-yet This issue or PR involves Sytests in AWSY help wanted Extra attention is needed T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@matrixbot
Copy link
Collaborator

This issue was originally created by @kegsay at matrix-org/dendrite#1319.

Sytests:

    × Ephemeral messages received from servers are correctly expired
    × Ephemeral messages received from clients are correctly expired
@matrixbot matrixbot added are-we-synapse-yet This issue or PR involves Sytests in AWSY help wanted Extra attention is needed T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. labels Oct 31, 2024
@matrixbot
Copy link
Collaborator Author

This comment was originally posted by @DAlperin at matrix-org/dendrite#1319 (comment).

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
Copy link
Collaborator Author

This comment was originally posted by @kegsay at matrix-org/dendrite#1319 (comment).

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
Copy link
Collaborator Author

This comment was originally posted by @DAlperin at matrix-org/dendrite#1319 (comment).

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
Copy link
Collaborator Author

This comment was originally posted by @DAlperin at matrix-org/dendrite#1319 (comment).

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: matrix-org/synapse@54dd5dc. I am gonna try that this week.

@matrixbot
Copy link
Collaborator Author

This comment was originally posted by @kegsay at matrix-org/dendrite#1319 (comment).

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

@matrixbot
Copy link
Collaborator Author

This comment was originally posted by @DAlperin at matrix-org/dendrite#1319 (comment).

@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
Copy link
Collaborator Author

This comment was originally posted by @kegsay at matrix-org/dendrite#1319 (comment).

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:

  • Timers can be modified by editing the message, so you can't just say "take the lowest expiry time and do that". This will cause potentially multiple syntethic redaction events to be created as edits also need to be redacted.

    Senders may edit the m.self_destruct fields in order to retrospectively change the intended lifetime of a message. Each new m.replaces event should be considered to replace the self-destruction information (if any) on the original, and restart the destruction timer. On destruction, the original event (and all m.replaces variants of it) should be redacted.

  • You probably want to track if the event has been seen (any read receipt has been received) so you know when to start the timer.
  • The redaction is entirely local, so don't send it over federation. You'll have to emit something from the roomserver producer, but only handle it in the syncapi where it will be consumed. This may also play weirdly with scrollback (/messages) so check your synthetic redactions work there too.

@matrixbot
Copy link
Collaborator Author

This comment was originally posted by @kegsay at matrix-org/dendrite#1319 (comment).

Still failing as of today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
are-we-synapse-yet This issue or PR involves Sytests in AWSY help wanted Extra attention is needed T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Development

No branches or pull requests

1 participant