[WIP][SPARK-26896] JDK 11 module adjustments for running tests #23804
Conversation
This adds a JDK11 profile, just intended for running tests. It adds extra runtime arguments to the test runners to get around new module limitations. This includes: 1) removal of jaxb from java itself (used in pmml export in mllib) 2) Some reflective access which results in failures, eg. Unable to make field jdk.internal.ref.PhantomCleanable jdk.internal.ref.PhantomCleanable.prev accessible: module java.base does not "opens jdk.internal.ref" to unnamed module 3) Some reflective access which results in warnings.
|
I have NOT fully tested this on jdk11. This is based on a similar patch I'm doing on a fork. I am just sharing it to help anybody else along / start discussion. |
|
Test build #102404 has finished for PR 23804 at commit
|
| @@ -2117,7 +2119,7 @@ | |||
| <include>**/*Suite.java</include> | |||
| </includes> | |||
| <reportsDirectory>${project.build.directory}/surefire-reports</reportsDirectory> | |||
| <argLine>-ea -Xmx4g -Xss4m -XX:ReservedCodeCacheSize=${CodeCacheSize}</argLine> | |||
| <argLine>-ea -Xmx3g -Xss4m -XX:ReservedCodeCacheSize=${CodeCacheSize} ${jdk11.module.args}</argLine> | |||
There was a problem hiding this comment.
Hi, @squito . Do we need to reduce this back to 3g for JDK11?
There was a problem hiding this comment.
oh no, sorry that was not supposed to be part of this change, sorry.
|
I'm unclear why these would only be applicable to test scope? I think we can't work around this by adding a bunch of command line args. Users can't be expected to do this. But I think you're saying it only affects tests. What was the source of the failure? there may be other answers. |
Yeah, all good questions -- honestly I don't know the answers either, I am not sure this is the right final change, but I thought it might be useful to share as others will hit the same issues. Maybe it is just fine to just add jaxb, even though it'll already be present in jdk8? I'm not sure. The Regardless, a change like this might be a step forward to get more people building and testing with jdk11? |
|
It's probably better in the end to have an explicit JAXB dependency even in Java 8. But IIRC the tests were passing for Java 11, or at least, I don't recall any JAXB issues. What fails if you don't add the That is ... I thought this was all already 99% working :) what are you seeing? |
|
I thought you only ran tests on core? I think all the issues I ran into were in other packages. Sorry my notes are a bit scattered -- most of this is from mllib stuff. Some tests failed with a stack trace pointing to a The bigger issue I ran into, actually, is SPARK-26839, which caused problems with unit tests in the hive module. I haven't posted the changes I've made yet, because I know what I've done is different in my fork. But if you've run hive tests successfully with jdk11 -- maybe that means I'm doing something else wrong in my setup? |
|
I see, could be, yeah. I don't see where we try to touch PhantomCleanable or prev though; do you have any more context on the failure? ideally we'd work around it or stop using some hack if isn't buying much. |
|
yeah I didn't understand it either. The stack trace really didn't make sense to me, but I found putting in the (sorry this has only partial attention from me at the moment -- like I said, just wanted to post for discussion as I'm sure others are experimenting with jdk11 ...) |
|
Oh I see it now: I can investigate too. |
|
I got the stack trace: The trick is to disable the JVM optimization that removes stack traces, with OK, it's because |
|
OK, this fixes this particular error: #23866 The JAXB thing relates to JPMML. I think my first attempt to fix that will be update JPMML, which seems to need an update for Java 9 anyway. I think it's specifying the Sun implementation somewhere: I hope we can fix these things in this way rather than profiles or command line args. |
|
great, thanks for following up on this, Sean. As it sounds like you are doing a deeper dive here, and we can probably avoid the profile altogether, I will close this for now. And thanks for the tip on |
This adds a JDK11 profile, just intended for running tests. It adds
extra runtime arguments to the test runners to get around new module
limitations. This includes:
removal of jaxb from java itself (used in pmml export in mllib)
Some reflective access which results in failures, eg.
Unable to make field jdk.internal.ref.PhantomCleanable
jdk.internal.ref.PhantomCleanable.prev accessible: module java.base does
not "opens jdk.internal.ref" to unnamed module