Skip to content

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

Merged

Conversation

haydnli-shopify
Copy link
Contributor

@haydnli-shopify haydnli-shopify commented Jun 24, 2025

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

  • Add auto_cleanup_duration to VolumeConfiguration
  • Track last usage with last_job_processed_at in VolumeModel
  • Background task (process_idle_volumes) runs every 60s to delete idle volumes
  • Update job processing to refresh volume usage timestamps
  • Database migration for new field
  • Unit and integration tests for all scenarios
  • Validation to prevent external volumes from using auto-cleanup (not supported)

Usage

  • Set auto_cleanup_duration in volume config (YAML or API)
  • Volumes not used for the specified time are auto-deleted
  • Volumes still attached or with auto_cleanup_duration: off are never deleted
  • External volumes (with volume_id) cannot use auto-cleanup

Testing

  • All core logic and edge cases are covered by tests
  • Validation tests ensure external volumes are properly handled

@haydnli-shopify haydnli-shopify deleted the feat/volume-idle-duration-cleanup branch June 24, 2025 18:57
@haydnli-shopify haydnli-shopify restored the feat/volume-idle-duration-cleanup branch June 24, 2025 19:01
@haydnli-shopify haydnli-shopify marked this pull request as ready for review June 26, 2025 14:20
@haydnli-shopify haydnli-shopify marked this pull request as draft June 26, 2025 18:36
@haydnli-shopify
Copy link
Contributor Author

haydnli-shopify commented Jun 26, 2025

For this PR,
I've tested it locally and it's working correctly. I created test volumes with different auto-cleanup settings (30s, disabled, integer values) and verified the configuration parsing works. I tested the full lifecycle - volumes get created, go idle, and are automatically deleted after the specified duration. The background task handles all the edge cases we discussed.

If needed, I can upload my test configurations to show the testing approach.

@haydnli-shopify haydnli-shopify marked this pull request as ready for review June 30, 2025 14:13
@haydnli-shopify haydnli-shopify requested a review from r4victor July 4, 2025 16:30
@haydnli-shopify haydnli-shopify force-pushed the feat/volume-idle-duration-cleanup branch from 7afb48b to df17d17 Compare July 7, 2025 14:25
Copy link
Collaborator

@r4victor r4victor left a 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.

@r4victor
Copy link
Collaborator

r4victor commented Jul 9, 2025

The issue and the PR description say to support idle_duration for volumes, but the PR actually supports auto_cleanup_duration. Which one have you intended to support?

Copy link
Collaborator

@r4victor r4victor left a 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.

@haydnli-shopify haydnli-shopify force-pushed the feat/volume-idle-duration-cleanup branch from 815c6c2 to 002be4d Compare July 9, 2025 18:09
@haydnli-shopify
Copy link
Contributor Author

@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.

@r4victor
Copy link
Collaborator

@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.

@r4victor
Copy link
Collaborator

Made some adjustments, merging

@r4victor r4victor merged commit 0c60b1e into dstackai:master Jul 15, 2025
26 checks passed
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.

3 participants