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

Retention deleted messages keep displaying #28799

Closed
xxfogs opened this issue Dec 22, 2024 · 8 comments
Closed

Retention deleted messages keep displaying #28799

xxfogs opened this issue Dec 22, 2024 · 8 comments
Labels
A-Storage Storage layer of the app, including IndexedDB, local storage, etc. O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect

Comments

@xxfogs
Copy link

xxfogs commented Dec 22, 2024

Steps to reproduce

Prerequisites

Retention config:

# retention, data control
retention:
    enabled: true
    default_policy:
        min_lifetime: 14d
        max_lifetime: 90d
    allowed_lifetime_min: 1d
    allowed_lifetime_max: 90d

media_retention:
    local_media_lifetime: 90d

delete_stale_devices_after: 30d

Steps

  1. Open Element desktop app
    • Where the homeserver has message retention enabled
  2. Go to a channel with messages older than the retention period
  3. Notice how some messages remain even beyond (Oldest message 105 days ago) the retention period (90 + 1 days)

image

  1. Open Element Android
  2. Navigate to the same channel
  3. Notice how all the messages beyond the retention period are gone (Oldest message 90 days ago)

image

Outcome

What did you expect?

I expected for all messages beyond the message retention period to be gone as it is on Element Android

What happened instead?

Some messages remain even beyond the retention period.
I presume this has something to do with the message cache (Security & Privacy > Encryption > Message search), however after resetting it, the messages still remained.

Operating system

Debian GNU/Linux 12

Application version

Element version: 1.11.89 Crypto version: Rust SDK 0.7.2 (2f872cf), Vodozemac 0.8.1

How did you install the app?

https://element.io/download APT repository

Homeserver

Synapse 1.121.1

Will you send logs?

No

@dosubot dosubot bot added A-Storage Storage layer of the app, including IndexedDB, local storage, etc. O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist labels Dec 22, 2024
@t3chguy
Copy link
Member

t3chguy commented Dec 22, 2024

The server does not notify the client when a message is removed due to retention, nor the retention period in place on that specific server, the server could/should send redactions to inform the client when events are removed otherwise the client has no way of knowing. I suggest raising an issue on the server or spec. Element Android likely just doesn't cache things, I know Element X doesn't cache things at all.

@t3chguy t3chguy closed this as not planned Won't fix, can't repro, duplicate, stale Dec 22, 2024
@xxfogs
Copy link
Author

xxfogs commented Dec 23, 2024

Can't you just check if the message can be found on the server, and if not, get rid of it on the client?

@t3chguy
Copy link
Member

t3chguy commented Dec 23, 2024

So your suggestion is for every single event you'd have to call the API to query if it is still there? That'd add minutes to the launch time if you're in a lot of rooms or on a slow server...

@xxfogs
Copy link
Author

xxfogs commented Dec 23, 2024

I should elaborate on my thought. What I meant above is to check whether the message is still there when the message gets loaded (for instance, upon opening a channel and scrolling upward). But now that I reconsider, it wouldn't solve the underlying issue of deleted messages still remaining on the device.

I have two more ideas:

  1. If it's possible to determine what is the retention period for a channel (or the default server period), all the locally cached messages exceeding it could be removed. That shouldn't be resource intensive at all.
  2. Don't messages have max_lifetime parameter? If the lifetime is exceeded, the message should be purged from the local cache. This option would require saving that parameter in the local cache as well (although I am not sure if I correctly understand what exactly is the said parameter)

@t3chguy
Copy link
Member

t3chguy commented Dec 23, 2024

The cache held on the client is small, roughly ~20 messages per room, scrolling up fetches from the server so should already be respect retention.

If it's possible to determine what is the retention period for a channel (or the default server period), all the locally cached messages exceeding it could be removed. That shouldn't be resource intensive at all.

For a channel it is, for the server it isn't.

Don't messages have max_lifetime parameter? If the lifetime is exceeded, the message should be purged from the local cache. This option would require saving that parameter in the local cache as well (although I am not sure if I correctly understand what exactly is the said parameter)

Not one that is sent to the client according to the spec: https://spec.matrix.org/v1.13/client-server-api/

@xxfogs
Copy link
Author

xxfogs commented Dec 23, 2024

The cache held on the client is small, roughly ~20 messages per room, scrolling up fetches from the server so should already be respect retention.

Makes sense now. Would it really be resource intensive then to check upon launch whether those ~20 exist?

For a channel it is, for the server it isn't.

I see

Not one that is sent to the client according to the spec: https://spec.matrix.org/v1.13/client-server-api/

I see

@t3chguy
Copy link
Member

t3chguy commented Dec 23, 2024

Makes sense now. Would it really be resource intensive then to check upon launch whether those ~20 exist?

~20 * hundreds of rooms * thousands of clients and you end up with a very unhappy server due to the thundering herd effect https://en.wikipedia.org/wiki/Thundering_herd_problem every morning

@xxfogs
Copy link
Author

xxfogs commented Dec 23, 2024

Fair enough

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Storage Storage layer of the app, including IndexedDB, local storage, etc. O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect
Projects
None yet
Development

No branches or pull requests

2 participants