NIFI-5176 Java 11 Build Compatibilty #3404
Conversation
|
@jtstorck speaking of dependency issues any issues with Lucene? In the past Lucene has had some weirdness around Java versions at least in the Java 7 days. |
|
@MikeThomsen I haven't explicitly tested Lucene. The tests run during the build are all passing, with a few exceptions for dependencies like Ignite and ActiveMQ that don't yet (the last time I checked) support Java 11. I haven't seen any problems related to Lucene yet. I'm in the process of creating smaller PRs based on the work done in this PR to make it easier to review and merge these changes to master. That's the main focus at this point, and this PR will eventually be closed as the changes it contains are parceled out and merged to master. Once these changes are all merged to master, we can address issues with individual components while running on Java 11. |
|
@jtstorck FYI this can't cleanly rebase against master now. |
|
I would expect that, @MikeThomsen. Other changes are being applied to master that may conflict with changes I've made in this PR, and parts of this PR have been extracted from this PR already and applied to master. Over time, all the changes from this PR will be merged into master in separate smaller PRs, and eventually this PR will be closed without merging. |
|
I'm rebasing this PR onto master since a few of the individual PRs to move NiFi towards Java 11 compatible build have been merged. This will make it easier to determine the remaining changes from that PR, and if it's worth pulling out any change-sets into separate PRs. I'm assuming it'll be a good idea to isolate the Groovy changes made in PR 3404 in a separate PR, along with the Groovy changes from @MikeThomsen's Groovy PR (PR 3015). After another rebase of PR 3404 onto master after the Groovy upgrade is merged the remaining changes may be manageable as the "final" PR to enable building on Java 11. Folllow-on PRs for bug fixes and stability can be created to address specific issues. |
nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/pom.xml
Show resolved
Hide resolved
|
Latest groovy is also 2.5.7 |
On my local clone while rebasing against master, I tried using Groovy 2.5.7, but there's an issue that was fixed in Groovy 2.5.4 that has resurfaced in 2.5.7. This issue occurs with Groovy 2.5.7 (with spock-core:1.2-groovy-2.5 and 1.3-groovy-2.5): The tests pass when Groovy 2.5.4 is used. For now, I'm going to leave the Groovy version set to 2.5.4. |
|
Rebased on top of PR 3547 (Groovy upgrade to 2.5.4) to slim the PR down to the remaining changes for Java 11. There are still a few issues to address with this PR, the main one being TLS version 1.2 and 1.3. When this PR was originally created, TLSv1.3 was not working on Java 11. More investigation and testing is needed to resolve this, as NiFi on Java 11 should work with TLSv1.3. Another issue is the ignoring of the RecordLookup tests, and the issue that was causing the error has since been fixed on master. I'll rerun those tests and verify that they work again. A third issue regarding components that do not support Java 11 yet (Ignite, ActiveMQ) may not be resolved before this PR is merged to master. This will be an issue for those that want to build and run NiFi on Java 11 and use those components. Those components should work fine if NiFi is built on Java 8. |
|
I wonder if we should create a profile that excludes Ignite, ActiveMQ, etc. so people can get using it on Java 11 with a simple build on their own. |
ijokarumawak
left a comment
ijokarumawak
left a comment
There was a problem hiding this comment.
@jtstorck I will keep researching on the issue around TLS 1.3. Posted a comment about my thoughts on that. Thanks.
...s/nifi-site-to-site-client/src/main/java/org/apache/nifi/remote/client/SiteToSiteClient.java
Outdated
Show resolved
Hide resolved
|
If no one has any objections, along with the rebase of the PR against master now that PR 3547 has been merged, I'd like to squash all of the commits into a single commit. Following that, I'll address as many of the TODO/FIXME comments in subsequent commits, and then squash again once the PR is ready to be taken out of draft status. |
jtstorck
force-pushed
the
NIFI-5176
branch
5 times, most recently
from
3f21178 to
e954639
Compare
jtstorck
changed the title
|
Java 8 builds worked, Java 11 build worked until it timed out. |
|
@MikeThomsen @ijokarumawak Would you mind reviewing this PR? When testing, please use TLSv1.2, since we'll do separate PRs for TLSv1.3 support for NiFi under Java 11. I'll continue to investigate why the Java 11 build times out on Travis, but it does complete locally. |
|
@alopresto There are changes in this PR to |
...dard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/pom.xml
Outdated
Show resolved
Hide resolved
|
Manual build locally gave me this |
To work around this error (which happens on master as well), add |
|
I ran a full build of this with Oracle JDK 1.8.0.201 and Adopt OpenJDK 11, both builds passed all tests except for an unrelated test failure with timezone stuff JdbcCommon. I also ran a secure cluster on both JDKs and ran basic flows without any issues. I'm a +1, nice work! |
|
@jtstorck should |
MikeThomsen
left a comment
MikeThomsen
left a comment
There was a problem hiding this comment.
Need to update the Dockerfiles to use Java 11 base images, but other than that +1.
|
I'm also +1. Full clean build and contrib check passed and a secured NiFi cluster worked as expected. I used Oracle JDK 11.0.3. Thanks for your effort @jtstorck ! |
It shouldn't. Since that issue is outside of this PR, I don't think a workaround should be committed in this PR. I'll have a PR up for the I'm tempted to remove the changes for the Travis build from this PR as well, so that the PR isn't held up by Travis failures. I have another branch in which I was working on getting the build to pass on Travis with Java 11, but there are some issues with the time it takes to complete the build. The caching on Travis isn't working as I'd expect, and all the dependencies need to be downloaded for each Java 11 build. If I remove the Travis changes from this PR, Travis would continue to build master with Java 8 only. I'd submit a follow-on PR for the Java 11 build on Travis.
I'd like to keep the docker builds set to use Java 8 since the convenience binaries released are built with Java 8, until the minimum version of Java for NiFi is moved to Java 11. There are also some components that are not able to be built on Java 11 (ActiveMQ, Ignite, Hadoop), so I think we should hold off on making that change in our Docker modules. Does anyone have specific issues with either of the above comments? @alopresto, if you are a +1 on the SSL-related changes, then it looks like the PR is in a good spot to be merged, once the Travis concerns are either fixed or removed from this PR. |
|
I've removed the @alopresto @thenatog Once you review and are a +1 on this PR, based on the other +1s already received, the PR can be merged. The JIRA for adding a Java 11 build to Travis CI is NIFI-5505. |
|
Reviewing |
Added "jigsaw" profile to multiple modules, which auto-activates when building with Java 11 and adds several dependencies: jaxb, activation, and annotation-api. Updated SslContextFactory to return tuple of socket factory with trust manager for issue with okhttp api changes in java 9+ Updated TestGet/PostHTTPGroovy to use default SSL context to get default cipher suites Updated StandardTemplateDAOSpec.groovy, was using a 37-character UUID, shortened to 36 characters. Multiple tests updated to specifically use TLSv1.2, since two-way TLSv1.3 for some component tests fail during the Java 11 build. Needs more investigation. Replaced GString with String concatenation for map key's value in ScriptedReportingTaskGroovyTest to avoid runtime error of casting GString to String when retrieved from a map that is storing the GString Removed nifi-toolkit-api plugin configuration of maven-compiler-plugin, it is inherited Resolved deprecation errors during Groovy compile for bouncycastle, Extension should be used in place of X509Extension Fixed JNAOverridingJUnitRunner's creation of the classpath for the custom classloader created to be able to mock jna classes Removed import of IOUtils (from the wrong package) from InferenceSchemaStrategy Updated maven-compiler-plugin version to 3.8.1 TLS (default in Java 11 is TLSv1.3) working for Site-to-Site client tests after upgrading JDK installation to JDK 11.0.3, httpclient5 5.0-beta4, and httpasyncclient 4.1.4 HttpNotificationService updated to find and use the first found X509TrustManager rather than casting directly from the array of TrustManagers returned from SslContextFactory Removed unnecessary throws declaration from getSslSocketFactory method Replaced DnsContextFactory.class.getName() with a string to avoid having to export/open the jdk.naming.dns module Updated TestGetIgniteCache and TestPutIgniteCache.java to skip tests on Java 11+ (via Assume), and noted the check should be removed once a version of Ignite is released that supports Java 11 Updated SpringContextProcessor to create proper ClassLoader and uses URLClassloader.getResource() instead of URLClassloader.findResource() in isConfigResolvable. Due to the application classloader no longer being a URLClassLoader in Java 9+, the URLClassLoader created in isConfigResolvable no longer has explicit access to the parent's resources as URLs. URLClassLoader.getResource() searches the parent classloaders, and must be used instead of URLClassLoader.findResource() which only searches the URLs in the URLClassLoader and does not search the parent classloaders.
…OMs of nifi-hbase_1_1_2-client-service and nifi-hbase_2-client-service
…blish the correct httpclient dependency for when building with Java 11
|
Created secure clusters, used ListenHTTP and InvokeHTTP processors and Remote Process Groups (sending back to input port on same cluster) with the following Java versions: Build on JDK 1.8.0_172 and run on JRE 1.8.0_172. ✅ Building on JDK 1.8.0_172 and run on JRE11.0.5+10 caused exceptions when attempting to send to local input port with RPG: But did not see this error on the previous builds (JDK8/JRE8, JDK11/JRE11). This is something that could be looked into separately as it looks like the expected combinations of build/runtime are working. +1 My |
Added "jigsaw" profile to multiple modules, which auto-activates when building with Java 11 and adds several dependencies: jaxb, activation, and annotation-api. Updated SslContextFactory to return tuple of socket factory with trust manager for issue with okhttp api changes in java 9+ Updated TestGet/PostHTTPGroovy to use default SSL context to get default cipher suites Updated StandardTemplateDAOSpec.groovy, was using a 37-character UUID, shortened to 36 characters. Multiple tests updated to specifically use TLSv1.2, since two-way TLSv1.3 for some component tests fail during the Java 11 build. Needs more investigation. Replaced GString with String concatenation for map key's value in ScriptedReportingTaskGroovyTest to avoid runtime error of casting GString to String when retrieved from a map that is storing the GString Removed nifi-toolkit-api plugin configuration of maven-compiler-plugin, it is inherited Resolved deprecation errors during Groovy compile for bouncycastle, Extension should be used in place of X509Extension Fixed JNAOverridingJUnitRunner's creation of the classpath for the custom classloader created to be able to mock jna classes Removed import of IOUtils (from the wrong package) from InferenceSchemaStrategy Updated maven-compiler-plugin version to 3.8.1 TLS (default in Java 11 is TLSv1.3) working for Site-to-Site client tests after upgrading JDK installation to JDK 11.0.3, httpclient5 5.0-beta4, and httpasyncclient 4.1.4 HttpNotificationService updated to find and use the first found X509TrustManager rather than casting directly from the array of TrustManagers returned from SslContextFactory Removed unnecessary throws declaration from getSslSocketFactory method Replaced DnsContextFactory.class.getName() with a string to avoid having to export/open the jdk.naming.dns module Updated TestGetIgniteCache and TestPutIgniteCache.java to skip tests on Java 11+ (via Assume), and noted the check should be removed once a version of Ignite is released that supports Java 11 Updated SpringContextProcessor to create proper ClassLoader and uses URLClassloader.getResource() instead of URLClassloader.findResource() in isConfigResolvable. Due to the application classloader no longer being a URLClassLoader in Java 9+, the URLClassLoader created in isConfigResolvable no longer has explicit access to the parent's resources as URLs. URLClassLoader.getResource() searches the parent classloaders, and must be used instead of URLClassLoader.findResource() which only searches the URLs in the URLClassLoader and does not search the parent classloaders. NIFI-5176 Moved exclusion of jdk.tools to the jigsaw profile in the POMs of nifi-hbase_1_1_2-client-service and nifi-hbase_2-client-service NIFI-5176 Updated site-to-site-client's POM to use properties to establish the correct httpclient dependency for when building with Java 11 This closes apache#3404.
Added "jigsaw" profile to multiple modules, which auto-activates when building with Java 11 and adds several dependencies: jaxb, activation, and annotation-api. Updated SslContextFactory to return tuple of socket factory with trust manager for issue with okhttp api changes in java 9+ Updated TestGet/PostHTTPGroovy to use default SSL context to get default cipher suites Updated StandardTemplateDAOSpec.groovy, was using a 37-character UUID, shortened to 36 characters. Multiple tests updated to specifically use TLSv1.2, since two-way TLSv1.3 for some component tests fail during the Java 11 build. Needs more investigation. Replaced GString with String concatenation for map key's value in ScriptedReportingTaskGroovyTest to avoid runtime error of casting GString to String when retrieved from a map that is storing the GString Removed nifi-toolkit-api plugin configuration of maven-compiler-plugin, it is inherited Resolved deprecation errors during Groovy compile for bouncycastle, Extension should be used in place of X509Extension Fixed JNAOverridingJUnitRunner's creation of the classpath for the custom classloader created to be able to mock jna classes Removed import of IOUtils (from the wrong package) from InferenceSchemaStrategy Updated maven-compiler-plugin version to 3.8.1 TLS (default in Java 11 is TLSv1.3) working for Site-to-Site client tests after upgrading JDK installation to JDK 11.0.3, httpclient5 5.0-beta4, and httpasyncclient 4.1.4 HttpNotificationService updated to find and use the first found X509TrustManager rather than casting directly from the array of TrustManagers returned from SslContextFactory Removed unnecessary throws declaration from getSslSocketFactory method Replaced DnsContextFactory.class.getName() with a string to avoid having to export/open the jdk.naming.dns module Updated TestGetIgniteCache and TestPutIgniteCache.java to skip tests on Java 11+ (via Assume), and noted the check should be removed once a version of Ignite is released that supports Java 11 Updated SpringContextProcessor to create proper ClassLoader and uses URLClassloader.getResource() instead of URLClassloader.findResource() in isConfigResolvable. Due to the application classloader no longer being a URLClassLoader in Java 9+, the URLClassLoader created in isConfigResolvable no longer has explicit access to the parent's resources as URLs. URLClassLoader.getResource() searches the parent classloaders, and must be used instead of URLClassLoader.findResource() which only searches the URLs in the URLClassLoader and does not search the parent classloaders. NIFI-5176 Moved exclusion of jdk.tools to the jigsaw profile in the POMs of nifi-hbase_1_1_2-client-service and nifi-hbase_2-client-service NIFI-5176 Updated site-to-site-client's POM to use properties to establish the correct httpclient dependency for when building with Java 11 This closes apache#3404.
5 participants
NiFi can be built and run on Java 11.
The goal of this PR is to establish general build compatibility for Java 11 while retaining complete build and runtime validity for Java 8.
TODOs/needed fixes will be added here.
Current status:
TLSorTLSv1.3in tests during the Java 11 build, need to investigate and resolve in a follow-on PR. For now, the tests explicitly useTLSv1.2.Things to test with this PR:
Note: When building, to work around
TestJdbcCommontimezone issues (which also occur on master), add-Duser.timezone=UTCto the mvn command when building.For the three build/run scenarios above:
The goal of this PR isn't to fix every issue running on Java 11, but it should not introduce any new issues for building on Java 8 and running the Java 8 build on Java 8 or 11.
Follow-on PRs can be created to fix issues encountered while running on Java 11 for specific components.