[SPARK-27397][Core] Take care of OpenJ9 JVM in Spark #24308

kiszk · 2019-04-06T07:41:10Z

What changes were proposed in this pull request?

This PR supports OpenJ9 in addition to IBM JDK and OpenJDK in Spark by handling System.getProperty("java.vendor") = "Eclipse OpenJ9".

In inferDefaultMemory() and getKrb5LoginModuleName(), this PR uses non IBM way.

$ ~/jdk-11.0.2+9_openj9-0.12.1/bin/jshell 
|  Welcome to JShell -- Version 11.0.2
|  For an introduction type: /help intro

jshell> System.out.println(System.getProperty("java.vendor"))
Eclipse OpenJ9

jshell> System.out.println(System.getProperty("java.vm.info"))
JRE 11 Linux amd64-64-Bit Compressed References 20190204_127 (JIT enabled, AOT enabled)
OpenJ9   - 90dd8cb40
OMR      - d2f4534b
JCL      - 289c70b6844 based on jdk-11.0.2+9

jshell> System.out.println(Class.forName("com.ibm.lang.management.OperatingSystemMXBean").getDeclaredMethod("getTotalPhysicalMemory"))
public abstract long com.ibm.lang.management.OperatingSystemMXBean.getTotalPhysicalMemory()

jshell> System.out.println(Class.forName("com.sun.management.OperatingSystemMXBean").getDeclaredMethod("getTotalPhysicalMemorySize"))
public abstract long com.sun.management.OperatingSystemMXBean.getTotalPhysicalMemorySize()

