[test] Java 17 upgrade w/ Java 8 bytecode #23803
Draft
+321
−121
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ZacBlanco
force-pushed
the
upgradeJava17
branch
5 times, most recently
from
a8d965a to
227f154
Compare
|
Collating the test failures I see so far - |
|
I looked into presto-accumuloThis one failed because the product-tests-specific-environment2This one failed because it looks like tests-other-modulesSeems mostly add-opens issues. Needs:
|
product-tests-specific-environment2I believe these failures happen since we are building with Java 17 but using Java 8 at runtime in the docker images. These issues should likely be fixed once the docker images are updated to use Java 17. We should also look into enhancing Tempto to be able to use multiple Java versions so that we can run our CI pipeline with both Java 8 and Java 17+ and pass an argument to be able to choose which Java version to use while starting the Presto server in the docker images. |
test-other-modulesApart from the add-opens issues, I see issues while trying to connect to the zookeeper client in presto-kafka due to which none of the tests ran. Most likely the version is not compatible with Java 17. |
test (:presto-spark-base -P presto-spark-tests-smoke)Current failures are all related to add-opens issues. But many tests were skipped due to these, we will have a better idea once we fix them. test (:presto-spark-base -P presto-spark-tests-all-queries)Current failures are all related to add-opens issues. But all tests were skipped due to these, we will have a better idea once we fix them. test / test (:presto-spark-base -P presto-spark-tests-spill-queries)Current failures are all related to add-opens issues. But all tests were skipped due to these, we will have a better idea once we fix them. |
imjalpreet
force-pushed
the
upgradeJava17
branch
from
3654a49 to
b6cab24
Compare
ZacBlanco
force-pushed
the
upgradeJava17
branch
2 times, most recently
from
6b1917b to
e6b7f1d
Compare
|
|
aaneja
force-pushed
the
upgradeJava17
branch
3 times, most recently
from
3893f06 to
72da587
Compare
aaneja
changed the title
There are a large number of parameters passed to argLine through the airbase POM. Set the airbase-specific props instead to aid in test failures
It seems later JVMs have drasitically improved their String representations. However, the string test is a bit of a wash because the estimated size entirely depends upon the strings which are used as input to the sketch. I think it's ok for this test to not be accurate for string-typed arguments for now.
CI actions seem to fail due to this line: https://github.com/apache/spark/blob/866816eb97002863ec205d854e1397982aecbc5e/core/src/main/scala/org/apache/spark/serializer/SerializationDebugger.scala#L70-L71 Attempting to access an item from sun.security.action
For future, we can leave this as the default (true), but for testing on the PR we want to see both actions run
to one that uses JDK 17
Temp change to limit the errors from using a JDK 17 only image
aaneja
force-pushed
the
upgradeJava17
branch
from
642c536 to
9ec6776
Compare
imjalpreet
force-pushed
the
upgradeJava17
branch
from
bbbf26d to
309e0d7
Compare
imjalpreet
force-pushed
the
upgradeJava17
branch
from
309e0d7 to
bc0dbe6
Compare
repository tag copied from the maven 3 super POM at https://maven.apache.org/ref/3.9.9/maven-model-builder/super-pom.html
- Uses new image 'aaneja/hdp2.6-hive-kerberized-jdk17' that is the base `hdp2.6-hive-kerberized` with JDK 17 added to `/opt/java` - presto-launcher-wrapper.sh was updated to use Java from `/opt/java` if present - New add-opens for Keberos classes
aaneja
force-pushed
the
upgradeJava17
branch
from
5a87598 to
bbdbb16
Compare
- multinode-tls-kerberos updated to use `aaneja/hdp2.6-hive-kerberized-jdk17` - singlenode-ldap updated to use new image `aaneja/centos6-oj8-openldap-jdk17` - product-tests-specific-environment2 updated to only run on JDK 17 for now
aaneja
force-pushed
the
upgradeJava17
branch
from
5643924 to
18d435a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Motivation and Context
Impact
Test Plan
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.
If release note is NOT required, use: