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 persistence to Ray #744

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

Conversation

javiermtorres
Copy link
Contributor

@javiermtorres javiermtorres commented Jan 27, 2025

What's changing

A Redis container has been added to the compose definition to hold the Ray metadata.

Closes #690

How to test it

Steps to test the changes:

  1. Start as usual with make local-up
  2. Run the backend integration tests with make test-backend-integration
  3. Check the jobs in the dashboards
  4. Stop the containers with ^C
  5. Restart them with make local-up
  6. Check the dashboard again. The jobs information should still be visible.

Additional notes for reviewers

N/A

I already...

  • Tested the changes in a working environment to ensure they work as expected
  • Added some tests for any new functionality
    • N/A
  • Updated the documentation (both comments in code and product documentation under /docs)
  • Checked if a (backend) DB migration step was required and included it if required
    • No DB changes done

Copy link
Contributor

@dpoulopoulos dpoulopoulos left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks for this! I left a minor comment.

profiles:
- local
volumes:
- redis-data:/data
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a make target to allow the user to clean this, if they choose to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have talked about using labels to prune the right containers. IIRC the volumes can get pruned too with a certain flag. I'll check open PRs to see how far we've done of this. IMO this volume should have the same life cycle as the ray and redis containers, so there should be no need to clean it up separately.

@javiermtorres javiermtorres marked this pull request as ready for review January 27, 2025 20:06
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.

DB is persistent but ray jobs aren't
2 participants