Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 42 additions & 2 deletions .github/workflows/tpcds-reusable.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,23 @@ on:
required: false
type: string
default: ''
assert-no-classcastexception:
description: >-
Whether to fail the job if the TPC-DS run logs any ClassCastException. Spark retries
absorb task-level deserialization failures, so queries can report PASS while throwing
hundreds of exceptions; this asserts on the log instead.
required: false
type: string
default: 'false'
jar-on-system-classpath:
description: >-
Whether to also copy the Auron jar into $SPARK_HOME/jars. When true the jar is
loaded by the application class loader; when false it reaches the JVM only through
spark-submit --jars, i.e. Spark's MutableURLClassLoader. Some class-loading defects
only reproduce in the latter configuration.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not sure this fully guarantees that Auron is loaded only through  --jars . The  auron-it  shaded JAR depends on the Auron uber JAR, so it looks like it may contain the same Auron classes itself. If that’s the case, removing the copy from  $SPARK_HOME/jars  may not reliably reproduce the original class-loader setup.

Would it make sense to exclude Auron from the integration-test fat JAR, or add a small runtime check that prints/asserts the actual class loader and code source?

required: false
type: string
default: 'true'
queries:
description: 'Optional list of queries to run'
required: false
Expand Down Expand Up @@ -257,12 +274,18 @@ jobs:
path: dev/tpcds_1g

- name: Install Auron JAR
env:
JAR_ON_SYSTEM_CLASSPATH: ${{ inputs.jar-on-system-classpath }}
run: |
ls -la
jar=$(ls -1 auron-${{ inputs.sparkver }}_${{ inputs.scalaver }}*.jar | head -n1)
[ -n "$jar" ] || { echo "No jar matched: auron-${{ inputs.sparkver }}_${{ inputs.scalaver }}*.jar"; exit 1; }
echo "AURON_SPARK_JAR=$jar" >> "$GITHUB_ENV"
cp "$jar" spark-bin-${{ inputs.sparkver }}_${{ inputs.scalaver }}/jars/
if [ "$JAR_ON_SYSTEM_CLASSPATH" = "false" ]; then
echo "Auron reaches the JVM only through spark-submit --jars (MutableURLClassLoader)"
else
cp "$jar" spark-bin-${{ inputs.sparkver }}_${{ inputs.scalaver }}/jars/
fi

- name: Setup Java and Maven cache
uses: actions/setup-java@v5
Expand Down Expand Up @@ -380,13 +403,30 @@ jobs:
SPARK_HOME: spark-bin-${{ inputs.sparkver }}_${{ inputs.scalaver }}
run: |
ls -la
set -o pipefail
dev/auron-it/run-it.sh \
${{ inputs.extrasparkconf }} \
--type tpcds \
--data-location dev/tpcds_1g \
--query-filter ${{ matrix.query }} \
--result-check \
--plan-check
--plan-check 2>&1 | tee tpcds-run-${{ matrix.query }}.log

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a positive check that the runtime bloom-filter ScalarSubquery was actually injected? Right now the job only checks that no ClassCastException occurred.

Also I think  --plan-check  is skipped for Spark 4.1, since PlanStabilityChecker currently supports only Spark 3.5. Without a positive assertion, the job could pass simply because the optimizer stopped producing the plan that triggers this path ..


# Task-level deserialization failures are absorbed by Spark's task retries, so the
# queries can still report PASS while throwing hundreds of exceptions. Assert on the
# log directly, otherwise a regression of AURON #2386 goes unnoticed.
- name: Assert no deserialization ClassCastException
if: ${{ inputs.assert-no-classcastexception == 'true' }}
env:
QUERY_LOG: tpcds-run-${{ matrix.query }}.log
run: |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this run only the query or small query set known to reproduce the issue and match the specific deserialization failure signature? Running all 99 TPC-DS queries adds substantial CI cost, while grepping every ClassCastException  can attribute unrelated failures to expression deserialization. I think something more focused might be better here?

count=$(grep -c 'ClassCastException' "$QUERY_LOG" || true)
if [ "$count" -gt 0 ]; then
echo "::error::$count ClassCastException(s) during expression deserialization"
grep -m5 'ClassCastException' "$QUERY_LOG" || true
exit 1
fi
echo "No ClassCastException found."

- name: Upload RSS log
if: ${{ failure() && (inputs.celebornver != '' || inputs.unifflever != '') }}
Expand Down
20 changes: 20 additions & 0 deletions .github/workflows/tpcds.yml
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,23 @@ jobs:
scalaver: '2.13'
hadoop-profile: 'hadoop3'
sparktests: 'true'

test-spark-41-jdk17-scala-2-13-with-bloomfilter-optimizer-enabled:
name: Test spark-4.1 JDK17 Scala-2.13 with bloomFilter optimizer enabled
uses: ./.github/workflows/tpcds-reusable.yml
with:
sparkver: spark-4.1
javaver: '17'
scalaver: '2.13'
hadoop-profile: 'hadoop3'
sparktests: 'true'
# Supply Auron only through spark-submit --jars, so it is defined by Spark's
# MutableURLClassLoader rather than the application class loader. The runtime
# bloom-filter ScalarSubquery deserialization defect (AURON #2386) cannot reproduce
# when the jar is also on $SPARK_HOME/jars.
jar-on-system-classpath: 'false'
assert-no-classcastexception: 'true'
extrasparkconf: >-
--conf spark.sql.optimizer.runtime.bloomFilter.enabled=true
--conf spark.sql.optimizer.runtime.bloomFilter.applicationSideScanSizeThreshold=1B
--conf spark.sql.autoBroadcastJoinThreshold=-1
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ package org.apache.spark.sql.auron

import java.io.ByteArrayInputStream
import java.io.ByteArrayOutputStream
import java.io.InputStream
import java.io.ObjectInputStream
import java.io.ObjectOutputStream
import java.io.ObjectStreamClass

import scala.collection.mutable
import scala.jdk.CollectionConverters._
Expand Down Expand Up @@ -1505,24 +1507,63 @@ object NativeConverters extends Logging {
}
}

/**
* ObjectInputStream that resolves classes against an explicit class loader.
*
* The default ObjectInputStream.resolveClass resolves each class through
* VM.latestUserDefinedLoader(), which selects a loader from the live call stack rather than the
* context class loader. During a nested read the most recent user-defined frame is often a
* Spark or Scala class, whose loader cannot see Auron classes when Auron is supplied through
* spark.jars and therefore loaded by MutableURLClassLoader. The expression graph then resolves
* only partially and an un-readResolve'd DefaultSerializationProxy is assigned into
* RDD.dependencies_, raising a ClassCastException. Pinning the loader keeps resolution
* independent of the call stack. Spark's own JavaDeserializationStream does the same.
*/
private class AuronObjectInputStream(in: InputStream, loader: ClassLoader)
extends ObjectInputStream(in) {

// scalastyle:off classforname
private def load(name: String, cl: ClassLoader): Class[_] = Class.forName(name, false, cl)
// scalastyle:on classforname

// resolveProxyClass is deliberately not overridden: the only non-deprecated way to obtain a
// proxy Class is Proxy.getProxyClass, and serialized expressions contain no dynamic proxies.
override def resolveClass(desc: ObjectStreamClass): Class[_] = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain more of your rationale here for not overriding resolveProxyClass?  IIUC, the deserializeExpression is shared by UDF, UDAF, and UDTF expression graphs, which may contain serializable dynamic proxies...leaving proxy resolution on ObjectInputStream ’s default path reintroduces the stack-dependent class-loader behavior this change is intended to avoid, so proxy interfaces available only through Spark’s MutableURLClassLoader may still fail to deserialize when supplied through  --jars, right?

val name = desc.getName
try {
load(name, loader)
} catch {
case _: ClassNotFoundException =>
// the loader that defined Auron always resolves Auron's own classes even when the
// context loader cannot, and its parent chain still covers Spark and Scala classes
try {
load(name, getClass.getClassLoader)
} catch {
case _: ClassNotFoundException => super.resolveClass(desc)
}
}
}
}

def deserializeExpression[E <: Expression, S <: Serializable](
serialized: Array[Byte]): (E with Serializable, S) = {
Utils.tryWithResource(new ByteArrayInputStream(serialized)) { bis =>
Utils.tryWithResource(new ObjectInputStream(bis)) { ois =>
def read(): (E with Serializable, S) = {
val expr = ois.readObject().asInstanceOf[E with Serializable]
val payload = ois.readObject().asInstanceOf[S with Serializable]
(expr, payload)
}
// Spark TaskMetrics#externalAccums is not thread-safe
val taskContext = TaskContext.get()
if (taskContext != null) {
taskContext.taskMetrics().synchronized {
Utils.tryWithResource(new AuronObjectInputStream(bis, Utils.getContextOrSparkClassLoader)) {
ois =>
def read(): (E with Serializable, S) = {
val expr = ois.readObject().asInstanceOf[E with Serializable]
val payload = ois.readObject().asInstanceOf[S with Serializable]
(expr, payload)
}
// Spark TaskMetrics#externalAccums is not thread-safe
val taskContext = TaskContext.get()
if (taskContext != null) {
taskContext.taskMetrics().synchronized {
read()
}
} else {
read()
}
} else {
read()
}
}
}
}
Expand Down
Loading