LuciferYang commented on code in PR #12525:
URL: https://github.com/apache/gluten/pull/12525#discussion_r3588469700
##########
tools/gluten-it/README.md:
##########
@@ -22,7 +22,7 @@ mvn clean package -P{Spark-Version}
sbin/gluten-it.sh
```
-Note: **Spark-Version** can only be **spark-3.2**, **spark-3.3**,
**spark-3.4** or **spark-3.5**.
+Note: **Spark-Version** can only be **spark-3.3**, **spark-3.4**,
**spark-3.5**, **spark-4.0** or **spark-4.1**.
Review Comment:
Reverted the gluten-it profile list to spark-3.3 / 3.4 / 3.5 for now.
sbin/gluten-it.sh currently hardcodes SPARK_SCALA_VERSION=2.12 and the wrapper
script does not derive Scala 2.13 from the -Pspark-4.0 / -Pspark-4.1 profiles,
so advertising those profiles here would be misleading. Making gluten-it
actually work on Spark 4.x is a separate change and out of scope for this
alignment PR.
##########
tools/gluten-it/common/src/main/java/org/apache/gluten/integration/SparkJvmOptions.java:
##########
@@ -25,13 +25,13 @@ public class SparkJvmOptions {
public static String read() {
try {
- final Class<?> clazz =
Class.forName("org.apache.spark.launcher.JavaModuleOptions");
+ final Class<?> clazz = Class.forName(MODULE_OPTIONS_CLASS_NAME);
final Method method = clazz.getMethod("defaultModuleOptions");
return (String) method.invoke(null);
- } catch (ClassNotFoundException e) {
- // Could happen in Spark 3.2 which doesn't have this class yet.
- return "";
- } catch (NoSuchMethodException | InvocationTargetException |
IllegalAccessException e) {
+ } catch (ClassNotFoundException
+ | NoSuchMethodException
+ | InvocationTargetException
+ | IllegalAccessException e) {
throw new RuntimeException(e);
Review Comment:
Added context to the RuntimeException: 'Failed to read Spark JVM module
options via org.apache.spark.launcher.JavaModuleOptions#defaultModuleOptions'.
Diagnosing classpath / Spark-version mismatches from sbin/gluten-it.sh should
now be much easier.
##########
shims/spark33/src/main/scala/org/apache/spark/sql/execution/datasources/orc/OrcFileFormat.scala:
##########
@@ -43,12 +43,10 @@ import org.apache.orc.mapreduce._
import java.io._
import java.net.URI
-/* -
- * This class is copied from Spark 3.2 and modified for Gluten. \n
- * Gluten should make sure this class is loaded before the original class.
- * If new Spark releases accepts changes and make this class incompatible,
- * we can move this class to shims-spark32,
- * shims-spark33, etc.
+/**
+ * This class is copied from Spark and modified for Gluten. Gluten should make
sure this class is
+ * loaded before the original class. If a new Spark release accepts changes
that make this class
+ * incompatible, we can move this class to a version-specific shim
(shims-spark33, etc.).
Review Comment:
Reworded the doc: 'we can move this file into a version-specific shim (e.g.
shims/spark34) so it is only used by the Spark versions that need it.' This
matches the actual layout (shims/spark33, shims/spark34, etc.) and the intent
(dedicate a version-specific shim once a divergence appears).
##########
docs/velox-backend-limitations.md:
##########
@@ -6,12 +6,12 @@ nav_order: 5
This document describes the limitations of velox backend by listing some known
cases where exception will be thrown, gluten behaves incompatibly with spark,
or certain plan's execution
must fall back to vanilla spark, etc.
-### Override of Spark classes (For Spark3.2 and Spark3.3)
-Gluten avoids to modify Spark's existing code and use Spark APIs if possible.
However, some APIs aren't exposed in Vanilla spark and we have to copy the
Spark file and do the hardcode changes. The list of override classes can be
found as ignoreClasses in package/pom.xml . If you use customized Spark, you
may check if the files are modified in your spark, otherwise your changes will
be overrided.
+### Override of Spark classes
+Gluten avoids to modify Spark's existing code and use Spark APIs if possible.
However, some APIs aren't exposed in Vanilla spark and we have to copy the
Spark file and do the hardcode changes. The list of override classes can be
found as ignoreClasses in package/pom.xml . If you use customized Spark, you
may check if the files are modified in your spark, otherwise your changes will
be overridden.
Review Comment:
Rewrote the paragraph for grammar and removed the stray space before the
period: 'Gluten avoids modifying Spark's existing code and prefers Spark APIs
when possible. However, some APIs are not exposed by vanilla Spark, so we have
to copy the Spark file and apply hardcoded changes. The list of overridden
classes can be found as \`ignoreClasses\` in \`package/pom.xml\`. If you use a
customized Spark, check whether these files are modified in your Spark
distribution, otherwise your changes will be overridden.'
##########
docs/get-started/Velox.md:
##########
@@ -7,18 +7,18 @@ parent: Getting-Started
# Supported Version
-| Type | Version |
-|-------|------------------------------|
-| Spark | 3.2.2, 3.3.1, 3.4.4, 3.5.5 |
-| OS | Ubuntu20.04/22.04, Centos7/8 |
-| jdk | openjdk8/jdk17 |
-| scala | 2.12 |
+| Type | Version |
+|-------|-------------------------------------|
+| Spark | 3.3.1, 3.4.4, 3.5.5, 4.0.2, 4.1.1 |
+| OS | Ubuntu20.04/22.04, Centos7/8 |
+| jdk | openjdk8/jdk17 |
+| scala | 2.12 |
Review Comment:
Added a note right below the version table: 'Spark 4.0 and 4.1 require JDK
17+ and Scala 2.13 (build with -Pspark-4.0 or -Pspark-4.1 plus -Pjava-17
-Pscala-2.13). Spark 3.3 to 3.5 can be built with JDK 8/17 and Scala 2.12.'
This calls out the JDK/Scala requirement matrix without changing the table
shape.
--
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]
