Revert "[serve] Replace Ray Client with Ray Job Submission for `serve… #32976

zcin · 2023-03-02T18:33:49Z

… run` (#30913)"

This reverts commit 72f7a7d.

Why are these changes needed?

Previously, if connecting to a local cluster, serve run will directly call ray.init() and serve.run
With the change in [serve] Replace Ray Client with Ray Job Submission for serve run #30913, even when connecting to a local cluster, serve run would submit a Ray Job to the local cluster and then call ray.init() and serve.run() inside that job. This caused the issue [serve] serve run command within job entrypoint fails if a working_dir is specified #32881, where if a user ran serve run inside a Job (or any other process with a runtime environment), they should expect to have access to the dependencies installed in that runtime environment. However since serve run goes through another layer of Job submission, they don't.

Related issue number

Closes #32881

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
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 :(

… run` (ray-project#30913)" This reverts commit 72f7a7d. Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

architkulkarni

Approving the revert, but let us know if you had to make any manual changes (e.g. to fix merge conflicts)

zcin · 2023-03-03T03:28:29Z

Yup, the following manual changes were made:

There were changes made to Ray Jobs (see changes in original PR) that I removed from this revert PR. tldr those changes allowed sending SIGINT instead of the SIGTERM as a job stop signal, and I didn't see the need to revert those changes, as there were merge conflicts and it's not related to the issue we are trying fix (LMK if there are concerns abt this).
I included the changes from [serve] Tear down serve on each serve run #31342 and [Serve] Change API to use ServeDeploySchema to replace ServeApplicationSchema #32285, which were both changes that affected run_script.py (which was a new file in the original PR). So with this revert, those changes needed to be added back into scripts.py. (However note that one of them was made after the 2.3.0 branch cut, so it will be removed when cherry-picked onto the release branch)
I edited the test test_run_teardown to work with the non-jobs-version of serve run (an issue with stdout vs stderr logs) (details).

zcin · 2023-03-03T03:30:50Z

Also, I made the same corresponding changes to the test PR on the release branch and tested it on workspaces and services, both work!

ray-project#32976) - Previously, if connecting to a local cluster, `serve run` will directly call `ray.init()` and `serve.run` - With the change in ray-project#30913, even when connecting to a local cluster, `serve run` would submit a Ray Job to the local cluster and then call `ray.init()` and `serve.run()` inside that job. This caused the issue ray-project#32881, where if a user ran `serve run` inside a Job (or any other process with a runtime environment), they should expect to have access to the dependencies installed in that runtime environment. However since `serve run` goes through another layer of Job submission, they don't. Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

#32976) (#33006) - Previously, if connecting to a local cluster, `serve run` will directly call `ray.init()` and `serve.run` - With the change in #30913, even when connecting to a local cluster, `serve run` would submit a Ray Job to the local cluster and then call `ray.init()` and `serve.run()` inside that job. This caused the issue #32881, where if a user ran `serve run` inside a Job (or any other process with a runtime environment), they should expect to have access to the dependencies installed in that runtime environment. However since `serve run` goes through another layer of Job submission, they don't. Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

ray-project#32976) - Previously, if connecting to a local cluster, `serve run` will directly call `ray.init()` and `serve.run` - With the change in ray-project#30913, even when connecting to a local cluster, `serve run` would submit a Ray Job to the local cluster and then call `ray.init()` and `serve.run()` inside that job. This caused the issue ray-project#32881, where if a user ran `serve run` inside a Job (or any other process with a runtime environment), they should expect to have access to the dependencies installed in that runtime environment. However since `serve run` goes through another layer of Job submission, they don't. Signed-off-by: Jack He <jackhe2345@gmail.com>

ray-project#32976) - Previously, if connecting to a local cluster, `serve run` will directly call `ray.init()` and `serve.run` - With the change in ray-project#30913, even when connecting to a local cluster, `serve run` would submit a Ray Job to the local cluster and then call `ray.init()` and `serve.run()` inside that job. This caused the issue ray-project#32881, where if a user ran `serve run` inside a Job (or any other process with a runtime environment), they should expect to have access to the dependencies installed in that runtime environment. However since `serve run` goes through another layer of Job submission, they don't.

ray-project#32976) - Previously, if connecting to a local cluster, `serve run` will directly call `ray.init()` and `serve.run` - With the change in ray-project#30913, even when connecting to a local cluster, `serve run` would submit a Ray Job to the local cluster and then call `ray.init()` and `serve.run()` inside that job. This caused the issue ray-project#32881, where if a user ran `serve run` inside a Job (or any other process with a runtime environment), they should expect to have access to the dependencies installed in that runtime environment. However since `serve run` goes through another layer of Job submission, they don't. Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>

ray-project#32976) - Previously, if connecting to a local cluster, `serve run` will directly call `ray.init()` and `serve.run` - With the change in ray-project#30913, even when connecting to a local cluster, `serve run` would submit a Ray Job to the local cluster and then call `ray.init()` and `serve.run()` inside that job. This caused the issue ray-project#32881, where if a user ran `serve run` inside a Job (or any other process with a runtime environment), they should expect to have access to the dependencies installed in that runtime environment. However since `serve run` goes through another layer of Job submission, they don't.

ray-project#32976) - Previously, if connecting to a local cluster, `serve run` will directly call `ray.init()` and `serve.run` - With the change in ray-project#30913, even when connecting to a local cluster, `serve run` would submit a Ray Job to the local cluster and then call `ray.init()` and `serve.run()` inside that job. This caused the issue ray-project#32881, where if a user ran `serve run` inside a Job (or any other process with a runtime environment), they should expect to have access to the dependencies installed in that runtime environment. However since `serve run` goes through another layer of Job submission, they don't. Signed-off-by: elliottower <elliot@elliottower.com>

ray-project#32976) - Previously, if connecting to a local cluster, `serve run` will directly call `ray.init()` and `serve.run` - With the change in ray-project#30913, even when connecting to a local cluster, `serve run` would submit a Ray Job to the local cluster and then call `ray.init()` and `serve.run()` inside that job. This caused the issue ray-project#32881, where if a user ran `serve run` inside a Job (or any other process with a runtime environment), they should expect to have access to the dependencies installed in that runtime environment. However since `serve run` goes through another layer of Job submission, they don't. Signed-off-by: Jack He <jackhe2345@gmail.com>

zcin added 4 commits March 2, 2023 10:33

Revert "[serve] Replace Ray Client with Ray Job Submission for `serve…

f5bc62b

… run` (ray-project#30913)" This reverts commit 72f7a7d. Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

don't revert job changes

aae1e6e

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

include change from ray-project#31342 and ray-project#32285

700db50

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

fix test

770278a

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

zcin added the v2.3.1-pick label Mar 2, 2023

zcin marked this pull request as ready for review March 3, 2023 00:50

zcin requested review from edoakes, simon-mo, jiaodong, shrekris-anyscale, sihanwang41, architkulkarni and a team as code owners March 3, 2023 00:50

zcin assigned architkulkarni, sihanwang41, edoakes, zcin and shrekris-anyscale and unassigned zcin Mar 3, 2023

architkulkarni approved these changes Mar 3, 2023

View reviewed changes

edoakes approved these changes Mar 3, 2023

View reviewed changes

edoakes merged commit 80384c3 into ray-project:master Mar 3, 2023

zcin mentioned this pull request Mar 3, 2023

Revert "[serve] Replace Ray Client with Ray Job Submission for `serve… #33006

Merged

7 tasks

zcin deleted the serve-run-revert-master branch March 3, 2023 19:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert "[serve] Replace Ray Client with Ray Job Submission for `serve… #32976

Revert "[serve] Replace Ray Client with Ray Job Submission for `serve… #32976

zcin commented Mar 2, 2023 •

edited

architkulkarni left a comment

zcin commented Mar 3, 2023 •

edited

zcin commented Mar 3, 2023

Revert "[serve] Replace Ray Client with Ray Job Submission for `serve… #32976

Revert "[serve] Replace Ray Client with Ray Job Submission for `serve… #32976

Conversation

zcin commented Mar 2, 2023 • edited

Why are these changes needed?

Related issue number

Checks

architkulkarni left a comment

Choose a reason for hiding this comment

zcin commented Mar 3, 2023 • edited

zcin commented Mar 3, 2023

zcin commented Mar 2, 2023 •

edited

zcin commented Mar 3, 2023 •

edited