-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-28708][SQL] IsolatedClientLoader will not load hive classes from application jars on JDK9+ #25429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Test build #109029 has finished for PR 25429 at commit
|
| if (SystemUtils.isJavaVersionAtLeast(JavaVersion.JAVA_9)) { | ||
| val jars: Array[URL] = if (SystemUtils.isJavaVersionAtLeast(JavaVersion.JAVA_9)) { | ||
| // Do nothing. The system classloader is no longer a URLClassLoader in Java 9, | ||
| // so it won't match the case in allJars above. It no longer exposes URLs of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe remove the word 'above' in the comment.
Hm, so this resolves the problem? That's great. But I wonder why. In Java 9+, won't allJars already return nothing? I remember that was why I made this change to avoid the check for no JARs. But does it pick something up in some cases? OK makes some sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for removing above from allJars above.
|
@wangyum . Did I miss something you did? I downloaded and rebased to the master. The following is the error, |
|
cc @dbtsai |
|
Interesting, that's a different error. I wonder if we could see the stack trace of the cause, whether it comes from Spark or Hive. I had the impression that these error were ultimately from the Hive code, when I looked into it, but maybe not in this case. |
|
@dongjoon-hyun Did you upgraded built-in Hive to 2.3.6-SNAPSHOT?
|
|
Thank you, @wangyum . Could you put that into the PR description? |
|
Ur, by the way, you are pointing your repository. |
|
Hmm. Wait a second. In this case, shall we proceed in this way, @wangyum and @srowen ?
|
|
Yeah fine to merge this as it gets closer to resolving the issue, even though it won't fully resolve it. |
|
Test build #109049 has finished for PR 25429 at commit
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What changes were proposed in this pull request?
We have 8 test cases in
HiveSparkSubmitSuitestill fail withjava.lang.ClassNotFoundExceptionwhen running on JDK9+:Note that this pr fixes
java.lang.ClassNotFoundException, but the test will fail again with a different reason, the Hive-sidejava.lang.ClassCastExceptionwhich will be resolved in the official Hive 2.3.6 release.How was this patch tested?
manual tests: