Skip to content

Conversation

@LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Jul 4, 2023

What changes were proposed in this pull request?

The main change of this pr as follows:

  1. rename TestHelloV2.jar and TestHelloV3.jar added in [SPARK-44246][CONNECT][FOLLOW-UP] Miscellaneous cleanups for Spark Connect Jar/Classfile Isolation #41789 to TestHelloV2_2.12.jar and TestHelloV3_2.12.jar
  2. Add corresponding TestHelloV2_2.13.jar and TestHelloV3_2.13.jar which compiled with Scala 2.13
  3. Make ClassLoaderIsolationSuite use the correct jar in testing

Why are the changes needed?

Make ClassLoaderIsolationSuite test pass with Scala 2.13.

The Scala 2.13 daily test failed after #41789

[info] - Executor classloader isolation with JobArtifactSet *** FAILED *** (83 milliseconds)
[info]   org.apache.spark.SparkException: Job aborted due to stage failure: Task 0 in stage 0.0 failed 1 times, most recent failure: Lost task 0.0 in stage 0.0 (TID 0) (localhost executor driver): java.lang.NoClassDefFoundError: scala/Serializable
[info] 	at java.lang.ClassLoader.defineClass1(Native Method)
[info] 	at java.lang.ClassLoader.defineClass(ClassLoader.java:756)
[info] 	at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
[info] 	at java.net.URLClassLoader.defineClass(URLClassLoader.java:473)
[info] 	at java.net.URLClassLoader.access$100(URLClassLoader.java:74)
[info] 	at java.net.URLClassLoader$1.run(URLClassLoader.java:369)
[info] 	at java.net.URLClassLoader$1.run(URLClassLoader.java:363)
[info] 	at java.security.AccessController.doPrivileged(Native Method)
[info] 	at java.net.URLClassLoader.findClass(URLClassLoader.java:362)
[info] 	at java.lang.ClassLoader.loadClass(ClassLoader.java:418)
[info] 	at java.lang.ClassLoader.loadClass(ClassLoader.java:351)
[info] 	at java.lang.Class.forName0(Native Method)
[info] 	at java.lang.Class.forName(Class.java:348)
[info] 	at org.apache.spark.util.SparkClassUtils.classForName(SparkClassUtils.scala:35)
[info] 	at org.apache.spark.util.SparkClassUtils.classForName$(SparkClassUtils.scala:30)
[info] 	at org.apache.spark.util.Utils$.classForName(Utils.scala:94)
[info] 	at org.apache.spark.executor.ClassLoaderIsolationSuite.$anonfun$new$3(ClassLoaderIsolationSuite.scala:53)
[info] 	at scala.runtime.java8.JFunction1$mcVI$sp.apply(JFunction1$mcVI$sp.scala:18)
[info] 	at scala.collection.IterableOnceOps.foreach(IterableOnce.scala:576)
[info] 	at scala.collection.IterableOnceOps.foreach$(IterableOnce.scala:574)
[info] 	at org.apache.spark.InterruptibleIterator.foreach(InterruptibleIterator.scala:28)
[info] 	at org.apache.spark.rdd.RDD.$anonfun$foreach$2(RDD.scala:1028)
[info] 	at org.apache.spark.rdd.RDD.$anonfun$foreach$2$adapted(RDD.scala:1028)
[info] 	at org.apache.spark.SparkContext.$anonfun$runJob$5(SparkContext.scala:2406)
[info] 	at org.apache.spark.scheduler.ResultTask.runTask(ResultTask.scala:93)
[info] 	at org.apache.spark.TaskContext.runTaskWithListeners(TaskContext.scala:161)
[info] 	at org.apache.spark.scheduler.Task.run(Task.scala:141)
[info] 	at org.apache.spark.executor.Executor$TaskRunner.$anonfun$run$4(Executor.scala:593)
[info] 	at org.apache.spark.util.Utils$.tryWithSafeFinally(Utils.scala:1478)
[info] 	at org.apache.spark.executor.Executor$TaskRunner.run(Executor.scala:596)
[info] 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
[info] 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
[info] 	at java.lang.Thread.run(Thread.java:750)
[info] Caused by: java.lang.ClassNotFoundException: scala.Serializable
[info] 	at java.net.URLClassLoader.findClass(URLClassLoader.java:387)
[info] 	at java.lang.ClassLoader.loadClass(ClassLoader.java:418)
[info] 	at java.lang.ClassLoader.loadClass(ClassLoader.java:351)
[info] 	... 33 more

Does this PR introduce any user-facing change?

No

How was this patch tested?

  • Pass GitHub Actions
  • Manual check:

Scala 2.12

build/sbt "core/testOnly *ClassLoaderIsolationSuite"
[info] ClassLoaderIsolationSuite:
[info] - Executor classloader isolation with JobArtifactSet (1 second, 394 milliseconds)
[info] Run completed in 2 seconds, 437 milliseconds.
[info] Total number of tests run: 1
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 1, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.

Scala 2.13

dev/change-scala-version.sh 2.13
build/sbt "core/testOnly *ClassLoaderIsolationSuite" -Pscala-2.13
[info] ClassLoaderIsolationSuite:
[info] - Executor classloader isolation with JobArtifactSet (1 second, 355 milliseconds)
[info] Run completed in 2 seconds, 264 milliseconds.
[info] Total number of tests run: 1
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 1, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.

@LuciferYang
Copy link
Contributor Author

LuciferYang commented Jul 4, 2023

cc @vicennial @HyukjinKwon FYI

Copy link
Contributor

@vicennial vicennial left a comment

Choose a reason for hiding this comment

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

Thank you for the fix @LuciferYang!

I'll wait for this PR to merge and rebase https://github.com/apache/spark/pull/41844/files on top of this (also uses a copy of these JARs).

@LuciferYang
Copy link
Contributor Author

Thank you for the fix @LuciferYang!

I'll wait for this PR to merge and rebase https://github.com/apache/spark/pull/41844/files on top of this (also uses a copy of these JARs).

OK, I will merge this as soon as possible if GA passed

@yaooqinn
Copy link
Member

yaooqinn commented Jul 4, 2023

Not related to this PR, including binary files in source seem to conflict with the Apache release policy.

@LuciferYang
Copy link
Contributor Author

Merged to master, thanks @HyukjinKwon @MaxGekk @yaooqinn @vicennial

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants