-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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
Conversation
[WIP][Serve] Define java backend
There was a problem hiding this 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 |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
can you try merge master? the CI failure mostly come that environment issue |
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
scripts/format.sh
to lint the changes in this PR.