Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

url_preview media cache is not expired when background database migrations are running #13637

Closed
richvdh opened this issue Aug 26, 2022 · 2 comments · Fixed by #13657
Closed
Assignees
Labels
A-Disk-Space things which fill up the disk A-Media-Repository Uploading, downloading images and video, thumbnailing O-Occasional Affects or can be seen by some users regularly or most users rarely S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution

Comments

@richvdh
Copy link
Member

richvdh commented Aug 26, 2022

Normally, any media downloaded as part of a url preview is deleted after a day. However, while a background database migration is running, the process is stopped. If there is a long-running database migration, this can therefore cause the media store to fill up with old preview files.

This logic was added in #2697 to make sure that we didn't try to run the expiry without an index on local_media_repository.created_ts; the original logic that needs that index was added in #2478 (in get_url_cache_media_before, as amended by 93247a4), and is still present.

It looks like too broad a constraint: instead, we should use has_completed_background_update to check if the specific background update we care about has completed. However, given you'd have to be upgrading from before Synapse v1.0.0 (which created the v54 schema snapshot) to not have this index in place... we should probably just drop the check altogether.

@richvdh richvdh added A-Media-Repository Uploading, downloading images and video, thumbnailing A-Disk-Space things which fill up the disk S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. O-Occasional Affects or can be seen by some users regularly or most users rarely Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution labels Aug 26, 2022
@richvdh
Copy link
Member Author

richvdh commented Aug 26, 2022

TL;DR: let's just remove this check.

@clokep
Copy link
Member

clokep commented Aug 26, 2022

any media downloaded as part of a url preview is deleted after a day

Just to avoid confusion -- they're deleted after two days. I don't think this changes any of the rest of the issue though. Thanks for researching!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Disk-Space things which fill up the disk A-Media-Repository Uploading, downloading images and video, thumbnailing O-Occasional Affects or can be seen by some users regularly or most users rarely S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants