[core][3/3] Use the new standalone runtime env http server. #37585

rynewang · 2023-07-19T22:07:31Z

This is the final patch in the series. Changes:

Rewrites agent_manager.cc. Removed its ability to do agent registration (no longer needs registration) and proxying runtime env agent (moved to the runtime_env_agent_client.cc). It will only do agent starting but we will have 2 instances in node_manager starting a dashboard agent and a runtime env agent.
Deletes the runtime env agent python code from dashboard agent.
Deletes the agent registration grpc interface, and the runtime env agent interface.
Starts the standalone runtime env http server in services.py.
Adds the extra port for the server everywhere: in services.py, node.py and gcs.proto.
added 1 more port to Node info: runtime_env_agent_port. Intended to be used with raylet_address, but in some cases (1 test IIRC) we don't have one and it'll be used with node_address
updated all related tests. Most tests used to use dashboard agent's port, now they use runtime env agent's port.

Related issue number

Part of issue #35472.

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 added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
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 :(

rkooo567

Finished reviewing cpp parts. I will review the python part tmrw.

Q: I don't quite understand the reasoning behind "we don't need registration anymore". Can you tell me why we had it before and why we don't have it anymore?

src/ray/raylet/main.cc

src/ray/raylet/node_manager.h

rkooo567 · 2023-07-21T16:32:25Z

src/ray/raylet/node_manager.h

-  /// The agent manager RPC service.
-  std::unique_ptr<rpc::AgentManagerServiceHandler> agent_manager_service_handler_;
-  rpc::AgentManagerGrpcService agent_manager_service_;
+  /// Wrapper client for RuntimeEnvManager. Always non-null.


Why don't we make it a none-pointer?

we need to pass it to WorkerPool

it doesn't have to be a pointer to pass by argument?

src/ray/raylet/node_manager.cc

src/ray/raylet/agent_manager.cc

src/ray/raylet/agent_manager.h

jjyao · 2023-07-21T19:58:44Z

python/ray/_private/node.py

+    @property
+    def runtime_env_agent_address(self):
+        """Get the address that exposes runtime env agent as http"""
+        return f"http://{self._raylet_ip_address}:{self._runtime_env_agent_port}"


Should we use node_ip_address or raylet_ip_address?

We need to use raylet_ip_address because the agent is started by raylet and always co-locates with raylet. However there are some cases I don't have a raylet address so I falled back to node_ip address.

Also, would you mind sharing some cases where node_ip_address != raylet_address?

jjyao · 2023-07-21T19:59:41Z

python/ray/_private/node.py

@@ -1092,7 +1108,7 @@ def start_ray_client_server(self):
            stderr_file=stderr_file,
            redis_password=self._ray_params.redis_password,
            fate_share=self.kernel_fate_share,
-            metrics_agent_port=self._ray_params.metrics_agent_port,
+            runtime_env_agent_address=self.runtime_env_agent_address,


why do we need to pass in address instead of port as before?

it's used in proxier.py from client server. Previously it uses 127.0.0.1

https://github.com/ray-project/ray/blob/master/python/ray/util/client/server/proxier.py#L131

Putting the raylet address in makes it clearer who it's talking to. I don't really have an idea of which raylet the client would like to connect to, so I tried to make things explicit. If you like to revert it back to 127.0.0.1 I can do that.

jjyao · 2023-07-21T20:22:38Z

@ckw017 could you review the ray client changes?

rynewang · 2023-07-21T22:00:30Z

Finished reviewing cpp parts. I will review the python part tmrw.

Q: I don't quite understand the reasoning behind "we don't need registration anymore". Can you tell me why we had it before and why we don't have it anymore?

Previously, raylet starts a dashboard_agent. Then dashboard_agent calls "registration" RPC to raylet to tell its own grpc listen port. Then raylet talks to the runtime env agent in dashboard_agent via that port. The registration is needed primarily to share this port (along with the agent_id and address but raylet already knows these).

Now, I let services.py to pick a free port for runtime env http agent, and passes this as an arg to raylet, so raylet knows the port at start. Raylet starts a runtime env http agent with the port, so the http agent use it. There's no registration needed since raylet knows everything (address + port) even before http agent is started.

ckw017 · 2023-07-21T22:29:36Z

@ckw017 could you review the ray client changes?

Ray Client server changes seem fine to me (assuming tests are still passing), but afaik the runtime env parts were added by Ed, so maybe check with him instead.

rkooo567

One additional questions regarding the port assignment. Is it possible to choose it inside the agent? (or is it required by the client server, so we should assign it before the agent starts?) I think we should follow up with this issue. Agent port issue causes problems for a long time, and introducing a new agent will double the probability of this issue.

Now, I let services.py to pick a free port for runtime env http agent, and passes this as an arg to raylet, so raylet knows the port at start. Raylet starts a runtime env http agent with the port, so the http agent use it. There's no registration needed since raylet knows everything (address + port) even before http agent is started.

I think particularly we should avoid doing this (maybe we should discuss offline for the solution, and we have to handle this before 2.7).

dashboard/modules/log/log_manager.py

rkooo567 · 2023-07-25T14:30:37Z

python/ray/_private/services.py

+        ),
+        os.path.join(RAY_PATH, "_private", "runtime_env", "agent", "main.py"),
+        f"--node-ip-address={node_ip_address}",
+        f"--runtime-env-agent-port={runtime_env_agent_port}",


