You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "LuciferYang (via GitHub)" <gi...@apache.org> on 2026/09/26 08:32:20 UTC
[PR] [MINOR][CORE] Treat linkage failures as class-not-present in isClassPresent [gluten]
LuciferYang opened a new pull request, #13146:
URL: https://github.com/apache/gluten/pull/13146
## What changes are proposed in this pull request?
`SparkReflectionUtil.isClassPresent` is used to detect optional connector extensions (Delta, Hudi, Paimon, Iceberg) across the backends. It calls `classForName(..., initialize = true)` and only catches `ClassNotFoundException`. When such a class is on the classpath but cannot be linked — a connector built against a different Spark version with a missing supertype, or a failing static initializer — `classForName` throws a `LinkageError` (e.g. `NoClassDefFoundError`, `ExceptionInInitializerError`) instead. That escapes the probe and crashes component discovery at startup, which is exactly the version-skew case the probe exists to tolerate.
This catches `LinkageError` alongside `ClassNotFoundException` and returns false, so an unlinkable class reads as "not present" like any other unusable optional dependency.
## How was this patch tested?
New `SparkReflectionUtilSuite`: a present-and-linkable class reads true, an absent class reads false, and a present class whose static initializer throws (surfacing as a `LinkageError`) now reads false instead of propagating.
## Was this patch authored or co-authored using generative AI tooling?
Yes. Generated-by: Claude Opus.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [MINOR][CORE] Treat linkage failures as class-not-present in isClassPresent [gluten]
Posted by "jackylee-ch (via GitHub)" <gi...@apache.org>.
jackylee-ch merged PR #13146:
URL: https://github.com/apache/gluten/pull/13146
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [MINOR][CORE] Treat linkage failures as class-not-present in isClassPresent [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13146:
URL: https://github.com/apache/gluten/pull/13146#issuecomment-5844633250
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [MINOR][CORE] Treat linkage failures as class-not-present in isClassPresent [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13146:
URL: https://github.com/apache/gluten/pull/13146#issuecomment-5852094707
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [MINOR][CORE] Treat linkage failures as class-not-present in isClassPresent [gluten]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #13146:
URL: https://github.com/apache/gluten/pull/13146#discussion_r4111169630
##########
gluten-core/src/test/scala/org/apache/spark/util/SparkReflectionUtilSuite.scala:
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.
+ */
+package org.apache.spark.util
+
+import org.scalatest.funsuite.AnyFunSuite
+
+class SparkReflectionUtilSuite extends AnyFunSuite {
+
+ test("isClassPresent returns true for a loadable class") {
+ assert(SparkReflectionUtil.isClassPresent(classOf[String].getName))
+ }
Review Comment:
Done in 05d7d16bd. Added a case asserting an absent class name reads false, covering the `ClassNotFoundException` path the description referred to.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [MINOR][CORE] Treat linkage failures as class-not-present in isClassPresent [gluten]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #13146:
URL: https://github.com/apache/gluten/pull/13146#discussion_r4130893039
##########
gluten-core/src/main/scala/org/apache/spark/util/SparkReflectionUtil.scala:
##########
@@ -35,6 +37,13 @@ object SparkReflectionUtil {
} catch {
case _: ClassNotFoundException =>
false
+ case e @ (_: NoClassDefFoundError | _: ExceptionInInitializerError) =>
+ // Present but unusable (a version-skewed optional dependency missing a supertype,
+ // or a failing static initializer): treat as not present, but log the cause so the
+ // skew is diagnosable. Broader linkage errors (VerifyError, UnsupportedClassVersionError,
+ // ...) are left to propagate, since they signal a genuinely broken build.
+ logWarning(s"Class $className is present but could not be linked; treating it as absent", e)
Review Comment:
These broader linkage errors are left to propagate on purpose. `UnsupportedClassVersionError` (a dependency built for a newer JVM) and `VerifyError` / `NoSuchMethodError` (bytecode or API skew) signal a genuinely broken deployment rather than a merely-absent optional dependency, and I would rather surface that than silently disable a backend. Spark's own `Utils.classIsLoadable` draws the same line: it is `Try { classForName(...) }.isSuccess`, and `Try` catches only `NonFatal`, which excludes `LinkageError`, so a linkage failure propagates there too and only `ClassNotFoundException` reads as not-loadable. I catch `NoClassDefFoundError` and `ExceptionInInitializerError` specifically because those are the normal signals of an absent or half-present optional dependency: a missing supertype, or a failing static initializer once the probe initializes the class.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [MINOR][CORE] Treat linkage failures as class-not-present in isClassPresent [gluten]
Posted by "jackylee-ch (via GitHub)" <gi...@apache.org>.
jackylee-ch commented on code in PR #13146:
URL: https://github.com/apache/gluten/pull/13146#discussion_r4142335799
##########
gluten-core/src/test/scala/org/apache/spark/util/SparkReflectionUtilSuite.scala:
##########
@@ -0,0 +1,41 @@
+/*
+ * 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.
+ */
+package org.apache.spark.util
+
+import org.scalatest.funsuite.AnyFunSuite
+
+class SparkReflectionUtilSuite extends AnyFunSuite {
Review Comment:
can we remove the tests?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [MINOR][CORE] Treat linkage failures as class-not-present in isClassPresent [gluten]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13146:
URL: https://github.com/apache/gluten/pull/13146#discussion_r4113880585
##########
gluten-core/src/main/scala/org/apache/spark/util/SparkReflectionUtil.scala:
##########
@@ -35,6 +37,13 @@ object SparkReflectionUtil {
} catch {
case _: ClassNotFoundException =>
false
+ case e @ (_: NoClassDefFoundError | _: ExceptionInInitializerError) =>
+ // Present but unusable (a version-skewed optional dependency missing a supertype,
+ // or a failing static initializer): treat as not present, but log the cause so the
+ // skew is diagnosable. Broader linkage errors (VerifyError, UnsupportedClassVersionError,
+ // ...) are left to propagate, since they signal a genuinely broken build.
+ logWarning(s"Class $className is present but could not be linked; treating it as absent", e)
Review Comment:
This still lets other `LinkageError` subclasses escape, despite the probe's contract and the PR description treating linkage failures as class-not-present. For example, a connector compiled for a newer JVM can throw `UnsupportedClassVersionError`, and verifier/API mismatches can throw `VerifyError` or `NoSuchMethodError`; these optional dependencies will still crash component discovery instead of returning `false`. Catch `LinkageError` broadly here so every class that cannot be linked is handled consistently.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [MINOR][CORE] Treat linkage failures as class-not-present in isClassPresent [gluten]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13146:
URL: https://github.com/apache/gluten/pull/13146#discussion_r4110740999
##########
gluten-core/src/test/scala/org/apache/spark/util/SparkReflectionUtilSuite.scala:
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.
+ */
+package org.apache.spark.util
+
+import org.scalatest.funsuite.AnyFunSuite
+
+class SparkReflectionUtilSuite extends AnyFunSuite {
+
+ test("isClassPresent returns true for a loadable class") {
+ assert(SparkReflectionUtil.isClassPresent(classOf[String].getName))
+ }
Review Comment:
The suite does not exercise the `ClassNotFoundException` path for an absent class, although the PR description lists that case as covered. Add an assertion using a definitely missing class name so this probe's existing optional-dependency behavior is also regression-tested.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [MINOR][CORE] Treat linkage failures as class-not-present in isClassPresent [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13146:
URL: https://github.com/apache/gluten/pull/13146#issuecomment-5910061742
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [MINOR][CORE] Treat linkage failures as class-not-present in isClassPresent [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13146:
URL: https://github.com/apache/gluten/pull/13146#issuecomment-5845690569
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [MINOR][CORE] Treat linkage failures as class-not-present in isClassPresent [gluten]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #13146:
URL: https://github.com/apache/gluten/pull/13146#discussion_r4143955256
##########
gluten-core/src/test/scala/org/apache/spark/util/SparkReflectionUtilSuite.scala:
##########
@@ -0,0 +1,41 @@
+/*
+ * 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.
+ */
+package org.apache.spark.util
+
+import org.scalatest.funsuite.AnyFunSuite
+
+class SparkReflectionUtilSuite extends AnyFunSuite {
Review Comment:
Removed in `640127d69` (the suite and its `StaticInitThrower` helper). Agreed, the linkage-catch change is small enough not to need a dedicated test.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [MINOR][CORE] Treat linkage failures as class-not-present in isClassPresent [gluten]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #13146:
URL: https://github.com/apache/gluten/pull/13146#issuecomment-6137324990
Thank you @jackylee-ch
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org