Skip to content

rq: add a garbage collector to the worker #1370

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

efahl
Copy link
Contributor

@efahl efahl commented Apr 20, 2025

Implement a maintenance hook on the standard redis queue worker to do garbage collection on expired builds. When a result expires from the queue, its data will be removed from the public/store/ directory at the regular maintenance interval (default is every 600 seconds).

@efahl
Copy link
Contributor Author

efahl commented Apr 20, 2025

I've been running this for about 9 months now, no issues on my local server.

I test it out by setting the ttl values way down in api.py and watching the logs on the server after I queue up a bunch of builds.

+    failure_ttl: str = "3m"
+    result_ttl: str = "15m"

@efahl
Copy link
Contributor Author

efahl commented Apr 20, 2025

But, are we running out of space due to old podman/docker containers hanging around, too?

Maybe an hourly cron job that does this?

podman container prune --force && podman image prune --force

Copy link

codecov bot commented Apr 20, 2025

Codecov Report

Attention: Patch coverage is 43.58974% with 22 lines in your changes missing coverage. Please review.

Project coverage is 88.79%. Comparing base (5e65dec) to head (49ba426).
Report is 241 commits behind head on main.

Files with missing lines Patch % Lines
asu/rq.py 42.10% 22 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1370      +/-   ##
==========================================
+ Coverage   80.75%   88.79%   +8.03%     
==========================================
  Files          15       15              
  Lines         977     1258     +281     
==========================================
+ Hits          789     1117     +328     
+ Misses        188      141      -47     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@@ -29,7 +29,7 @@ jobs:
- name: Set __version__ and poetry version
run: |
TAG="$(git describe --tags --always | awk -F"-" '{if (NF>1) {print substr($1, 2)".post"$2} else {print substr($1, 2)}}')"
echo "__version__ = \"$TAG\"" > asu/__init__.py
sed "s/__version__.*/__version__ = \"$TAG\"/" -i asu/__init__.py
Copy link
Member

@aparcar aparcar Apr 20, 2025

Choose a reason for hiding this comment

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

this change seems unrelated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's so the other contents of the init file are not deleted.

@aparcar aparcar requested a review from Copilot April 20, 2025 21:50
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a garbage collector to the redis queue worker by implementing a custom GCWorker, which periodically deletes expired builds from the public store directory.

  • Updated the podman-compose.yml to launch the worker with garbage collection enabled
  • Added a GCWorker class in asu/rq.py to perform periodic clean-ups
  • Updated version handling and registration in asu/init.py and the GitHub publish workflow

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
podman-compose.yml Added scheduler and worker-class arguments to enable GC functionality
asu/rq.py Introduced GCWorker with a clean_store method for deleting expired builds
asu/init.py Registered GCWorker, ensuring it is exported
.github/workflows/publish.yml Updated version assignment mechanism using sed for improved consistency

asu/rq.py Outdated
Comment on lines 33 to 36
insignficant, likely due to `stat` calls being the bottleneck.
(Just for comparison, tests against store mounted on a fast SSD
Copy link
Preview

Copilot AI Apr 20, 2025

Choose a reason for hiding this comment

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

The comment contains a spelling error ('insignficant'). It should be corrected to 'insignificant'.

Suggested change
insignficant, likely due to `stat` calls being the bottleneck.
(Just for comparison, tests against store mounted on a fast SSD
insignificant, likely due to `stat` calls being the bottleneck.

Copilot uses AI. Check for mistakes.

@efahl
Copy link
Contributor Author

efahl commented Apr 20, 2025

Just been playing with the podman API, this might be added to that worker, too. It appears that pruning only affects unused containers/images, so you can safely just call these and reclaim the orphaned space.

$ python -c 'from asu.util import get_podman; print(get_podman().containers.prune())'
{'ContainersDeleted': [], 'SpaceReclaimed': 0}

$ python -c 'from asu.util import get_podman; print(get_podman().images.prune())'
{'ImagesDeleted': [{'Deleted': '4a2e6bf10ec5d4c70ff5b8598193285b6d090d47ad737bfe322c0be298e1e6d1', 'Untagged': ''}], 'SpaceReclaimed': 927715421}

@aparcar
Copy link
Member

aparcar commented Apr 20, 2025

Right now this is my crontab, however we could also use the GC worker:

@daily cd /srv && find ./store  -ctime +1 -exec rm -rf {} \;
@daily podman volume prune -af

Implement a maintenance hook on the standard redis queue worker to do garbage
collection on expired builds.  When a result expires from the queue,
its data will be removed from the public/store/ directory at the regular
maintenance interval (default is every 600 seconds).

Signed-off-by: Eric Fahlgren <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants