Skip to content

[Java] Add jvm-parameters in Config.#3065

Merged
raulchen merged 4 commits into
ray-project:masterfrom
antgroup:java-jvm-params
Oct 16, 2018
Merged

[Java] Add jvm-parameters in Config.#3065
raulchen merged 4 commits into
ray-project:masterfrom
antgroup:java-jvm-params

Conversation

@jovany-wang

Copy link
Copy Markdown
Contributor

What do these changes do?

We couldn't specify the system properties for a Java worker now. If we want to do this, we must write the ray.conf in worker classpathes. Obviously, it's not convenient.

This PR add a jvm-parameters item in config that we can specify the system properties for a Java worker.

Related issue number

This fixed the 2nd point in the issue #3062 :
can not set worker jvm parameters in config file.

@jovany-wang jovany-wang requested a review from raulchen October 15, 2018 07:14
@jovany-wang

Copy link
Copy Markdown
Contributor Author

@chuxi cc

@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/8656/
Test PASSed.

@raulchen raulchen left a comment

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.

Thanks, I left 2 small comments.

if (config.hasPath("ray.jvm-parameters")) {
jvmParameters = config.getStringList("ray.jvm-parameters");
} else {
jvmParameters = null;

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.

Use empty list (ImmutableList.of())

// custom classpath
classpath = config.getStringList("ray.classpath");
// custom worker jvm parameters
if (config.hasPath("ray.jvm-parameters")) {

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.

better rename it to ray.worker.jvm-parameters.

@jovany-wang

Copy link
Copy Markdown
Contributor Author

@raulchen Addressed

@raulchen raulchen left a comment

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.

LGTM. will merge when CI passes

@AmplabJenkins

Copy link
Copy Markdown

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

@guoyuhong

Copy link
Copy Markdown
Contributor

@AmplabJenkins retest this, please.

@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/8671/
Test PASSed.

@raulchen raulchen merged commit 64e5eb3 into ray-project:master Oct 16, 2018
@jovany-wang jovany-wang deleted the java-jvm-params 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