[Client][Proxy] Prevent Logstream from Timing Out when Delays in DataClient

[ijrsvt]

Why are these changes needed?

This ensures that the LogstreamServicer does not time out fetching a channel when the DataStream initial connection takes a long time.

Related issue number

Closes #16178

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 :(
• Manually testing that connections still happen with time.sleep(10) in rewrite_runtime_env_uris.

[ericl]

Did you test this fails before the fix?

[ijrsvt]

@ericl Added a check to ensure that logsclient is working! I had previously tested this interactively and forgot that the logclient thread failing mainly prints a scary warning.

[mwtian]

Sorry I don't think I grok all details about this change. Besides moving the logic in the new create_specific_server() to run earlier, and skipping not-ready servers in the background _check_processes() thread, what other changes helps prevent Logstream from timing out?

[mwtian]

LGTM!

[ijrsvt]

Sorry I don't think I grok all details about this change. Besides moving the logic in the new create_specific_server() to run earlier, and skipping not-ready servers in the background _check_processes() thread, what other changes helps prevent Logstream from timing out?

No worries @mwtian !

Moving the creation logic to create_specific_server() means that when the Logstream makes a call to get_channel the call will wait on wait_ready() as opposed to fail to find the SpecificServer!

I prefer to keep the creation logic associated with Dataclient/Datapath because that is how the server normally manages the lifecycle of the connection (i.e. the DataServicer manages the num_clients https://github.com/ray-project/ray/blob/master/python/ray/util/client/server/dataservicer.py )

[mwtian]

Sorry I don't think I grok all details about this change. Besides moving the logic in the new create_specific_server() to run earlier, and skipping not-ready servers in the background _check_processes() thread, what other changes helps prevent Logstream from timing out?

No worries @mwtian !

Moving the creation logic to create_specific_server() means that when the Logstream makes a call to get_channel the call will wait on wait_ready() as opposed to fail to find the SpecificServer!

I prefer to keep the creation logic associated with Dataclient/Datapath because that is how the server normally manages the lifecycle of the connection (i.e. the DataServicer manages the num_clients https://github.com/ray-project/ray/blob/master/python/ray/util/client/server/dataservicer.py )

I see. It seems in the existing implementation, start_specific_server() creates the SpecificServer for client_id and is also called before get_channel(). How does the server fail to be found currently?

[ijrsvt]

@mwtian The current failure is that the SpecificServer is created after the InitRequest is received. This can be delayed if hashing the working directory takes a long time here:

ray/python/ray/util/client/worker.py

Line 463 in e1da31f

runtime_env.rewrite_runtime_env_uris(job_config)