Build and test on Java 11 #6119

tomwhite · 2019-08-26T16:06:55Z

Investigate the changes needed to run on Java 11.

tomwhite · 2019-09-02T15:07:42Z

The goal of this PR is to investigate if what needs doing to compile GATK and run its tests on Java 11.

Some notes:

The Scala 2.12 version of Spark 2.4 is needed to run on Java 11. I disabled ADAMKryoRegistrator in this branch since the version we are using is compiled against Scala 2.11. We might consider removing it entirely and no longer support ADAM formats directly in GATK - the workaround would be to use ADAM to convert to/from BAM/VCF. ADAM is also needed for reading 2bit files.
Java 11 deprecates some APIs. Most of these are fairly easy to fix or suppress. The exception is the Javadoc API com.sun.javadoc, which has been replaced by jdk.javadoc.doclet. The javadoc tools in org.broadinstitute.hellbender.utils.help may need to be re-written (and it's not clear if it's possible to support Java 8 and Java 11 simultaneously).
Travis build. Getting this to build and test on Java 11 in addition to the current builds may be fairly involved as the matrix is already quite complicated. (The current PR just changes Java 8 to Java 11 for testing purposes - we'd need a way of getting both to run.)

The vast majority of tests are passing on Java 11, the following are failing:

Missing TwoBitRecord (from ADAM)
- ReferenceMultiSparkSourceUnitTest
- ImpreciseVariantDetectorUnitTest
- SVVCFWriterUnitTest
- DiscoverVariantsFromContigAlignmentsSAMSparkIntegrationTest
- StructuralVariationDiscoveryPipelineSparkIntegrationTest
- SvDiscoverFromLocalAssemblyContigAlignmentsSparkIntegrationTest
java.lang.NoSuchMethodError: java.nio.ByteBuffer.clear()Ljava/nio/ByteBuffer;
- SeekableByteChannelPrefetcherTest
- GatherVcfsCloudIntegrationTest
Could not serialize lambda
- ExampleAssemblyRegionWalkerSparkIntegrationTest
- PileupSparkIntegrationTest
Native HMM library code caused the tests to crash on my Mac:

Running Test: Test method testLikelihoodsFromHaplotypes[0](org.broadinstitute.hellbender.utils.pairhmm.VectorLoglessPairHMM@6282d367, true)(org.broadinstitute.hellbender.utils.pairhmm.VectorPairHMMUnitTest)
dyld: lazy symbol binding failed: can't resolve symbol __ZN13shacc_pairhmm9calculateERNS_5BatchE in /private/var/folders/cj/wyp4zgw17vj4m9qdmddvmcc80000gn/T/libgkl_pairhmm13775554937319419112.dylib because dependent dylib #1 could not be loaded
dyld: can't resolve symbol __ZN13shacc_pairhmm9calculateERNS_5BatchE in /private/var/folders/cj/wyp4zgw17vj4m9qdmddvmcc80000gn/T/libgkl_pairhmm13775554937319419112.dylib because dependent dylib #1 could not be loaded

tomwhite · 2019-09-03T14:27:08Z

Using the latest version of ADAM (which has a Scala 2.12 version) fixes the 2bit failures. I also added a fix for the java.nio.ByteBuffer.clear() problem. All unit tests are passing, and the only integration test failures are the Could not serialize lambda problems. It should be possible to fix these by making the relevant classes implement Serializable (like in samtools/htsjdk#1408).

tomwhite · 2019-09-04T10:15:31Z

As a side note, htsjdk could have 2bit support soon: samtools/htsjdk#1417

tomwhite · 2019-09-09T14:52:38Z

Unit and integration tests are now all passing on openjdk11.

The build does not however work on Java 8, since the javadoc compilation has been commented out. This will need to be made conditional on the Java version.

tomwhite · 2019-09-11T09:20:03Z

I've now fixed this PR to build on Java 8 as usual. For Java 11 testing, I've suppressed the warnings for using the com.sun.javadoc classes (they are still available in Java 11, it's just that they are deprecated - or actually marked for removal).

Ready for review.

lbergelson

@tomwhite Some comments here. I'm not certain we're actually building/running tests on 11 since I think we're doing so in the java 8 docker.

lbergelson · 2019-09-13T16:35:48Z

build.gradle

-                "The ClassLoader obtained from the Java ToolProvider is null. "
-                + "A Java $requiredJavaVersion JDK must be installed. $buildPrerequisitesMessage")
-    }
+def ensureBuildPrerequisites(largeResourcesFolder, buildPrerequisitesMessage) {


I think we still need some sort of test here because this also catches people who are running with a java 8 JRE instead of a JDK. It looks like gradle can tell you the version using JavaVersion.current() and we can still have this test in the case of java 8. We could throw a clear error message for lower versions, and a warning for versions 9/10/ 12+ that it's not tested on those.

Good point. I've updated the PR to do what you suggest.

lbergelson · 2019-09-13T17:00:15Z

.travis.yml

-       env: TEST_TYPE=integration TEST_VERBOSITY=minimal TEST_REQUIRE_GCLOUD=true
+       env: SCALA_VERSION=2.11 TEST_TYPE=integration TEST_VERBOSITY=minimal TEST_REQUIRE_GCLOUD=true
+     - jdk: openjdk11
+       env: SCALA_VERSION=2.12 TEST_TYPE=integration TEST_DOCKER=true TEST_VERBOSITY=minimal


I think this isn't actually running the tests under java 11. The docker runs on java 8, so I think the travis machine is java 11, but the actual tests are under java 8. Fixing that might be complicated. I think we would essentially have to build 2 different base images and then parameterize the docker file to take one or the other.

Would removing the TEST_DOCKER=true help? This would then be like the oraclejdk8 entry in the matrix.

lbergelson · 2019-09-13T17:49:01Z

src/main/java/org/broadinstitute/hellbender/utils/nio/SeekableByteChannelPrefetcher.java

@@ -142,7 +144,7 @@ public ByteBuffer getBuf() throws ExecutionException, InterruptedException {

        public WorkUnit resetForIndex(long blockIndex) {
            this.blockIndex = blockIndex;
-            buf.clear();
+            ((Buffer) buf).clear(); // for Java 11, see https://github.com/jruby/jruby/issues/5450


Maybe a more specific comment here as well would be useful. Also, is it possible to leave this out if we run javac with the -release 8 command? It looks like that will cause it to build using java 8 library descriptions so that it can run on 8, which we should probably be doing anyway if we're supporting both.

lbergelson · 2019-09-13T19:13:29Z

src/test/java/org/broadinstitute/hellbender/tools/spark/utils/LargeLongHopscotchSetTest.java

@@ -155,7 +155,7 @@ void loadRandomLongsTest() {
        for (int valNo = 0; valNo != HHASH_NVALS; ++valNo) {
            final long randLong = randomLong(rng);
            hopscotchSet.add(randLong);
-            hashSet.add(new Long(randLong));
+            hashSet.add(Long.valueOf(randLong));


we can probably just rely on autoboxing in most of these places

I've fixed these in #6145.

lbergelson · 2019-09-17T19:58:59Z

It looks like we need to update mockito.
https://storage.googleapis.com/hellbender-test-logs/build_reports/master_27538.13/tests/test/index.html


java.lang.IllegalArgumentException: Unknown Java version: 11
	at net.bytebuddy.ClassFileVersion.ofJavaVersion(ClassFileVersion.java:135)
	at net.bytebuddy.ClassFileVersion$VersionLocator$ForJava9CapableVm.locate(ClassFileVersion.java:357)
	at net.bytebuddy.ClassFileVersion.ofThisVm(ClassFileVersion.java:147)
	at net.bytebuddy.dynamic.loading.ClassInjector$UsingReflection$Dispatcher$CreationAction.run(ClassInjector.java:301)
	at net.bytebuddy.dynamic.loading.ClassInjector$UsingReflection$Dispatcher$CreationAction.run(ClassInjector.java:290)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at net.bytebuddy.dynamic.loading.ClassInjector$UsingReflection.<clinit>(ClassInjector.java:70)
	at net.bytebuddy.dynamic.loading.ClassLoadingStrategy$Default$InjectionDispatcher.load(ClassLoadingStrategy.java:184)
	at net.bytebuddy.dynamic.TypeResolutionStrategy$Passive.initialize(TypeResolutionStrategy.java:79)
	at net.bytebuddy.dynamic.DynamicType$Default$Unloaded.load(DynamicType.java:4456)
	at org.mockito.internal.creation.bytebuddy.SubclassBytecodeGenerator.mockClass(SubclassBytecodeGenerator.java:115)
	at org.mockito.internal.creation.bytebuddy.TypeCachingBytecodeGenerator$1.call(TypeCachingBytecodeGenerator.java:37)
	at org.mockito.internal.creation.bytebuddy.TypeCachingBytecodeGenerator$1.call(TypeCachingBytecodeGenerator.java:34)
	at net.bytebuddy.TypeCache.findOrInsert(TypeCache.java:138)
	at net.bytebuddy.TypeCache$WithInlineExpunction.findOrInsert(TypeCache.java:346)
	at net.bytebuddy.TypeCache.findOrInsert(TypeCache.java:161)
	at net.bytebuddy.TypeCache$WithInlineExpunction.findOrInsert(TypeCache.java:355)
	at org.mockito.internal.creation.bytebuddy.TypeCachingBytecodeGenerator.mockClass(TypeCachingBytecodeGenerator.java:32)
	at org.mockito.internal.creation.bytebuddy.SubclassByteBuddyMockMaker.createMockType(SubclassByteBuddyMockMaker.java:71)
	at org.mockito.internal.creation.bytebuddy.SubclassByteBuddyMockMaker.createMock(SubclassByteBuddyMockMaker.java:42)
	at org.mockito.internal.creation.bytebuddy.ByteBuddyMockMaker.createMock(ByteBuddyMockMaker.java:25)
	at org.mockito.internal.util.MockUtil.createMock(MockUtil.java:35)
	at org.mockito.internal.MockitoCore.mock(MockitoCore.java:51)
	at org.mockito.Mockito.mock(Mockito.java:1798)
	at org.mockito.Mockito.mock(Mockito.java:1711)
	at org.broadinstitute.hellbender.utils.iterators.AllLocusIteratorUnitTest.testIteratorReturnsPileupsForAllLociData(AllLocusIteratorUnitTest.java:23)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:108)
	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:55)
	at org.testng.internal.MethodInvocationHelper.invokeMethodNoCheckedException(MethodInvocationHelper.java:45)
	at org.testng.internal.MethodInvocationHelper.invokeDataProvider(MethodInvocationHelper.java:115)
	at org.testng.internal.Parameters.handleParameters(Parameters.java:509)
	at org.testng.internal.Invoker.handleParameters(Invoker.java:1308)
	at org.testng.internal.Invoker.createParameters(Invoker.java:1036)
	at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1126)

lbergelson · 2019-09-17T20:03:12Z

3.0.0 seems to be the most recent version available in maven. Hopefully it knows about java 11?

tomwhite · 2019-09-18T09:04:34Z

@lbergelson I updated to the latest 2.x Mockito and that fixed the problem.

The only failing test now is FuncotatorIntegrationTest#nonTrivialLargeDataValidationTest. The output VCF differs in the FUNCOTATION annotation, and it’s to do with ordering of the fields. E.g. it will be

1_%7C_1|false_%7C_false|false_%7C_false

not

false_%7C_false|1_%7C_1|false_%7C_false

It looks like it could be a case of using HashSet not LinkedHashSet, or HashMap not LinkedHashMap - but a quick replace throughout the GATK and HTSJDK codebase didn’t fix the problem.

lbergelson · 2019-09-18T14:09:27Z

This is exactly the sort of nightmare bug we get every java update. Just 1 isn't bad at all. I'm surprised replacing all the hashsets didn't work. It could come down to inadequate tiebreaking in a sort which falls back to identityHash. We saw that in a different bug recently. I'll take a look.

lbergelson · 2019-09-18T14:39:19Z

Interesting, when I run locally I get:

Gradle suite > Gradle test > org.broadinstitute.hellbender.tools.funcotator.FuncotatorIntegrationTest > nonTrivialLargeDataValidationTest[3](/Users/louisb/Workspace/gatk/src/test/resources/org/broadinstitute/hellbender/tools/funcotator/validationTestData/regressionTestHg19Large.vcf, /Users/louisb/Workspace/gatk/src/test/resources/large/Homo_sapiens_assembly19.fasta.gz, hg19, /Users/louisb/Workspace/gatk/src/test/resources/large/funcotator/funcotator_dataSources/, /Users/louisb/Workspace/gatk/src/test/resources/org/broadinstitute/hellbender/tools/funcotator/validationTestData/regressionTestHg19Large_expected.vcf) FAILED
    java.lang.UnsatisfiedLinkError: 'void org.sqlite.core.NativeDB._open_utf8(byte[], int)'
        at org.sqlite.core.NativeDB._open_utf8(Native Method)
        at org.sqlite.core.NativeDB._open(NativeDB.java:71)
        at org.sqlite.core.DB.open(DB.java:174)
        at org.sqlite.core.CoreConnection.open(CoreConnection.java:220)
        at org.sqlite.core.CoreConnection.<init>(CoreConnection.java:76)
        at org.sqlite.jdbc3.JDBC3Connection.<init>(JDBC3Connection.java:25)
        at org.sqlite.jdbc4.JDBC4Connection.<init>(JDBC4Connection.java:24)
        at org.sqlite.SQLiteConnection.<init>(SQLiteConnection.java:45)
        at org.sqlite.JDBC.createConnection(JDBC.java:114)
        at org.sqlite.JDBC.connect(JDBC.java:88)
        at java.sql/java.sql.DriverManager.getConnection(DriverManager.java:677)
        at java.sql/java.sql.DriverManager.getConnection(DriverManager.java:189)
        at org.broadinstitute.hellbender.tools.funcotator.dataSources.cosmic.CosmicFuncotationFactory.<init>(CosmicFuncotationFactory.java:161)
        at org.broadinstitute.hellbender.tools.funcotator.dataSources.DataSourceUtils.createCosmicDataSource(DataSourceUtils.java:469)
        at org.broadinstitute.hellbender.tools.funcotator.dataSources.DataSourceUtils.createDataSourceFuncotationFactoriesForDataSources(DataSourceUtils.java:289)
        at org.broadinstitute.hellbender.tools.funcotator.Funcotator.onTraversalStart(Funcotator.java:779)
        at org.broadinstitute.hellbender.engine.GATKTool.doWork(GATKTool.java:1046)
        at org.broadinstitute.hellbender.cmdline.CommandLineProgram.runTool(CommandLineProgram.java:139)
        at org.broadinstitute.hellbender.cmdline.CommandLineProgram.instanceMainPostParseArgs(CommandLineProgram.java:191)
        at org.broadinstitute.hellbender.cmdline.CommandLineProgram.instanceMain(CommandLineProgram.java:210)
        at org.broadinstitute.hellbender.Main.runCommandLineProgram(Main.java:163)
        at org.broadinstitute.hellbender.Main.instanceMain(Main.java:149)
        at org.broadinstitute.hellbender.Main.instanceMain(Main.java:190)
        at org.broadinstitute.hellbender.CommandLineProgramTest.runCommandLine(CommandLineProgramTest.java:27)
        at org.broadinstitute.hellbender.testutils.CommandLineProgramTester.runCommandLine(CommandLineProgramTester.java:101)
        at org.broadinstitute.hellbender.tools.funcotator.FuncotatorIntegrationTest.nonTrivialLargeDataValidationTest(FuncotatorIntegrationTest.java:624)

lbergelson · 2019-09-18T16:26:56Z

Ok. It turns out I was hitting adoptium/temurin-build#1211. Reverting to java 11.0.3 resolves that and now I get the same error you see.

lbergelson · 2019-09-18T16:32:14Z

That was a good use of my morning :(

lbergelson · 2019-09-20T18:48:08Z

I found a pretty nasty funcotator bug as part of looking into this test failure. I think we will need to patch that as a separate pr.

tomwhite · 2019-09-25T08:56:23Z

I can confirm that the fix in #6178 resolves the last failing test here for Java 11. Thanks @lbergelson.

(The reason my bulk change to LinkedHashSet/Map didn't work was because it missed Collectors.toMap().)

lbergelson · 2019-10-04T17:58:13Z

@tomwhite This needs rebasing, but now that #6173 is fixed it should hopefully work.

This reverts commit 15d6a63.

… Java 11 due a Spark error that is not fixed until Spark 3.

tomwhite · 2019-10-07T09:13:24Z

@lbergelson this is now passing thanks to your Funcotator fix.

lbergelson

👍 Looks like we have beta support for java 11...

tomwhite requested a review from lbergelson August 26, 2019 16:06

tomwhite force-pushed the tw_java11 branch from 03e9767 to 3f67bff Compare September 2, 2019 11:19

tomwhite force-pushed the tw_java11 branch from c01ec4d to 81cd162 Compare September 9, 2019 10:27

tomwhite changed the title ~~Build and test on Java 11 (DO NOT MERGE)~~ Build and test on Java 11 Sep 11, 2019

tomwhite mentioned this pull request Sep 11, 2019

Fix Java 11 deprecation warnings #6145

Merged

lbergelson requested changes Sep 13, 2019

View reviewed changes

tomwhite force-pushed the tw_java11 branch 3 times, most recently from bb6d926 to cd5d596 Compare September 17, 2019 19:55

tomwhite mentioned this pull request Sep 25, 2019

Fix VCFFuncotation output order bug #6178

Merged

tomwhite added 5 commits October 5, 2019 09:28

Use Spark 2.4 with Scala 2.12

29d8320

Use adam-core-spark2_2.12:0.28.0

3113b4a

Fix java.nio.ByteBuffer.clear() error

e3b385d

Serializable identityFunction

3325ce1

Fix 'Could not serialize lambda' for AssemblyRegionWalkerSpark

717c6b8

tomwhite added 7 commits October 5, 2019 09:28

Suppress deprecation warnings from closeQuietly in Commons IO

19102dd

Try testing on Java 11 on Travis

648b73e

Don't use docker for Java 11

4631b17

Revert "Fix java.nio.ByteBuffer.clear() error"

2d7b2ad

This reverts commit 15d6a63.

Pass in scala version in non-docker test

0efe35d

Upgrade mockito to version that works with Java 11

b57011a

Skip PileupSparkIntegrationTest#testFeaturesPileupHdfs which fails on…

e4d37a3

… Java 11 due a Spark error that is not fixed until Spark 3.

tomwhite force-pushed the tw_java11 branch from 31d510f to e4d37a3 Compare October 5, 2019 08:32

lbergelson approved these changes Oct 7, 2019

View reviewed changes

tomwhite merged commit fd7abc4 into master Oct 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build and test on Java 11 #6119

Build and test on Java 11 #6119

tomwhite commented Aug 26, 2019

tomwhite commented Sep 2, 2019

tomwhite commented Sep 3, 2019

tomwhite commented Sep 4, 2019

tomwhite commented Sep 9, 2019

tomwhite commented Sep 11, 2019

lbergelson left a comment

lbergelson Sep 13, 2019

tomwhite Sep 17, 2019

lbergelson Sep 13, 2019

tomwhite Sep 17, 2019

lbergelson Sep 13, 2019 •

edited

lbergelson Sep 13, 2019

tomwhite Sep 17, 2019

lbergelson commented Sep 17, 2019

lbergelson commented Sep 17, 2019

tomwhite commented Sep 18, 2019

lbergelson commented Sep 18, 2019

lbergelson commented Sep 18, 2019 •

edited

lbergelson commented Sep 18, 2019

lbergelson commented Sep 18, 2019

lbergelson commented Sep 20, 2019

tomwhite commented Sep 25, 2019

lbergelson commented Oct 4, 2019

tomwhite commented Oct 7, 2019

lbergelson left a comment

Build and test on Java 11 #6119

Build and test on Java 11 #6119

Conversation

tomwhite commented Aug 26, 2019

tomwhite commented Sep 2, 2019

tomwhite commented Sep 3, 2019

tomwhite commented Sep 4, 2019

tomwhite commented Sep 9, 2019

tomwhite commented Sep 11, 2019

lbergelson left a comment

Choose a reason for hiding this comment

lbergelson Sep 13, 2019

Choose a reason for hiding this comment

tomwhite Sep 17, 2019

Choose a reason for hiding this comment

lbergelson Sep 13, 2019

Choose a reason for hiding this comment

tomwhite Sep 17, 2019

Choose a reason for hiding this comment

lbergelson Sep 13, 2019 • edited

Choose a reason for hiding this comment

lbergelson Sep 13, 2019

Choose a reason for hiding this comment

tomwhite Sep 17, 2019

Choose a reason for hiding this comment

lbergelson commented Sep 17, 2019

lbergelson commented Sep 17, 2019

tomwhite commented Sep 18, 2019

lbergelson commented Sep 18, 2019

lbergelson commented Sep 18, 2019 • edited

lbergelson commented Sep 18, 2019

lbergelson commented Sep 18, 2019

lbergelson commented Sep 20, 2019

tomwhite commented Sep 25, 2019

lbergelson commented Oct 4, 2019

tomwhite commented Oct 7, 2019

lbergelson left a comment

Choose a reason for hiding this comment

lbergelson Sep 13, 2019 •

edited

lbergelson commented Sep 18, 2019 •

edited