jshell> System.out.println(Class.forName("com.ibm.security.auth.module.Krb5LoginModule"))
|  Exception java.lang.ClassNotFoundException: com.ibm.security.auth.module.Krb5LoginModule
|        at Class.forNameImpl (Native Method)
|        at Class.forName (Class.java:339)
|        at (#1:1)

jshell> System.out.println(Class.forName("com.sun.security.auth.module.Krb5LoginModule"))
class com.sun.security.auth.module.Krb5LoginModule

How was this patch tested?

Existing test suites
Manual testing with OpenJ9.

Note

Beyond the scope of this PR, OpenJ9 on Mac has an issue with zstd-jni.
As @dongjoon-hyun found here, multiple tests that use zstd-jni fails with OpenJ9 on Mac. This is an issue in OpenJ9

core/src/main/scala/org/apache/spark/util/SizeEstimator.scala

SparkQA · 2019-04-06T11:48:57Z

Test build #104339 has finished for PR 24308 at commit 96bcfd4.

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

kiszk · 2019-04-06T17:29:01Z

Retest this please

dongjoon-hyun · 2019-04-06T18:27:53Z

Hi, @kiszk . Could you update How was this patch tested? section? Actually, Jenkins doesn't trigger OpenJ9 here with your PR. Maybe, you want to mention that Manual testing. Pass all tests with OpenJ9?

SparkQA · 2019-04-06T22:16:02Z

Test build #104342 has finished for PR 24308 at commit 96bcfd4.

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

srowen · 2019-04-06T23:09:55Z

core/src/main/scala/org/apache/spark/util/SizeEstimator.scala

-    // java.vm.info provides compressed ref info for IBM JDKs
-    if (System.getProperty("java.vendor").contains("IBM")) {
+    // java.vm.info provides compressed ref info for IBM and OpenJ9 JDKs
+    if (System.getProperty("java.vendor").contains("IBM") ||


Nit: do you want to retrieve the value of java.vendor once here and compare it twice? the overall change seems fine, though I don't know if it means these JDKs will always work. We only test against the Oracle/OpenJDK

SparkQA · 2019-04-06T23:21:42Z

Test build #104348 has finished for PR 24308 at commit 6265481.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2019-04-07T04:07:12Z

@kiszk . Although JDK8 doesn't have jshell, we can not test Spark on OpenJ9 JDK11, could you update the PR description with the information from Eclipse OpenJ9 VM (build openj9-0.12.1, JRE 1.8.0 Linux amd64-64-Bit Compressed References 20190205_213 (JIT enabled, AOT enabled)?

SparkQA · 2019-04-07T04:23:28Z

Test build #4696 has finished for PR 24308 at commit 6265481.

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

dongjoon-hyun

@kiszk . It seems to require additional installation steps to pass the UT.
During local verification, I'm facing the following UT failure due to Zstd JNI issue.
In the PR description, could you add a guide about how to pass the UT in OpenJ9?

Oracle JDK8

~/P/PR-24308:PR-24308$ export JAVA_HOME=/Library/Java/JavaVirtualMachines/jdk1.8.0_201.jdk/Contents/Home

~/P/PR-24308:PR-24308$ java -version
java version "1.8.0_201"
Java(TM) SE Runtime Environment (build 1.8.0_201-b09)
Java HotSpot(TM) 64-Bit Server VM (build 25.201-b09, mixed mode)

~/P/PR-24308:PR-24308$ build/sbt "core/testOnly *.EventLoggingListenerSuite"
Using /Library/Java/JavaVirtualMachines/jdk1.8.0_201.jdk/Contents/Home as default JAVA_HOME.
...
[info] All tests passed.
[info] Passed: Total 9, Failed 0, Errors 0, Passed 9
[success] Total time: 123 s, completed Apr 7, 2019 5:34:03 PM

OpenJ9 JDK8

~/P/PR-24308:PR-24308$ export JAVA_HOME=/Library/Java/JavaVirtualMachines/jdk8u202-b08/Contents/Home/

~/P/PR-24308:PR-24308$ java -version
openjdk version "1.8.0_202"
OpenJDK Runtime Environment (build 1.8.0_202-b08)
Eclipse OpenJ9 VM (build openj9-0.12.1, JRE 1.8.0 Mac OS X amd64-64-Bit 20190205_1 (JIT enabled, AOT enabled)
OpenJ9   - 90dd8cb40
OMR      - d2f4534b
JCL      - d002501a90 based on jdk8u202-b08)

~/P/PR-24308:PR-24308$ build/sbt "core/testOnly *.EventLoggingListenerSuite"
Using /Library/Java/JavaVirtualMachines/jdk8u202-b08/Contents/Home/ as default JAVA_HOME.
Note, this will be overridden by -java-home if it is set.
...
[info] EventLoggingListenerSuite:
[info] - Verify log file exist (394 milliseconds)
[info] - Basic event logging (1 second, 307 milliseconds)
[info] org.apache.spark.scheduler.EventLoggingListenerSuite *** ABORTED *** (2 seconds, 118 milliseconds)
[info]   java.lang.UnsatisfiedLinkError: zstd-jni (Not found in java.library.path)
[info] Unsupported OS/arch, cannot find /darwin/amd64/libzstd-jni.dylib or load zstd-jni from system libraries. Please try building from source the jar or providing libzstd-jni in you system.
[info]   at java.lang.ClassLoader.loadLibraryWithPath(ClassLoader.java:1434)
[info]   at java.lang.ClassLoader.loadLibraryWithClassLoader(ClassLoader.java:1404)
[info]   at java.lang.System.loadLibrary(System.java:540)
[info]   at com.github.luben.zstd.util.Native.load(Native.java:69)
[info]   at com.github.luben.zstd.ZstdOutputStream.<clinit>(ZstdOutputStream.java:17)
[info]   at org.apache.spark.io.ZStdCompressionCodec.compressedOutputStream(CompressionCodec.scala:197)
...
[info] *** 1 SUITE ABORTED ***
[error] Error: Total 1, Failed 0, Errors 1, Passed 0
[error] Error during tests:
[error] 	org.apache.spark.scheduler.EventLoggingListenerSuite
[error] (core/test:testOnly) sbt.TestsFailedException: Tests unsuccessful
[error] Total time: 156 s, completed Apr 7, 2019 5:38:22 PM

attilapiros · 2019-04-08T19:45:18Z

launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java

@@ -124,6 +124,9 @@ static JavaVendor getJavaVendor() {
    if (vendorString.contains("OpenJDK")) {
      return JavaVendor.OpenJDK;
    }
+    if (vendorString.contains("OpenJ9")) {


Do you know who uses this getJavaVendor method?
As it is package private so only classes from the same package could access this.
But in the entire Spark code there is no usage for it:

$ find . -type f -exec grep "getJavaVendor" \{} \; -print static JavaVendor getJavaVendor() { ./launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java

Could it be a 3rd party tool should use the package name: org.apache.spark.launcher?

Hm yeah it's unused as far as I can see. It shouldn't be used by third parties anyway. Delete it?

There is one more method near this one also seams to me unused: firstNonEmptyValue. Should I create a Minor PR or OK to remove within this PR?

Oh and there will be the enum as well: getJavaVendor seams to me the only place where used (and mima exclude).

Agree, you can delete the unused code here.
It looks like this was added to account for the fact that IBM JDKs don't have a PermGen, but Spark no longer tries to set this now that Java 8 is required and doesn't have a PermGen.

kiszk · 2019-04-09T06:11:09Z

@dongjoon-hyun Thank you. I will address them on middle or late this week since I am on a business trip this week.

kiszk · 2019-04-14T19:07:41Z

When I came back to home (I leave Mac at home), I realized that this link issue occurs only on Mac.

In addition, I realized that this is not an issue in Spark. May be in OpenJ9 or zstd. I am investigating this now.

kiszk · 2019-04-15T03:03:26Z

@dongjoon-hyun I created an issue on OpenJ9 to address a jni link issue.
Should we track this link issue in the separate PR? Or, should we wait for a new openj9 in this PR?

OpenJ9 will be updated in July.

SparkQA · 2019-04-15T06:50:05Z

Test build #104580 has finished for PR 24308 at commit 08eb67a.

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

dongjoon-hyun

Thank you, @kiszk .
Please mentioned the reported issue in the PR description.
For the rest, +1, LGTM for this PR.

srowen · 2019-04-16T14:12:11Z

Merged to master

initial commit

96bcfd4

beliefer reviewed Apr 6, 2019

View reviewed changes

core/src/main/scala/org/apache/spark/util/SizeEstimator.scala Outdated Show resolved Hide resolved

dongjoon-hyun changed the title ~~[SPARK-27397[Core] Take care of OpenJ9 JVM in Spark~~ [SPARK-27397][Core] Take care of OpenJ9 JVM in Spark Apr 6, 2019

srowen reviewed Apr 6, 2019

View reviewed changes

address review comment

6265481

dongjoon-hyun requested changes Apr 8, 2019

View reviewed changes

attilapiros reviewed Apr 8, 2019

View reviewed changes

address review comment

08eb67a

kiszk mentioned this pull request Apr 15, 2019

Different result presented in os.arch between openjdk and openj9 on Mac eclipse-openj9/openj9#5493

Closed

srowen approved these changes Apr 15, 2019

View reviewed changes

dongjoon-hyun approved these changes Apr 15, 2019

View reviewed changes

srowen closed this in 257d01a Apr 16, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-27397][Core] Take care of OpenJ9 JVM in Spark #24308

[SPARK-27397][Core] Take care of OpenJ9 JVM in Spark #24308

kiszk commented Apr 6, 2019 •

edited

Loading

SparkQA commented Apr 6, 2019

kiszk commented Apr 6, 2019

dongjoon-hyun commented Apr 6, 2019

SparkQA commented Apr 6, 2019

srowen Apr 6, 2019

SparkQA commented Apr 6, 2019

dongjoon-hyun commented Apr 7, 2019

SparkQA commented Apr 7, 2019

dongjoon-hyun left a comment •

edited

Loading

attilapiros Apr 8, 2019 •

edited

Loading

srowen Apr 8, 2019 •

edited

Loading

attilapiros Apr 8, 2019

attilapiros Apr 8, 2019

srowen Apr 8, 2019

kiszk commented Apr 9, 2019

kiszk commented Apr 14, 2019 •

edited

Loading

kiszk commented Apr 15, 2019 •

edited

Loading

SparkQA commented Apr 15, 2019

dongjoon-hyun left a comment

srowen commented Apr 16, 2019

[SPARK-27397][Core] Take care of OpenJ9 JVM in Spark #24308

[SPARK-27397][Core] Take care of OpenJ9 JVM in Spark #24308

Conversation

kiszk commented Apr 6, 2019 • edited Loading

What changes were proposed in this pull request?

How was this patch tested?

Note

SparkQA commented Apr 6, 2019

kiszk commented Apr 6, 2019

dongjoon-hyun commented Apr 6, 2019

SparkQA commented Apr 6, 2019

srowen Apr 6, 2019

Choose a reason for hiding this comment

SparkQA commented Apr 6, 2019

dongjoon-hyun commented Apr 7, 2019

SparkQA commented Apr 7, 2019

dongjoon-hyun left a comment • edited Loading

Choose a reason for hiding this comment

attilapiros Apr 8, 2019 • edited Loading

Choose a reason for hiding this comment

srowen Apr 8, 2019 • edited Loading

Choose a reason for hiding this comment

attilapiros Apr 8, 2019

Choose a reason for hiding this comment

attilapiros Apr 8, 2019

Choose a reason for hiding this comment

srowen Apr 8, 2019

Choose a reason for hiding this comment

kiszk commented Apr 9, 2019

kiszk commented Apr 14, 2019 • edited Loading

kiszk commented Apr 15, 2019 • edited Loading

SparkQA commented Apr 15, 2019

dongjoon-hyun left a comment

Choose a reason for hiding this comment

srowen commented Apr 16, 2019

kiszk commented Apr 6, 2019 •

edited

Loading

dongjoon-hyun left a comment •

edited

Loading

attilapiros Apr 8, 2019 •

edited

Loading

srowen Apr 8, 2019 •

edited

Loading

kiszk commented Apr 14, 2019 •

edited

Loading

kiszk commented Apr 15, 2019 •

edited

Loading