Is the runtime env agent port decided within the runtime env agent, or chosen before it is started? We had long term issue where agent port conflict causes issues because we choose it before agent is started (and it is due to some limitation we haven't fixed). It'd be the best we avoid the same issue from runtime env agent (the probability of port conflict will double.)

In this PR it's decided before the agent starts, by services.py. Per offline talk, I will investigate how to assign all ports in services.py in another PR.

src/ray/raylet/agent_manager.h

rkooo567

LGTM. Can you address the comments before merging it?

Also, have you run the release test in this PR or a separate one?

rynewang · 2023-07-26T17:25:31Z

LGTM. Can you address the comments before merging it?

Updated PR.

Also, have you run the release test in this PR or a separate one?

Yes, but I will do another round in the latest commit.

…ated Signed-off-by: Ruiyang Wang <[email protected]>

Signed-off-by: Ruiyang Wang <[email protected]>

Co-authored-by: angelinalg <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]>

…er.py Co-authored-by: angelinalg <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]>

Co-authored-by: angelinalg <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]>

Signed-off-by: Ruiyang Wang <[email protected]>

github-actions · 2023-08-02T22:05:49Z

Attention: External code changed

This PR changes code that is used or cited in external sources, e.g. blog posts.

Before merging this PR, please make sure that the code in the external sources is still working, and consider updating them to reflect the changes.

The affected files and the external sources are:

doc/external/pytorch_tutorials_hyperparameter_tuning_tutorial.py: https://pytorch.org/tutorials/beginner/hyperparameter_tuning_tutorial.html

rynewang · 2023-08-03T00:05:57Z

list of failed unit tests now:

//python/ray/tests:test_client_library_integration failed by [RLlib] DreamerV3: Minor fixes and enhancements. #37977
//python/ray/tests:test_output failed by [RLlib] DreamerV3: Minor fixes and enhancements. #37977
//python/ray/train:test_trainer_restore failed by [RLlib] DreamerV3: Minor fixes and enhancements. #37977
//python/ray/train:test_rl_trainer
semgrep failed by [air] pyarrow.fs persistence (3/n): Introduce new Checkpoint API #37925

looks like there's no newly introduced failures.

