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

Remote media purge leaves a lot of directories #16229

Closed
namazso opened this issue Sep 3, 2023 · 2 comments
Closed

Remote media purge leaves a lot of directories #16229

namazso opened this issue Sep 3, 2023 · 2 comments
Labels
A-Media-Repository Uploading, downloading images and video, thumbnailing O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@namazso
Copy link

namazso commented Sep 3, 2023

Description

I recently noticed my remote media folder grew to 5 GB again, so I ran a media purge for all files (timestamp in future). However, after finishing the remote media folder still weighed 500 MB. It turns out that was the combined size of the empty folders left behind.

Steps to reproduce

  • run homeserver for a while
  • run media purge
  • observe leftover empty folders

Homeserver

namazso.eu

Synapse Version

1.91.0

Installation Method

Docker (matrixdotorg/synapse)

Database

PostgreSQL 15

Workers

Single process

Platform

Linux 6.5.0

Configuration

No response

Relevant log output

-

Anything else that would be useful to know?

No response

@DMRobertson
Copy link
Contributor

Relevant source:

async def delete_old_remote_media(self, before_ts: int) -> Dict[str, int]:
old_media = await self.store.get_remote_media_ids(
before_ts, include_quarantined_media=False
)
deleted = 0
for media in old_media:
origin = media["media_origin"]
media_id = media["media_id"]
file_id = media["filesystem_id"]
key = (origin, media_id)
logger.info("Deleting: %r", key)
# TODO: Should we delete from the backup store
async with self.remote_media_linearizer.queue(key):
full_path = self.filepaths.remote_media_filepath(origin, file_id)
try:
os.remove(full_path)
except OSError as e:
logger.warning("Failed to remove file: %r", full_path)
if e.errno == errno.ENOENT:
pass
else:
continue
thumbnail_dir = self.filepaths.remote_media_thumbnail_dir(
origin, file_id
)
shutil.rmtree(thumbnail_dir, ignore_errors=True)
await self.store.delete_remote_media(origin, media_id)
deleted += 1
return {"deleted": deleted}

I suppose when deleting a file on disk we could try to os.rmdir() its parent directories, catching and ignoring OSError due to the directory not being empty?

Would probably also want to do something similar in delete_old_local_media.

As a one-time cleanup job I guess one could find -empty -type d -delete?

@DMRobertson DMRobertson added A-Media-Repository Uploading, downloading images and video, thumbnailing S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Sep 4, 2023
@clokep
Copy link
Member

clokep commented Sep 5, 2023

Duplicate of #7690.

@clokep clokep closed this as completed Sep 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Media-Repository Uploading, downloading images and video, thumbnailing O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

3 participants