LuciferYang commented on code in PR #39866:
URL: https://github.com/apache/spark/pull/39866#discussion_r1096022999
##########
connector/connect/client/jvm/pom.xml:
##########
@@ -104,39 +106,70 @@
<shadedArtifactAttached>false</shadedArtifactAttached>
<artifactSet>
<includes>
- <include>com.google.guava:*</include>
- <include>io.grpc:*</include>
- <include>com.google.protobuf:*</include>
<include>org.apache.spark:spark-connect-common_${scala.binary.version}</include>
+ <include>com.google.android:*</include>
+ <include>com.google.api.grpc:*</include>
+ <include>com.google.code.findbugs:*</include>
+ <include>com.google.code.gson:*</include>
+ <include>com.google.errorprone:*</include>
+ <include>com.google.j2objc:*</include>
+ <include>com.google.protobuf:*</include>
+ <include>io.grpc:*</include>
+ <include>io.netty:*</include>
+ <include>io.perfmark:*</include>
+ <include>org.codehaus.mojo:*</include>
+ <include>org.objenesis:*</include>
</includes>
</artifactSet>
<relocations>
<relocation>
<pattern>io.grpc</pattern>
-
<shadedPattern>${spark.shade.packageName}.connect.client.grpc</shadedPattern>
+
<shadedPattern>${spark.shade.packageName}.connect.client.io.grpc</shadedPattern>
<includes>
<include>io.grpc.**</include>
</includes>
</relocation>
<relocation>
- <pattern>com.google.protobuf</pattern>
-
<shadedPattern>${spark.shade.packageName}.connect.protobuf</shadedPattern>
- <includes>
- <include>com.google.protobuf.**</include>
- </includes>
+ <pattern>com.google</pattern>
+
<shadedPattern>${spark.shade.packageName}.connect.client.com.google</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>io.netty</pattern>
+
<shadedPattern>${spark.shade.packageName}.connect.client.io.netty</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.checkerframework</pattern>
+
<shadedPattern>${spark.shade.packageName}.connect.client.checkerframework</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>javax.annotation</pattern>
+
<shadedPattern>${spark.shade.packageName}.connect.client.javax.annotation</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>io.perfmark</pattern>
+
<shadedPattern>${spark.shade.packageName}.connect.client.io.perfmark</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.codehaus</pattern>
+
<shadedPattern>${spark.shade.packageName}.connect.client.org.codehaus</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>android.annotation</pattern>
+
<shadedPattern>${spark.shade.packageName}.connect.client.android.annotation</shadedPattern>
</relocation>
+ <!-- Relocate the protos from spark-common into a client only
package -->
<relocation>
- <pattern>com.google.common</pattern>
-
<shadedPattern>${spark.shade.packageName}.connect.client.guava</shadedPattern>
+ <pattern>org.apache.spark.connect.proto</pattern>
+
<shadedPattern>${spark.shade.packageName}.connect.client.proto</shadedPattern>
<includes>
- <include>com.google.common.**</include>
+ <include>org.apache.spark.connect.proto.**</include>
</includes>
</relocation>
<relocation>
- <pattern>com.google.thirdparty</pattern>
-
<shadedPattern>${spark.shade.packageName}.connect.client.guava</shadedPattern>
+ <pattern>org.apache.spark.sql.connect.common</pattern>
Review Comment:
> It allows client and server in the same JVM
`client` and `server` should not in the same JVM due to `server` depends on
`sql` on runtime , but `client` and `sql` have many classes with the same
`packageName + className`, let `client` and `sql` in same JVM will cause some
issue with class loading. For example, When `client` and `sql` are in the same
classpath, the JVM will only load one `org.apache.spark.sql.SparkSession`,
which may from `client` or `sql`, unless we manually use a different
`ClassLoader` to load them, otherwise, JVM cannot load and use two different
`org.apache.spark.sql.SparkSession` in the same process.
> client 3.4 + server 4.0 in the same JVM
I don't understand why relocation `common` can support this. For the class
in `common`, they are still in the same package and some classes have some
class name, maybe adding the spark version in to package name can make them
really different. Of course, even then, there will be the problems mentioned
above
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]