Skip to content

[Java] Add loadResourcesFromLocal field in RayConfig.#3044

Closed
jovany-wang wants to merge 1 commit into
ray-project:masterfrom
antgroup:fix-driver-res
Closed

[Java] Add loadResourcesFromLocal field in RayConfig.#3044
jovany-wang wants to merge 1 commit into
ray-project:masterfrom
antgroup:fix-driver-res

Conversation

@jovany-wang

@jovany-wang jovany-wang commented Oct 10, 2018

Copy link
Copy Markdown
Contributor

What do these changes do?

https://github.com/ray-project/ray/blob/master/java/runtime/src/main/java/org/ray/runtime/functionmanager/FunctionManager.java#L92-L100

If user don't specify the driver resources path, it will catch an exception and set the classLoader to a default one. But it doesn't make sense: we shouldn't choose the code path by exception.

This PR added a field in config that user can specify the way the worker load resources: from local or not.

Related issue number

N/A

@AmplabJenkins

Copy link
Copy Markdown

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/8591/
Test PASSed.

@robertnishihara

Copy link
Copy Markdown
Collaborator

@jovan-wong what does it mean if a driver loads resources from a file? This is only for the case where the driver creates the Ray processes through ray.init, right? It doesn't make sense in the case where the driver attaches to an existing cluster, right? Or am I misinterpreting it?

Also, what is the use case for this? The application could just read the file and load the resources and then pass them into ray.init, right?

@jovany-wang

Copy link
Copy Markdown
Contributor Author

@robertnishihara
Sorry i didn't express it clearly.

  1. Re @jovan-wong what does it mean if a driver loads resources from a file? This is only for the case where the driver creates the Ray processes through ray.init, right? It doesn't make sense in the case where the driver attaches to an existing cluster, right? Or am I misinterpreting it?:
    I think you might misinterpretted it.
    For a worker, we should load the resources(jars package) to execute tasks.
    In cluster mode, when a driver(in node N0) is attached an existing cluster, it may submit task(name taskA), and the task may dispatch to other node(named N1), and one worker(W0) in node N1 will execute it. But if there is no taskA's jars in worker W0's classpath, it means that the worker W0 have no any code for taskA right?

So before these PRs(this one and #3001), if we want to run a driver successfully in a cluster mode, the only way is we put the jars resources under the worker W0's classpath.

It is friendly for user to configure classpath for driver code in worker node.
After these PRs, we can configure the driver codes(jars resources) in ray.conf file.

  1. Re Also, what is the use case for this? The application could just read the file and load the resources and then pass them into ray.init, right?:
    //use case:
    a. deployed driver resources(jars) on the target machine in advance.
    b. configure the ray.conf for workers.
    c. run driver.
    we just read the resource files(jars) when worker execute tasks.

// And if worker.mode is DRIVER, this field will not.
resource {
// Whether the worker should load resources from local path.
from-local: false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR looks good to me overall. But I think the naming is confusing. What this flag really means is whether each driver should load resources from its own path (instead of all driver loading resources from the same path). Thus, from-local doesn't express the exact meaning.

After thinking more about it, I think it might be better to just use one config ray.driver.resource.path. And we can document this config like this: If this config is set, worker will use different paths to load resources when executing tasks from different drivers. E.g., if it's set to '/tmp/driver_resource', the path for driver 123 will be '/tmp/driver_resource/123'

@jovany-wang

Copy link
Copy Markdown
Contributor Author

Thanks @robertnishihara and @raulchen .
I requested another PR #3046 to do this.

raulchen pushed a commit that referenced this pull request Oct 11, 2018
## What do these changes do?
Fix the issue how we load driver resources by a specified path.
Also this addressed the comments from the related PR [3044](#3044).

## Related PRs:
 [#3044](#3044) and [#3001](#3001).
@jovany-wang jovany-wang deleted the fix-driver-res branch October 19, 2018 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants