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

[dashboard] Remove redis in dashboard #22788

Merged
merged 8 commits into from
Mar 4, 2022

Conversation

fishbone
Copy link
Contributor

@fishbone fishbone commented Mar 2, 2022

Why are these changes needed?

As we are turning redisless ray by default, dashboard doesn't need to talk with redis anymore. Instead it should talk with gcs and gcs can talk with redis.

Clean up the code.

Related issue number

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@fishbone fishbone changed the title Remove redis in dashboard [dashboard] Remove redis in dashboard Mar 3, 2022
@fishbone fishbone marked this pull request as ready for review March 3, 2022 22:03
@fishbone
Copy link
Contributor Author

fishbone commented Mar 3, 2022

The next step is to remove ray core - python redis related code. And then I'll remove redis from setup.py

Copy link
Contributor

@ericl ericl left a comment

Choose a reason for hiding this comment

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

LGTM! Btw, should we remove our "GCS HA" buildkite builds? Or should those test the "with Redis" path?

python/setup.py Outdated Show resolved Hide resolved
@ericl ericl added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Mar 3, 2022
@@ -220,24 +189,7 @@ def _load_modules(self):
return modules

async def get_gcs_address(self):
Copy link
Member

Choose a reason for hiding this comment

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

Remove async keyword, or migrate callers to self.gcs_address?

dashboard/modules/reporter/reporter_agent.py Outdated Show resolved Hide resolved
@fishbone
Copy link
Contributor Author

fishbone commented Mar 3, 2022

LGTM! Btw, should we remove our "GCS HA" buildkite builds? Or should those test the "with Redis" path?

I think redis path has some issues right now because we didn't test that :( and we'll need to fix it and add with redis path.

So in the end, we'll test memory storage and redis storage code path.

@fishbone
Copy link
Contributor Author

fishbone commented Mar 3, 2022

We'll fix that once booststrap and kv redis code paths are deleted.
@mwtian will work on delete pubsub redis code path. But I think it's won't impact the fixing of redis storage code path.

@fishbone fishbone merged commit 11bbf00 into ray-project:master Mar 4, 2022
frosk1 pushed a commit to frosk1/ray that referenced this pull request Mar 9, 2022
As we are turning redisless ray by default, dashboard doesn't need to talk with redis anymore. Instead it should talk with gcs and gcs can talk with redis.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants