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

[WIP] [Placement Group] resizing placement group feature part1: dynamic add bundles. #19753

Conversation

clay4444
Copy link
Contributor

Why are these changes needed?

resizing placement group feature part1: dynamically add bundles.

the design doc API proposal

Note that there is a new update at the end of the document.

Python integration tests will be added later.

Related issue number

Closes #16403

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 :(

@bveeramani
Copy link
Member

‼️ ACTION REQUIRED ‼️

We've switched our code formatter from YAPF to Black (see #21311).

To prevent issues with merging your code, here's what you'll need to do:

  1. Install Black
pip install -I black==21.12b0
  1. Format changed files with Black
curl -o format-changed.sh https://gist.githubusercontent.com/bveeramani/42ef0e9e387b755a8a735b084af976f2/raw/7631276790765d555c423b8db2b679fd957b984a/format-changed.sh
chmod +x ./format-changed.sh
./format-changed.sh
rm format-changed.sh
  1. Commit your changes.
git add --all
git commit -m "Format Python code with Black"
  1. Merge master into your branch.
git pull upstream master
  1. Resolve merge conflicts (if necessary).

After running these steps, you'll have the updated format.sh.

@rkooo567
Copy link
Contributor

cc @edoakes

@clay4444
Copy link
Contributor Author

sorry, Sang, @rkooo567 , The pub-sub module was refactored too much, Do you know how can I configure enable Redis pub-sub for the test case?

@rkooo567
Copy link
Contributor

rkooo567 commented Mar 7, 2022

Hey @clay4444 this has slipped for some reasons.

I think you need to set

Screen Shot 2022-03-07 at 3 17 54 PM

false, 'redis', false

cc @mwtian can you confirm if this is right?

@mwtian
Copy link
Member

mwtian commented Mar 8, 2022

Redis pubsub is no longer supported. Ray only supports external Redis KV now. I guess we need a guide for adding new topics to Ray pubsub.

@wuisawesome
Copy link
Contributor

Hey @clay4444 how will this PR work with the autoscaler? Can we make sure we understand the plan for how this will work with autoscaling before merging?

@mwtian
Copy link
Member

mwtian commented Mar 8, 2022

This is an example of adding a channel to Ray pubsub: https://github.com/ray-project/ray/pull/20075/files

@kfstorm
Copy link
Member

kfstorm commented Mar 13, 2022

‼️ ACTION REQUIRED ‼️

We've updated our formatting configuration for C++ code. (see #22725)

This PR includes C++ code change. To prevent issues with merging your code, here's what you'll need to do:

  1. Merge the latest changes from upstream/master branch into your branch.
git pull upstream master
git merge upstream/master
  1. Resolve merge conflicts (if necessary).

After running these steps, you'll have the updated C++ formatting configuration.

  1. Format changed files.
scripts/format.sh
  1. Commit your changes.
git add --all
git commit -m "Format C++ code"

@stale
Copy link

stale bot commented Apr 16, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

  • If you'd like to keep this open, just leave any comment, and the stale label will be removed.

@stale stale bot added the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Apr 16, 2022
@rkooo567 rkooo567 removed the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Apr 18, 2022
@rkooo567
Copy link
Contributor

It's not stale

@scv119 scv119 added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label May 3, 2022
@stale
Copy link

stale bot commented Jun 19, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

  • If you'd like to keep this open, just leave any comment, and the stale label will be removed.

@stale stale bot added the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Jun 19, 2022
@stale
Copy link

stale bot commented Jul 13, 2022

Hi again! The issue will be closed because there has been no more activity in the 14 days since the last message.

Please feel free to reopen or open a new issue if you'd still like it to be addressed.

Again, you can always ask for help on our discussion forum or Ray's public slack channel.

Thanks again for opening the issue!

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. stale The issue is stale. It will be closed within 7 days unless there are further conversation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support resizing placement groups
8 participants