[justinyeh1995]

Description

When ray.init("ray://...") fails, is_client_mode_enabled turns to true globally despite the failure. The upcoming ray.init() would then fail with a vague AssertionError without giving much information.

This PR raise an exception with clearer message when users call ray.init() while is_client_mode_enabled is True. This includes both successful and failing client connections.

Testing

cd ray
python -m pytest python/ray/tests/test_ray_init.py

Related issues

Fixes #67142

Additional information

[gemini-code-assist]

Code Review

This pull request adds a check in ray.init() to prevent starting or connecting to a cluster if Ray Client is already initialized, along with corresponding tests. However, the feedback highlights a significant issue: if a client connection fails, the is_client_mode_enabled flag is not reset, meaning the new check and tests incorrectly lock the process and block subsequent retries or fallbacks. The reviewer suggests resetting this flag upon connection failure instead of asserting the buggy behavior.

[gemini-code-assist]

This test asserts that calling ray.init() after a failed client connection raises a RuntimeError stating that Ray Client is already initialized. However, if a client connection fails, Ray Client is not successfully initialized, and the global is_client_mode_enabled flag should be reset to False.\n\nBy expecting a RuntimeError here, we are codifying a buggy behavior where a failed connection attempt permanently locks the process into client mode, preventing any subsequent fallback to a local Ray cluster or retries.\n\nInstead of asserting this error, we should fix the root cause in the client builder (where connect() is called) to ensure that is_client_mode_enabled is reset to False if the connection attempt fails. Once that is fixed, a subsequent ray.init() should be able to succeed.

[justinyeh1995]

Marking is_client_mode_enabled to False would be out of scope for #67142.

[gemini-code-assist]

While raising a clearer exception when Ray Client is already initialized is helpful, doing so when a previous connection attempt failed is problematic. If ray.init(\"ray://...\") fails, is_client_mode_enabled is left as True due to a lack of cleanup in the connection failure path.\n\nWith this check, any subsequent fallback to a local ray.init() or retry will be blocked by this RuntimeError, forcing the user to restart their Python process.\n\nWe should ensure that is_client_mode_enabled is properly reset to False upon connection failure in the client builder, rather than treating a failed connection as an 'already initialized' state.

[justinyeh1995]

same as above. #67142 clearly states that the fix should only raise a more informative exception rather than fixing the is_client_mode_enabled problem.

[Jadiouo]

Verified on a Ray nightly (4f54411d6) with your worker.py change applied (the branch merges an older master, so I applied the diff on top of the nightly), using a real ray start --head --ray-client-server-port head and fresh processes.

• After a failed ray:// connect, ray.init() now raises your RuntimeError instead of a bare AssertionError. Retrying ray.init("ray://<live>") and allow_multiple=True still connect, and ray.util.disconnect() still recovers, same as base. So recovery is unchanged and the message is clearer, which matches expected #1 in the issue.
• Your two new tests fail on base and pass with the change.

One question: after a successful client connect, ray.init() and ray.init(ignore_reinit_error=True) used to return quietly, and now they raise. Some library code calls the latter unconditionally, e.g. ray.init(runtime_env=..., ignore_reinit_error=True) in llm/_internal/batch/processor/vllm_engine_proc.py:160 (and the SGLang processor), so building those processors under Ray Client would now fail. I haven't run that path since it needs vLLM, this is from reading the code. Is that change intended, or should the check only apply when the client connection failed?

[justinyeh1995]

Hi @Jadiouo, thanks for the review!

Raising on double connection even if the previous connection succeeds is intended as I think it reflects the proposed error message "Ray Client is already initialized in this process. " better. Still, introducing a breaking change for users sounds like a bad idea at the same time. I'll check with the author first #67142 (comment) but I think your concern is valid.

[gemini-code-assist]

Code Review

This pull request introduces a check in ray.init() to raise a RuntimeError if Ray Client is already initialized, along with corresponding unit tests. However, the feedback highlights a critical issue: if a previous Ray Client connection attempt fails, the global client mode state is leaked and remains enabled. This causes the new check to incorrectly raise a RuntimeError and block users from falling back to a local Ray cluster. The reviewer recommends fixing this root cause by cleaning up the client mode state in RayAPIStub.connect() upon connection failure, which would render the new check in worker.py unnecessary and allow local initialization to succeed as expected.

[gemini-code-assist]

Issue: Misleading Error and Blocked Fallback due to Global State Leak\n\nThe check introduced here:\npython\nif client_mode_should_convert() and num_connected_contexts() == 0:\n raise RuntimeError(\"Ray Client is already initialized in this process.\")\n\nraises a misleading RuntimeError and blocks users from starting a local Ray cluster (via ray.init()) if a previous Ray Client connection attempt failed.\n\n#### Why this happens:\n1. When ray.init(\"ray://...\") is called, it enables client mode globally by calling _explicitly_enable_client_mode().\n2. If the connection attempt fails (e.g., throwing a ConnectionError), the exception is propagated, but _explicitly_disable_client_mode() is never called because RayAPIStub.connect() does not clean up on failure.\n3. This leaves is_client_mode_enabled as True globally, even though no client connection was ever established (num_connected_contexts() == 0).\n4. Consequently, any subsequent call to ray.init() (local) triggers this new check and raises RuntimeError: Ray Client is already initialized in this process., which is incorrect (it was never successfully initialized) and prevents the user from falling back to a local cluster.\n\n#### Recommended Solution:\nInstead of adding this check to ray.init(), we should fix the root cause by ensuring that is_client_mode_enabled is cleaned up/disabled if the connection attempt fails.\n\nWe can achieve this by wrapping the connection logic in RayAPIStub.connect() (located in python/ray/util/client/__init__.py) in a try...except block:\n\npython\n def connect(self, *args, **kw_args):\n self.get_context()._inside_client_test = self._inside_client_test\n try:\n conn = self.get_context().connect(*args, **kw_args)\n global _lock, _all_contexts\n with _lock:\n _all_contexts.add(self._cxt.handler)\n return conn\n except Exception:\n global _lock, _all_contexts\n with _lock:\n if len(_all_contexts) == 0:\n _explicitly_disable_client_mode()\n raise\n\n\nIf we fix the leak there, client_mode_should_convert() will correctly return False after a failed connection, and this check in worker.py becomes unnecessary and can be removed.

[gemini-code-assist]

This test asserts that calling ray.init() (local) after a failed client connection raises a RuntimeError. However, if a client connection failed, the user should be allowed to fall back to starting a local Ray cluster. Asserting that it raises RuntimeError enforces a broken/undesirable behavior caused by the global state leak of is_client_mode_enabled.\n\nOnce the leak is fixed in RayAPIStub.connect(), this test should instead assert that ray.init() successfully starts a local cluster.