[Core][deprecate run_on_all_workers 1/n] set worker's sys.path through JobConfig._py_driver_sys_path

[scv119]

Why are these changes needed?

Today we use run_on_all_workers to set worker's system path, where the run_on_all_workers suffers from weak ordering guarantees and will be deprecated.

Instead, we should use JobConfig._py_driver_sys_path to set worker's system path, where the worker will add these paths into its sys.path on startup.

Note we have JobConfig.code_search_path which servers similar functionality, however it uses a different load_code_from_local code path and behaves differently and introduced bugs that failed tests (#17605 (comment)).

Related issue number

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

[cadedaniel]

Can you help me understand why this block needed to move before core worker initializatilon?

[scv119]

there are two (relevant) types of workers: drivers and task/actor execution workers.
the job_config flow from drivers -> raylet (worker_pool) -> executor-workers, both implicitly happens through coreworker constructor worker.core_worker = ray._raylet.CoreWorker, where driver set the job_config to raylet and executor receive the job_config from the raylet.

That's why we need to change it before core_worker initialization, as otherwise the job_config has already being sent to raylet.

[scv119]

btw how executor-worker get job_config will change by #31375 soon

[fishbone]

Thanks for fixing this. As synced offline, my only concern is that we make the code_search_path and the new introduced ones doing similar works. Maybe we should try to unify them.
One way is to introduce something like enable_load_code_from_local so that we make these two features:

1. we have a way to load from local instead of reading from GCS
2. we have a unified way to setup code search path

Not very strong opinion here if this require a lot of work.

[fishbone]

Btw, could you also delete import thread which is not useful anymore with this work.

[scv119]

Btw, could you also delete import thread which is not useful anymore with this work.

Deletion will be done in #30895, since @rkooo567 has some concern on deprecation logistic.

[scv119]

Thanks for fixing this. As synced offline, my only concern is that we make the code_search_path and the new introduced ones doing similar works. Maybe we should try to unify them.
One way is to introduce something like enable_load_code_from_local so that we make these two features:
we have a way to load from local instead of reading from GCS
we have a unified way to setup code search path
Not very strong opinion here if this require a lot of work.

Prototyped it and found it a bit more confusing to me... I'd propose we stick with with current implementation, since ultimately we will replace it by runtime env.