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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
SparkConnectEngineDemo #6365
SparkConnectEngineDemo #6365
Conversation
1. "$@" is a array, we want use string to compare. so update "$@" => "$*" 2. `tty` mean execute the command, we can use $(tty) replace it 3. param $# is a number, compare number should use -gt/-lt,not >/<
1. "$@" is a array, we want use string to compare. so update "$@" => "$*" 2. `tty` mean execute the command, we can use $(tty) replace it 3. param $# is a number, compare number should use -gt/-lt,not >/< 4. not sure the /bin/kyuubi line 63 'exit -1' need modify? so the directory bin only have a shellcheck note in /bin/kyuubi
# Conflicts: # bin/docker-image-tool.sh
Revert "fix_4186"
# Conflicts: # externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/kyuubi/SQLOperationListener.scala # externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/kyuubi/SparkConsoleProgressBar.scala
currently only support config(not test)
currently only support config(not test)
@pan3793 @yaooqinn @ulysses-you this pr is only a draft for discussion. What do you think about this method to compatible spark grpc ? |
Thank you @davidyuan1223 . We should add the grpc frontend service first, it is used to receive spark connect client request and send to connect engine. |
...gine/src/main/scala/org/apache/kyuubi/engine/spark/connect/SparkConnectFrontendService.scala
Show resolved
Hide resolved
@ulysses-you what do you think about the new module kyuubi-grpc(only structure, not write test case yet) |
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.
@davidyuan1223 is it possbile add spark-connect dependency and just delegate request to the SparkConnectService ? If so, we do not need to copy proto files.
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.
@ulysses-you yes, but i think, maybe we need to add other request/response, so my plan is put the proto files in kyuubi grpc, we can add kyuubi's interface based on the spark-connect proto, so not add the spark-connect common dependency
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.
Can you elaborate your plan ? I'm afrait things becomes complex if we decide to support connect based on proto.
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.
Can you elaborate your plan ? I'm afrait things becomes complex if we decide to support connect based on proto.
Actually, it's a assume, because the proto is based on spark, if we wanna support it with another engine, maybe need modify the proto? So i think put the proto in kyuubi-grpc could help us compite with other sql engine in the future. And if we wanna add spark-connect-common dependency, we need the spark version > 3.4, i think it's not compitable with other versions, because the rpc request will be handle by kyuubi's grpc backendService, not spark's rpc server, based on the proto, we could support rpc process in other spark version
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.
So i think put the proto in kyuubi-grpc could help us compite with other sql engine in the future.
It's too early to consider that. Even if we add proto, there is no standard proto interface, how can Spark proto compatible with other engine ? For each proto based engine, I think we can also add their server dependency.
And if we wanna add spark-connect-common dependency, we need the spark version > 3.4, i think it's not compitable with other versions, because the rpc request will be handle by kyuubi's grpc backendService, not spark's rpc server, based on the proto, we could support rpc process in other spark version
It sounds like just like copy all Spark connect server code into Kyuubi repo ? I think we only need to support connect since Spark3.5 and later, that's why we need a new module for Kyuubi Server. We do not need support older Spark version.
Besides, Add a Kyuubi grpc proto is fine to me for those statements, e.g., show kyuubi session/operators etc which are for Kyuubi itself.
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.
consume
So i think put the proto in kyuubi-grpc could help us compite with other sql engine in the future.
It's too early to consider that. Even if we add proto, there is no standard proto interface, how can Spark proto compatible with other engine ? For each proto based engine, I think we can also add their server dependency.
And if we wanna add spark-connect-common dependency, we need the spark version > 3.4, i think it's not compitable with other versions, because the rpc request will be handle by kyuubi's grpc backendService, not spark's rpc server, based on the proto, we could support rpc process in other spark version
It sounds like just like copy all Spark connect server code into Kyuubi repo ? I think we only need to support connect since Spark3.5 and later, that's why we need a new module for Kyuubi Server. We do not need support older Spark version.
Besides, Add a Kyuubi grpc proto is fine to me for those statements, e.g., show kyuubi session/operators etc which are for Kyuubi itself.
You are right, i will modify them.
@davidyuan1223 yeah, a new module for kyuubi grpc server is better. We can refactor and improve code after completing the connect frontend in future. |
馃攳 Description
Issue References 馃敆
This pull request fixes #
Describe Your Solution 馃敡
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Types of changes 馃敄
Test Plan 馃И
Behavior Without This Pull Request 鈿帮笍
Behavior With This Pull Request 馃帀
Related Unit Tests
Checklist 馃摑
Be nice. Be informative.