From 4e0effc72d8f1fdd6a7e5ab5baf6d151d96337de Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Fri, 17 Jul 2026 14:11:04 +0800 Subject: [PATCH 1/2] [MINOR] Clean up Spark 3.2 / 3.1-era compat leftovers in non-CH modules Now that Gluten supports only Spark 3.3+, drop a handful of comment and code leftovers referring to Spark 3.1 / 3.2: * `gluten-substrait/.../WriteFilesExecTransformer.scala`: comment updated from "Spark3.2/3.3/3.4" to "Spark 3.3/3.4", with an extra sentence clarifying that Spark 3.5 strips these fields natively via SPARK-43123. Manual cleanup is still needed for the 3.3/3.4 branches. * `gluten-arrow/.../ArrowColumnarBatch.scala`: rewrite the class docstring. The old claim "Spark-3.2 declares ColumnarBatch as final" is factually wrong on every currently supported branch (`public class ColumnarBatch implements AutoCloseable` on 3.3/3.4/3.5/4.0/4.1). Replaced with a factual note that this is a historical fork kept standalone to preserve the writable-column contract. * `gluten-celeborn/.../CelebornShuffleManager.java`: drop the "Added in SPARK-32055, for Spark 3.1 and above" comment. That condition holds on every currently supported Spark version, so the note has no signal. * `backends-velox/.../velox/VeloxFormatWriterInjects.scala`: drop the "Do NOT add override keyword for compatibility on spark 3.1" comment and add the `override` keyword to the anonymous `OutputWriter.path()`. Spark's `OutputWriter.path(): String` is abstract on every supported branch, so `override` is legal there and now matches the neighbouring `override def close()`. * `tools/gluten-it/common/.../SparkJvmOptions.java`: replace the reflection-based lookup of `org.apache.spark.launcher.JavaModuleOptions` with a direct `JavaModuleOptions.defaultModuleOptions()` call. The class is `public` with a public static `defaultModuleOptions()` method, documented `@since 3.3.0`, and reachable via the `spark-launcher` transitive dep of `spark-core` on every supported branch. The Spark 3.2 `ClassNotFoundException` fallback was already removed by #12525; this eliminates the remaining reflection scaffolding. No behavior change on any supported Spark version. --- .../velox/VeloxFormatWriterInjects.scala | 3 +-- .../vectorized/ArrowColumnarBatch.scala | 6 ++++-- .../celeborn/CelebornShuffleManager.java | 1 - .../execution/WriteFilesExecTransformer.scala | 3 ++- .../gluten/integration/SparkJvmOptions.java | 20 ++----------------- 5 files changed, 9 insertions(+), 24 deletions(-) diff --git a/backends-velox/src/main/scala/org/apache/spark/sql/execution/datasources/velox/VeloxFormatWriterInjects.scala b/backends-velox/src/main/scala/org/apache/spark/sql/execution/datasources/velox/VeloxFormatWriterInjects.scala index 06e9d91c0fa..f67446d846f 100644 --- a/backends-velox/src/main/scala/org/apache/spark/sql/execution/datasources/velox/VeloxFormatWriterInjects.scala +++ b/backends-velox/src/main/scala/org/apache/spark/sql/execution/datasources/velox/VeloxFormatWriterInjects.scala @@ -92,8 +92,7 @@ trait VeloxFormatWriterInjects extends GlutenFormatWriterInjectsBase { datasourceJniWrapper.close(dsHandle) } - // Do NOT add override keyword for compatibility on spark 3.1. - def path(): String = { + override def path(): String = { filePath } } diff --git a/gluten-arrow/src/main/scala/org/apache/gluten/vectorized/ArrowColumnarBatch.scala b/gluten-arrow/src/main/scala/org/apache/gluten/vectorized/ArrowColumnarBatch.scala index decd87e78f5..67e823452ef 100644 --- a/gluten-arrow/src/main/scala/org/apache/gluten/vectorized/ArrowColumnarBatch.scala +++ b/gluten-arrow/src/main/scala/org/apache/gluten/vectorized/ArrowColumnarBatch.scala @@ -20,8 +20,10 @@ import org.apache.spark.sql.catalyst.InternalRow import org.apache.spark.sql.vectorized.ColumnVector /** - * Because Spark-3.2 declares ColumnarBatch as final, so `ArrowColumnarBatch` can't extend - * `ColumnarBatch`. The code is mainly copied from Spark-3.2 + * A `ColumnarBatch`-like container of [[ArrowWritableColumnVector]]s. Originally forked from + * Spark's `ColumnarBatch` before Gluten could depend on that class directly; it is kept as a + * standalone class rather than extending `ColumnarBatch` to preserve the writable-column contract + * and existing call sites. * * @param writableColumns * the columns this class wraps diff --git a/gluten-celeborn/src/main/java/org/apache/spark/shuffle/gluten/celeborn/CelebornShuffleManager.java b/gluten-celeborn/src/main/java/org/apache/spark/shuffle/gluten/celeborn/CelebornShuffleManager.java index 31d82ce0c7d..0a82e327265 100644 --- a/gluten-celeborn/src/main/java/org/apache/spark/shuffle/gluten/celeborn/CelebornShuffleManager.java +++ b/gluten-celeborn/src/main/java/org/apache/spark/shuffle/gluten/celeborn/CelebornShuffleManager.java @@ -379,7 +379,6 @@ public ShuffleWriter getWriter( } } - // Added in SPARK-32055, for Spark 3.1 and above public ShuffleReader getReader( ShuffleHandle handle, int startMapIndex, diff --git a/gluten-substrait/src/main/scala/org/apache/gluten/execution/WriteFilesExecTransformer.scala b/gluten-substrait/src/main/scala/org/apache/gluten/execution/WriteFilesExecTransformer.scala index 13689f1b28a..b0bf2fd5b4e 100644 --- a/gluten-substrait/src/main/scala/org/apache/gluten/execution/WriteFilesExecTransformer.scala +++ b/gluten-substrait/src/main/scala/org/apache/gluten/execution/WriteFilesExecTransformer.scala @@ -287,7 +287,8 @@ object WriteFilesExecTransformer { .toLowerCase(Locale.ROOT) } - // To be compatible with Spark3.2/3.3/3.4, we do cleanup spark internal metadata manually. + // To be compatible with Spark 3.3/3.4, we do cleanup spark internal metadata manually. + // Spark 3.5 already strips these fields via SPARK-43123. // See https://github.com/apache/spark/pull/40776 private val INTERNAL_METADATA_KEYS = Seq( "__autoGeneratedAlias", diff --git a/tools/gluten-it/common/src/main/java/org/apache/gluten/integration/SparkJvmOptions.java b/tools/gluten-it/common/src/main/java/org/apache/gluten/integration/SparkJvmOptions.java index d2c2934eaa7..e29a7df6d12 100644 --- a/tools/gluten-it/common/src/main/java/org/apache/gluten/integration/SparkJvmOptions.java +++ b/tools/gluten-it/common/src/main/java/org/apache/gluten/integration/SparkJvmOptions.java @@ -16,28 +16,12 @@ */ package org.apache.gluten.integration; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; +import org.apache.spark.launcher.JavaModuleOptions; public class SparkJvmOptions { - private static final String MODULE_OPTIONS_CLASS_NAME = - "org.apache.spark.launcher.JavaModuleOptions"; public static String read() { - try { - final Class clazz = Class.forName(MODULE_OPTIONS_CLASS_NAME); - final Method method = clazz.getMethod("defaultModuleOptions"); - return (String) method.invoke(null); - } catch (ClassNotFoundException - | NoSuchMethodException - | InvocationTargetException - | IllegalAccessException e) { - throw new RuntimeException( - "Failed to read Spark JVM module options via " - + MODULE_OPTIONS_CLASS_NAME - + "#defaultModuleOptions", - e); - } + return JavaModuleOptions.defaultModuleOptions(); } public static void main(String[] args) { From bf250e982641ec722604fdf8868698caedd6b610 Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Fri, 17 Jul 2026 14:48:59 +0800 Subject: [PATCH 2/2] [MINOR] Reword WriteFilesExecTransformer comment for clarity Address Copilot review feedback: "we do cleanup spark internal metadata" reads awkwardly. Change to "we clean up Spark internal metadata". No code change. --- .../org/apache/gluten/execution/WriteFilesExecTransformer.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gluten-substrait/src/main/scala/org/apache/gluten/execution/WriteFilesExecTransformer.scala b/gluten-substrait/src/main/scala/org/apache/gluten/execution/WriteFilesExecTransformer.scala index b0bf2fd5b4e..98593a3e12b 100644 --- a/gluten-substrait/src/main/scala/org/apache/gluten/execution/WriteFilesExecTransformer.scala +++ b/gluten-substrait/src/main/scala/org/apache/gluten/execution/WriteFilesExecTransformer.scala @@ -287,7 +287,7 @@ object WriteFilesExecTransformer { .toLowerCase(Locale.ROOT) } - // To be compatible with Spark 3.3/3.4, we do cleanup spark internal metadata manually. + // To be compatible with Spark 3.3/3.4, we clean up Spark internal metadata manually. // Spark 3.5 already strips these fields via SPARK-43123. // See https://github.com/apache/spark/pull/40776 private val INTERNAL_METADATA_KEYS = Seq(