[SPARK-28877][PYSPARK][test-hadoop3.2][test-java11] Make jaxb-runtime compile-time dependency #25587

dongjoon-hyun · 2019-08-26T23:46:33Z

What changes were proposed in this pull request?

mllib has jaxb-runtime-2.3.2 as a runtime dependency. This PR makes it as a compile-time dependency. This doesn't change our dependency manifest and LICENSEs. Instead, this will add the following three jars into our pre-built artifacts.

jaxb-runtime-2.3.2.jar
jakarta.xml.bind-api-2.3.2.jar
istack-commons-runtime-3.0.8.jar

Why are the changes needed?

We need to use the followings.

JDK8: com.sun.xml.internal.bind.v2.ContextFactory
JDK11: com.sun.xml.bind.v2.ContextFactory

com.sun.xml.bind.v2.ContextFactory is inside jaxb-runtime-2.3.2.

$ javap -cp jaxb-runtime-2.3.2.jar com.sun.xml.bind.v2.ContextFactory
Compiled from "ContextFactory.java"
public class com.sun.xml.bind.v2.ContextFactory {
  public static final java.lang.String USE_JAXB_PROPERTIES;
  public com.sun.xml.bind.v2.ContextFactory();
  public static javax.xml.bind.JAXBContext createContext(java.lang.Class[], java.util.Map<java.lang.String, java.lang.Object>) throws javax.xml.bind.JAXBException;
  public static com.sun.xml.bind.api.JAXBRIContext createContext(java.lang.Class[], java.util.Collection<com.sun.xml.bind.api.TypeReference>, java.util.Map<java.lang.Class, java.lang.Class>, java.lang.String, boolean, com.sun.xml.bind.v2.model.annotation.RuntimeAnnotationReader, boolean, boolean, boolean) throws javax.xml.bind.JAXBException;
  public static com.sun.xml.bind.api.JAXBRIContext createContext(java.lang.Class[], java.util.Collection<com.sun.xml.bind.api.TypeReference>, java.util.Map<java.lang.Class, java.lang.Class>, java.lang.String, boolean, com.sun.xml.bind.v2.model.annotation.RuntimeAnnotationReader, boolean, boolean, boolean, boolean) throws javax.xml.bind.JAXBException;
  public static javax.xml.bind.JAXBContext createContext(java.lang.String, java.lang.ClassLoader, java.util.Map<java.lang.String, java.lang.Object>) throws javax.xml.bind.JAXBException;
}

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass the Jenkins with test-java11.

For manual testing, do the following with JDK11.

$ java -version
openjdk version "11.0.3" 2019-04-16
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.3+7)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.3+7, mixed mode)

$ build/sbt -Pyarn -Phadoop-3.2 -Phadoop-cloud -Phive -Phive-thriftserver -Psparkr test:package

$ python/run-tests.py --python-executables python --modules pyspark-ml
...
Finished test(python): pyspark.ml.recommendation (65s)
Tests passed in 174 seconds

HyukjinKwon · 2019-08-27T00:49:45Z

This is nice!!

HyukjinKwon

LGTM if tests pass

HyukjinKwon · 2019-08-27T01:05:14Z

retest this please

SparkQA · 2019-08-27T01:35:29Z

Test build #109762 has finished for PR 25587 at commit 278de83.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2019-08-27T01:40:14Z

retest this please

dongjoon-hyun · 2019-08-27T01:55:23Z

Thank you for review and approval and retriggering, @HyukjinKwon .
Thank you, @shaneknapp . It's nice that I can use test-java11. :)

srowen

Yeah I think that's probably the right answer here. We need to bundle some (possibly many) JAXB implementations for JDK 11 and it shouldn't hurt JDK 8. We might need to slip in an extra entry in one of the license files, otherwise can't think of anything else.

EDIT: No it's already there. It's already a runtime dependency. Hm, wait, then why isn't it present for the tests already? we should already be bundling this. I somehow think this wasn't the issue, it was already fine but the issue is in how Pyspark tests form the classpath from the assembly jars/ dir. Let's take a look.

dongjoon-hyun · 2019-08-27T02:31:30Z

Thank you for review, @srowen . However, our assembly jars doesn't have it without this PR, does it? So, this is an issue in not only test class path, but also pre-built artifacts. Without this PR, the pre-build artifacts will raise the same exception on JDK11. How do you think about that?

BEFORE THIS PR

$ ls assembly/target/scala-2.12/jars/*runtime*
assembly/target/scala-2.12/jars/antlr-runtime-3.5.2.jar
assembly/target/scala-2.12/jars/antlr4-runtime-4.7.1.jar

srowen · 2019-08-27T02:34:59Z

Yeah if it causes them to be in the assembly, then it's the right fix. It sort of should already be there, but the rules for transitive dependency resolution are complex. Making it compile-scope isn't strictly correct, but if it causes the resolution to work out, seems fine, and does little harm (we won't be writing code directly depending on it).

dongjoon-hyun · 2019-08-27T02:40:31Z

Ah, transitive dependency. Since this passes the dependency manifest test, I thought we are safe. But, I didn't check the others. I'll check the full diff and update the PR description. Thanks.

srowen · 2019-08-27T02:46:38Z

I bet you will find it does not change the transitive dependencies as reported by Maven or SBT - it's already a runtime dependency. This was the reason to be suspicious that it's not the fix. But the fact that it's not in the assembly is the issue, and the reasons are arcane (maybe even a more general problem) about how the assembly plugin treats transitive dependencies of various scopes. We need not get into that here; if it resolves the issue it's OK as a patch.

dongjoon-hyun · 2019-08-27T02:46:55Z

I updated the PR description. jakarta.xml.bind-api-2.3.2.jar and istack-commons-runtime-3.0.8.jar are required and added to our pre-build artifacts. For transitive dependency, what I worried was our artifact size increment.

dongjoon-hyun · 2019-08-27T02:58:15Z

I'll take a look once more.

SparkQA · 2019-08-27T03:11:56Z

Test build #109764 has finished for PR 25587 at commit 278de83.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-08-27T03:41:14Z

Test build #109768 has finished for PR 25587 at commit 278de83.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2019-08-27T04:29:36Z

Technically, the UT failures here are irrelevant to this PR. Especially, last two failures seems to be due to the new test suite flakiness. (#25567 (comment))

SparkQA · 2019-08-27T06:20:18Z

Test build #4842 has finished for PR 25587 at commit 278de83.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2019-08-27T06:21:39Z

This PR passed the Jenkins with JDK8 building and JDK11 testing.

========================================================================
Building Spark
========================================================================
[info] Building Spark using SBT with these arguments:  -Phadoop-3.2 -Pkubernetes -Phive-thriftserver -Phadoop-cloud -Pkinesis-asl -Pyarn -Pspark-ganglia-lgpl -Phive -Pmesos test:package streaming-kinesis-asl-assembly/assembly
Using /usr/java/jdk1.8.0_191 as default JAVA_HOME.
...
[info] Done packaging.
[success] Total time: 21 s, completed Aug 26, 2019 8:54:41 PM

========================================================================
Running Spark unit tests
========================================================================
[info] Running Spark tests using SBT with these arguments:  -Phadoop-3.2 -Pkubernetes -Phadoop-cloud -Phive -Phive-thriftserver -Pyarn -Pspark-ganglia-lgpl -Pkinesis-asl -Pmesos -Dtest.exclude.tags=org.apache.spark.tags.ExtendedHiveTest,org.apache.spark.tags.ExtendedYarnTest -Djava.version=11 test
Using /usr/java/jdk-11.0.1 as default JAVA_HOME.
...
[success] Total time: 5438 s, completed Aug 26, 2019, 10:30:04 PM

========================================================================
Running PySpark tests
========================================================================
Running PySpark tests. Output is in /home/jenkins/workspace/NewSparkPullRequestBuilder@2/python/unit-tests.log
Will test against the following Python executables: ['python2.7', 'python3.6', 'pypy']
...
Tests passed in 1222 seconds

========================================================================
Running SparkR tests
========================================================================
WARNING: An illegal reflective access operation has occurred
...
DONE ===========================================================================

dongjoon-hyun · 2019-08-27T06:22:36Z

After this PR, we can use test-java11 freely. And, we can move forward to optimize our build.

Merged to master. Thank you all.

HyukjinKwon · 2019-08-27T06:32:12Z

This is nice!

liangrui1988 · 2019-08-30T10:22:00Z

666

[SPARK-28877][PYSPARK] Make jaxb-runtime compile-time dependency

278de83

HyukjinKwon approved these changes Aug 27, 2019

View reviewed changes

HyukjinKwon changed the title ~~[SPARK-28877][PYSPARK][test-hadoop3.2][test-java11] Make jaxb-runtime compile-time dependency~~ [SPARK-28877][PYSPARK] Make jaxb-runtime compile-time dependency Aug 27, 2019

HyukjinKwon changed the title ~~[SPARK-28877][PYSPARK] Make jaxb-runtime compile-time dependency~~ [SPARK-28877][PYSPARK][test-hadoop3.2][test-java11] Make jaxb-runtime compile-time dependency Aug 27, 2019

srowen reviewed Aug 27, 2019

View reviewed changes

dongjoon-hyun mentioned this pull request Aug 27, 2019

[SPARK-28527][SQL][TEST] Re-run all the tests in SQLQueryTestSuite via Thrift Server #25567

Closed

dongjoon-hyun added BUILD PYSPARK labels Aug 27, 2019

dongjoon-hyun closed this in 7701d29 Aug 27, 2019

dongjoon-hyun deleted the SPARK-28877 branch August 27, 2019 06:23

[SPARK-28877][PYSPARK][test-hadoop3.2][test-java11] Make jaxb-runtime compile-time dependency #25587

[SPARK-28877][PYSPARK][test-hadoop3.2][test-java11] Make jaxb-runtime compile-time dependency #25587

Uh oh!

Conversation

dongjoon-hyun commented Aug 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

HyukjinKwon commented Aug 27, 2019

Uh oh!

HyukjinKwon left a comment

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon commented Aug 27, 2019

Uh oh!

SparkQA commented Aug 27, 2019

Uh oh!

HyukjinKwon commented Aug 27, 2019

Uh oh!

dongjoon-hyun commented Aug 27, 2019

Uh oh!

srowen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Aug 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

srowen commented Aug 27, 2019

Uh oh!

dongjoon-hyun commented Aug 27, 2019

Uh oh!

srowen commented Aug 27, 2019

Uh oh!

dongjoon-hyun commented Aug 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dongjoon-hyun commented Aug 27, 2019

Uh oh!

SparkQA commented Aug 27, 2019

Uh oh!

SparkQA commented Aug 27, 2019

Uh oh!

dongjoon-hyun commented Aug 27, 2019

Uh oh!

SparkQA commented Aug 27, 2019

Uh oh!

dongjoon-hyun commented Aug 27, 2019

Uh oh!

dongjoon-hyun commented Aug 27, 2019

Uh oh!

HyukjinKwon commented Aug 27, 2019

Uh oh!

liangrui1988 commented Aug 30, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dongjoon-hyun commented Aug 26, 2019 •

edited

Loading

srowen left a comment •

edited

Loading

dongjoon-hyun commented Aug 27, 2019 •

edited

Loading

dongjoon-hyun commented Aug 27, 2019 •

edited

Loading