…ect#37585) Rewrites agent_manager.cc. Removed its ability to do agent registration (no longer needs registration) and proxying runtime env agent (moved to the runtime_env_agent_client.cc). It will only do agent starting but we will have 2 instances in node_manager starting a dashboard agent and a runtime env agent. Deletes the runtime env agent python code from dashboard agent. Deletes the agent registration grpc interface, and the runtime env agent interface. Starts the standalone runtime env http server in services.py. Adds the extra port for the server everywhere: in services.py, node.py and gcs.proto. added 1 more port to Node info: runtime_env_agent_port. Intended to be used with raylet_address, but in some cases (1 test IIRC) we don't have one and it'll be used with node_address updated all related tests. Most tests used to use dashboard agent's port, now they use runtime env agent's port. Signed-off-by: NripeshN <[email protected]>

…ect#37585) Rewrites agent_manager.cc. Removed its ability to do agent registration (no longer needs registration) and proxying runtime env agent (moved to the runtime_env_agent_client.cc). It will only do agent starting but we will have 2 instances in node_manager starting a dashboard agent and a runtime env agent. Deletes the runtime env agent python code from dashboard agent. Deletes the agent registration grpc interface, and the runtime env agent interface. Starts the standalone runtime env http server in services.py. Adds the extra port for the server everywhere: in services.py, node.py and gcs.proto. added 1 more port to Node info: runtime_env_agent_port. Intended to be used with raylet_address, but in some cases (1 test IIRC) we don't have one and it'll be used with node_address updated all related tests. Most tests used to use dashboard agent's port, now they use runtime env agent's port. Signed-off-by: e428265 <[email protected]>

…ect#37585) Rewrites agent_manager.cc. Removed its ability to do agent registration (no longer needs registration) and proxying runtime env agent (moved to the runtime_env_agent_client.cc). It will only do agent starting but we will have 2 instances in node_manager starting a dashboard agent and a runtime env agent. Deletes the runtime env agent python code from dashboard agent. Deletes the agent registration grpc interface, and the runtime env agent interface. Starts the standalone runtime env http server in services.py. Adds the extra port for the server everywhere: in services.py, node.py and gcs.proto. added 1 more port to Node info: runtime_env_agent_port. Intended to be used with raylet_address, but in some cases (1 test IIRC) we don't have one and it'll be used with node_address updated all related tests. Most tests used to use dashboard agent's port, now they use runtime env agent's port. Signed-off-by: Victor <[email protected]>

rynewang requested review from SongGuyang, raulchen, edoakes, architkulkarni, a team, wuisawesome, ericl, AmeerHajAli, robertnishihara and pcmoritz as code owners July 19, 2023 22:07

rynewang assigned rkooo567 Jul 19, 2023

rynewang requested a review from DmitriGekhtman as a code owner July 20, 2023 04:49

rkooo567 reviewed Jul 21, 2023

View reviewed changes

rkooo567 added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Jul 21, 2023

jjyao reviewed Jul 21, 2023

View reviewed changes

jjyao assigned ckw017 Jul 21, 2023

jjyao assigned SongGuyang Jul 21, 2023

rynewang removed the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Jul 21, 2023

rynewang force-pushed the runtime_env_http_final branch from 4cba2fa to bc23447 Compare July 24, 2023 20:06

rkooo567 reviewed Jul 25, 2023

View reviewed changes

rkooo567 added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Jul 25, 2023

rynewang removed the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Jul 25, 2023

rynewang mentioned this pull request Jul 26, 2023

[RFC] Runtime env agent: standlone python http server #36917

Closed

rkooo567 reviewed Jul 26, 2023

View reviewed changes

src/ray/raylet/agent_manager.h Outdated Show resolved Hide resolved

src/ray/raylet/agent_manager.h Outdated Show resolved Hide resolved

rkooo567 approved these changes Jul 26, 2023

View reviewed changes

rynewang and others added 17 commits August 2, 2023 18:05

explicitly mark tests to python3.7 until underlying docker images upd…

86e89c2

…ated Signed-off-by: Ruiyang Wang <[email protected]>

update the new test's version

09b33f2

Signed-off-by: Ruiyang Wang <[email protected]>

use byod

0a8497c

Signed-off-by: Ruiyang Wang <[email protected]>

update 3.7 to 3.8

40c6cfb

Signed-off-by: Ruiyang Wang <[email protected]>

fix more 3.8s

46ee887

Signed-off-by: Ruiyang Wang <[email protected]>

Update python/ray/_private/parameter.py

5c917ff

Co-authored-by: angelinalg <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]>

Update python/ray/util/state/state_manager.py

53bc791

Co-authored-by: angelinalg <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]>

Update release/nightly_tests/stress_tests/test_many_runtime_envs_runn…

055ebd2

…er.py Co-authored-by: angelinalg <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]>

Update src/ray/raylet/node_manager.cc

dd0407a

Co-authored-by: angelinalg <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]>

Update python/ray/_private/services.py

d726799

Co-authored-by: angelinalg <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]>

Update python/ray/_private/services.py

7c7210c

Co-authored-by: angelinalg <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]>

Update python/ray/_private/parameter.py

7e1c08a

Co-authored-by: angelinalg <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]>

Update src/ray/raylet/agent_manager.h

9b6359a

Co-authored-by: angelinalg <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]>

Update python/ray/_private/workers/default_worker.py

bea9978

Co-authored-by: angelinalg <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]>

Update python/ray/_private/parameter.py

5b9a1d7

Co-authored-by: angelinalg <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]> Signed-off-by: Ruiyang Wang <[email protected]>

remove leftover runtime env stub refs in unregister_agent_client

e8a1890

Signed-off-by: Ruiyang Wang <[email protected]>

revert release_tests.yaml, only add our new test

39059a4

Signed-off-by: Ruiyang Wang <[email protected]>

rynewang force-pushed the runtime_env_http_final branch from 7a7cc62 to bf3b73f Compare August 2, 2023 22:05

github-actions bot added the external-code-affected label Aug 2, 2023

rynewang force-pushed the runtime_env_http_final branch from bf3b73f to 39059a4 Compare August 2, 2023 22:15

aslonnie removed the request for review from a team August 3, 2023 00:35

rkooo567 merged commit 55dbf09 into ray-project:master Aug 3, 2023

stephanie-wang mentioned this pull request Sep 1, 2023

[CI] osx://python/ray/tests:test_reconstruction_2 is failing/flaky on master. #38277

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core][3/3] Use the new standalone runtime env http server. #37585

[core][3/3] Use the new standalone runtime env http server. #37585

rynewang commented Jul 19, 2023 •

edited

Loading

rkooo567 left a comment

rkooo567 Jul 21, 2023

rynewang Jul 21, 2023

rkooo567 Jul 26, 2023

jjyao Jul 21, 2023

rynewang Jul 21, 2023

jjyao Jul 21, 2023

rynewang Jul 21, 2023

jjyao commented Jul 21, 2023

rynewang commented Jul 21, 2023

ckw017 commented Jul 21, 2023

rkooo567 left a comment •

edited

Loading

rkooo567 Jul 25, 2023

rynewang Jul 25, 2023

rkooo567 left a comment

rynewang commented Jul 26, 2023

github-actions bot commented Aug 2, 2023

rynewang commented Aug 3, 2023

[core][3/3] Use the new standalone runtime env http server. #37585

[core][3/3] Use the new standalone runtime env http server. #37585

Conversation

rynewang commented Jul 19, 2023 • edited Loading

Related issue number

Checks

rkooo567 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jjyao commented Jul 21, 2023

rynewang commented Jul 21, 2023

ckw017 commented Jul 21, 2023

rkooo567 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rkooo567 left a comment

Choose a reason for hiding this comment

rynewang commented Jul 26, 2023

github-actions bot commented Aug 2, 2023

Attention: External code changed

rynewang commented Aug 3, 2023

rynewang commented Jul 19, 2023 •

edited

Loading

rkooo567 left a comment •

edited

Loading