-
Notifications
You must be signed in to change notification settings - Fork 185
feat: add volume idle duration cleanup feature (#2497) #2842
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
feat: add volume idle duration cleanup feature (#2497) #2842
Conversation
src/dstack/_internal/server/background/tasks/process_idle_volumes.py
Outdated
Show resolved
Hide resolved
For this PR, If needed, I can upload my test configurations to show the testing approach. |
src/dstack/_internal/server/background/tasks/process_idle_volumes.py
Outdated
Show resolved
Hide resolved
src/dstack/_internal/server/background/tasks/process_idle_volumes.py
Outdated
Show resolved
Hide resolved
src/tests/_internal/server/background/tasks/test_process_idle_volumes.py
Outdated
Show resolved
Hide resolved
7afb48b
to
df17d17
Compare
src/dstack/_internal/server/background/tasks/process_idle_volumes.py
Outdated
Show resolved
Hide resolved
src/dstack/_internal/server/background/tasks/process_idle_volumes.py
Outdated
Show resolved
Hide resolved
src/dstack/_internal/server/background/tasks/process_idle_volumes.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR is ok but it does not actually deletes idle volumes – just marks them as deleted in the db.
The issue and the PR description say to support |
src/dstack/_internal/server/background/tasks/process_idle_volumes.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created a volume:
type: volume
name: my-gcp-volume
backend: gcp
region: europe-west4
availability_zone: europe-west4-b
size: 100GB
auto_cleanup_duration: 1m
process_idle_volumes fails with exception sqlalchemy.exc.MissingGreenlet: greenlet_spawn
– should be easy to reproduce if creating any volume.
...stack/_internal/server/migrations/versions/d268739bd76b_merge_volume_cleanup_and_secrets_.py
Outdated
Show resolved
Hide resolved
815c6c2
to
002be4d
Compare
@r4victor Thanks for your response, I have fixed the issue you mentioned, and the PR description has been updated! A force push happened in this fix because I removed the merge migration as requested, updated the migration chain to properly rebase on master, and added validation to prevent external volumes from using auto_cleanup_duration at creation time. |
@haydnli-shopify, Just do a regular merge to pull changes from master to avoid force push. No need to rebase the branch on master since we'll merge with squash anyway. |
Made some adjustments, merging |
Summary
Adds automatic cleanup of unused volumes after a configurable idle duration. Users can now set
auto_cleanup_duration
for volumes; if a volume isn't used by any job for that period, it's deleted automatically.Addresses [Feature]: Delete unused volumes after idle duration #2497.
Changes
auto_cleanup_duration
toVolumeConfiguration
last_job_processed_at
inVolumeModel
process_idle_volumes
) runs every 60s to delete idle volumesUsage
auto_cleanup_duration
in volume config (YAML or API)auto_cleanup_duration: off
are never deletedvolume_id
) cannot use auto-cleanupTesting