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

Add cron service and cleanup script #167

Merged
merged 24 commits into from
Feb 8, 2024
Merged

Add cron service and cleanup script #167

merged 24 commits into from
Feb 8, 2024

Conversation

nrobinaubertin
Copy link
Contributor

@nrobinaubertin nrobinaubertin commented Jan 30, 2024

Overview

Synapse can leave empty directories in the media directory after deleting old/unused files. There is currently no option to remove them and they can accumulate, using inodes and space.

This PR adds a cron service inside the workload container to regularly launch a script meant to delete those empty directories.
The PR was structure to simplify the future addition of scripts that should be handled by the cron service. Just add them in the corresponding cron.{hourly,daily,weekly,monthly} directory.

Remarks:

  • I tried the cleanup script on a VPS with half a million empty directories: CPU and memory usage were not noticeable and I tweaked the script to lower the IOPS (see the comments inside it for explanations)

Checklist

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

license-eye has totally checked 115 files.

Valid Invalid Ignored Fixed
51 2 62 0
Click to see the invalid file list
  • synapse_rock/cron/cron.weekly/remote_content_empty_directory_cleanup.py
  • synapse_rock/scripts/run_cron.py

@nrobinaubertin nrobinaubertin marked this pull request as ready for review February 1, 2024 21:58
@nrobinaubertin nrobinaubertin requested a review from a team as a code owner February 1, 2024 21:58
Copy link
Collaborator

@javierdelapuente javierdelapuente left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@merkata merkata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

Copy link
Contributor

github-actions bot commented Feb 8, 2024

Test coverage for 5b7a9e8

Name                            Stmts   Miss Branch BrPart  Cover   Missing
---------------------------------------------------------------------------
src/actions/__init__.py             3      0      0      0   100%
src/actions/register_user.py       22      0      2      0   100%
src/actions/reset_instance.py      21      0      2      0   100%
src/backup.py                     159      5     30      2    96%   322-323, 366-367, 423->425, 426
src/backup_observer.py             80      8      8      0    91%   126-129, 134-137
src/charm.py                      222      8     50      4    96%   165-166, 227-228, 247-248, 289, 303
src/charm_state.py                 63      1     10      1    97%   127
src/charm_types.py                 23      0      6      0   100%
src/database_client.py             53      1     10      3    94%   35, 47->exit, 69->exit
src/database_observer.py           48      4      4      0    92%   69-71, 87
src/exceptions.py                   4      1      0      0    75%   22
src/mjolnir.py                     76      6     20      1    91%   60-64, 73
src/observability.py                9      0      0      0   100%
src/pebble.py                     102     12     16      6    85%   95-96, 98-99, 107, 109, 111, 115, 136-137, 152-153
src/saml_observer.py               45      1      8      0    98%   64
src/smtp_observer.py               70      3     14      1    95%   70-74, 96->101
src/synapse/__init__.py             3      0      0      0   100%
src/synapse/api.py                161      2     22      2    98%   207, 376
src/synapse/workload.py           275      8     42      6    96%   421->exit, 450-451, 503-504, 540-541, 557, 606->609, 655, 663->665, 665->667
src/user.py                        24      0      4      0   100%
---------------------------------------------------------------------------
TOTAL                            1463     60    248     26    95%

Static code analysis report

Run started:2024-02-08 08:54:51.323672

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 7895
  Total lines skipped (#nosec): 4
  Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
  Total issues (by severity):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
  Total issues (by confidence):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
Files skipped (0):

@nrobinaubertin nrobinaubertin merged commit dbe44c4 into main Feb 8, 2024
21 checks passed
@nrobinaubertin nrobinaubertin deleted the cron-cleanup branch February 8, 2024 14:09
amandahla added a commit that referenced this pull request Feb 8, 2024
* Add cron service and cleanup script

* Fix rockfile

* Fix restart_synapse()

* Remove unused fn

* Fix restart_synapse()

* Fix linting

* Fix rockfile

* Throttle cleanup operations

* Add missing headers

* Add some licensing exceptions

* Try to fix headers ignore once again

* Make scripts added to the rock executable

* Rerun CI

* Only modify rockcraft.yml where necessary

* Fix rockcraft scripts permissions

* Fix typo

---------

Co-authored-by: Amanda H. L. de Andrade Katz <amanda.katz@canonical.com>
Co-authored-by: arturo-seijas <102022572+arturo-seijas@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants