Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Serve] Define BackendConfig protobuf and adapt it in Java #17201

Merged
merged 30 commits into from
Aug 6, 2021

Conversation

liuyang-my
Copy link
Contributor

Why are these changes needed?

This PR is about Java users using Ray Serve. We defined the proto file of BackendConfig, so that Python Controller could startup Java Backend Actor.

Detailed design is in doc :https://docs.google.com/document/d/1b9VCZsC2jVhWw_c_ifMu_ED3NxkARcOnU-OFqUYu-r4/edit#heading=h.b3vvz7iwnixn

Related issue number

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

@liuyang-my liuyang-my added enhancement Request for new feature and/or capability serve Ray Serve Related Issue labels Jul 20, 2021
@liuyang-my liuyang-my linked an issue Jul 20, 2021 that may be closed by this pull request
Copy link
Contributor

@simon-mo simon-mo left a comment

Choose a reason for hiding this comment

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

LGTM overall, main question is about the use of Hessian as a serialization format for initArgs and user config.

Object result = Hessian2Seserializer.decode(initArgsbytes);
return (Object[]) result;
} else {
// TODO
Copy link
Contributor

Choose a reason for hiding this comment

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

explain the TODO?

@@ -0,0 +1,37 @@
package io.ray.serve.serializer;

import com.caucho.hessian.io.Hessian2Input;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we use Hessian as the serialization format here? If it's expected to be used cross language we should use messagepack, if not is Hessian a standard serialization protocol for Java? If so does Ray core use it?

Choose a reason for hiding this comment

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

Hesssian's compatibility is better when adding new properties to the object. But MsgPack performance better in cross-language scenarios. I think MsgPack is the right choice here.

@simon-mo simon-mo changed the title Define BackendConfig protobuf and adapt it in Java [Serve] Define BackendConfig protobuf and adapt it in Java Jul 20, 2021
@simon-mo
Copy link
Contributor

simon-mo commented Aug 3, 2021

can you try merge master? the CI failure mostly come that environment issue

@simon-mo simon-mo merged commit 12bd904 into ray-project:master Aug 6, 2021
@jovany-wang jovany-wang deleted the java-backend-config-pb branch August 7, 2021 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Request for new feature and/or capability serve Ray Serve Related Issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[serve] create protobuf definition for cross language calls
3 participants