zozo123 commented on code in PR #59230: URL: https://github.com/apache/spark/pull/59230#discussion_r4280268318
########## udf/worker/runtime/pom.xml: ########## @@ -0,0 +1,200 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ Licensed to the Apache Software Foundation (ASF) under one or more + ~ contributor license agreements. See the NOTICE file distributed with + ~ this work for additional information regarding copyright ownership. + ~ The ASF licenses this file to You under the Apache License, Version 2.0 + ~ (the "License"); you may not use this file except in compliance with + ~ the License. You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, software + ~ distributed under the License is distributed on an "AS IS" BASIS, + ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + ~ See the License for the specific language governing permissions and + ~ limitations under the License. + --> + +<project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 + http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>org.apache.spark</groupId> + <artifactId>spark-parent_2.13</artifactId> + <version>5.0.0-SNAPSHOT</version> + <relativePath>../../../pom.xml</relativePath> + </parent> + + <artifactId>spark-udf-worker-runtime_2.13</artifactId> + <packaging>jar</packaging> + <name>Spark Project UDF Worker Runtime</name> + <url>https://spark.apache.org/</url> + + <properties> + <sbt.project.name>udf-worker-runtime</sbt.project.name> + </properties> + + <!-- + Packages spark-udf-worker-grpc for the Spark runtime classpath. This module has + no main sources: its jar is spark-udf-worker-grpc plus the gRPC runtime, + with gRPC, Guava and their small helper libraries relocated under + org.sparkproject.spark_udf_worker so they cannot clash with Spark Connect's + relocated gRPC or with the Guava that Spark exposes to applications. + + Two dependencies are deliberately left unrelocated and unbundled: + - Netty. gRPC's Netty transport links to the Netty that Spark ships, including + the epoll and kqueue native transports that Unix domain sockets need. + Relocating Netty would change the native library names it loads. + - protobuf-java. The generated stubs and gRPC's protobuf marshallers must use + the same protobuf classes as the spark-udf-worker-proto messages, which are + not part of this jar. + + The relocations must stay consistent with UDFWorkerRuntime in + project/SparkBuild.scala. + --> + <dependencies> + <dependency> + <groupId>org.apache.spark</groupId> + <artifactId>spark-udf-worker-grpc_${scala.binary.version}</artifactId> + <version>${project.version}</version> + </dependency> + <!-- Bundled and relocated below; the parent manages both as provided. --> + <dependency> + <groupId>com.google.guava</groupId> + <artifactId>guava</artifactId> + <scope>compile</scope> + </dependency> + <dependency> + <groupId>com.google.guava</groupId> + <artifactId>failureaccess</artifactId> + <scope>compile</scope> + </dependency> + <!-- + The jar links to classes it does not bundle: the UDF worker proto and core + modules, Netty and protobuf-java. Spark provides them, and declaring Spark core Review Comment: @HyukjinKwon your classpath reading is right, and each point in the review needs its own answer. **POM comments.** I built the Maven distribution at `7669c969` with `./build/mvn -B -pl assembly -am -DskipTests package` on JDK 21 and loaded classes from `assembly/target/scala-2.13/jars`. No jar contains `com/google/protobuf/GeneratedMessage.class`. `Class.forName("org.apache.spark.udf.worker.Init")` fails with `NoClassDefFoundError: com/google/protobuf/MessageOrBuilder`. `DataRequest` fails the same way. `DirectDispatcherFactory` loads. The same claim in `udf/worker/grpc/pom.xml` is wrong for that distribution too. A comment that says unrelocated protobuf-java is present only in sbt `jars` directories would match that measurement. The comment still would not make a Maven package run an external UDF. The Maven `spark-sql` jar calls `DataRequest$Builder.setData(Lorg/sparkproject/spark_core/protobuf/ByteString;)`. The `spark-udf-worker-proto` jar defines `setData(com.google.protobuf.ByteString)`. `PythonInitAdapter` has the same split on `setPayload`, `setInputSchema`, and `setOutputSchema`. The sbt suites pass because sbt does not shade `sql/core` and copies `protobuf-java` into `jars`. The follow-up will take protobuf type names out of those engine call sites and put the proto messages, with one relocated protobuf copy, in the runtime jar. The POM comments will then state that contract and the relocation coupling, in the file you pointed at. **Scaladoc.** The sentence in `ExecuteExternalUDFExec` that says worker creation currently fails is stale. `PlanExternalUDFsSuite` already asserts rows from a real worker. I will replace that sentence with the current behavior. The following sentence stays accurate. Flow control remains required before this leaves experimental status. **CI.** I will add `udf-worker-runtime` to the workflow group that already packages the assembly, and keep `udf-worker` selected. Those suites are the guard on the hand-maintained shade rules. Leaving them out of CI is not the choice I want. **Flow control.** Shipping behind `spark.sql.execution.udf.unified.execution.enabled`, which defaults to false, is not the production path named by the `GrpcWorkerSession` TODO. I will update that TODO and the `ExecuteExternalUDFExec` note. Both will say the transport ships behind the experimental flag, and that flow control is still required before the flag leaves experimental status. Flow control itself stays out of this packaging change. The pull request is draft until a Maven-built distribution passes the same packaged worker test the sbt suite runs. -- 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]
