CNDB-11508 main-5.0 branch #1408

ekaterinadimitrova2 · 2024-11-07T00:52:08Z

What is the issue

We cannot:

compile and run main-5.0 on JDK22
run tests on JDK22 with the current config

What does this PR fix and why was it fixed

...

Adding test fixes based on testing we already did in a POC branch; enables us to build and run Cassandra on JDK22 plus run all test suites on JDK22 from the main-5.0 branch. We also added numerous fixes that get CI to a very good state on JDK22. CI runs posted on the GH issue.
apply a few fixes for JDK17 to bring CI to a good shape too. CI runs posted on the GH issue.

Checklist before you submit for review

Make sure there is a PR in the CNDB project updating the Converged Cassandra version
Use NoSpamLogger for log lines that may appear frequently in the logs
Verify test results on Butler
Test coverage for new/modified code is > 80%
Proper code formatting
Proper title for each commit staring with the project-issue number, like CNDB-1234
Each commit has a meaningful description
Each commit is not very long and contains related changes
Renames, moves and reformatting are in distinct commits

ekaterinadimitrova2 · 2024-11-08T17:13:25Z

CI is in a bad shape today going up and down and I am not being able to really check test results.
I am pushing some review feedback updates and JDK17 test fixes and I will move the PRs to draft so they do not trigger useless CI triggers. I will move to another issue I am working on and check back later.

conf/jvm17-server.options

JeremiahDJordan · 2024-11-13T17:06:15Z

conf/jvm-server.options

@@ -171,6 +171,9 @@
 #-Xms4G
 #-Xmx4G

+# Need experimental bytebuddy for JDK21
+-Dnet.bytebuddy.experimental


Should we just upgrade to a newer byte buddy? JDK 22 is officially supported in 1.14.9 and the latest 1.15.10 supports up to jdk24.

Actually we are already using 1.14.17 which supports up to JDK 23. So we should not need this already.

I added it because our tests showed we need it -
https://jenkins-stargazer.aws.dsinternal.org/job/ds-cassandra-build/867/#showFailuresLink and https://jenkins-stargazer.aws.dsinternal.org/job/ds-cassandra-build/867/testReport/junit/org.apache.cassandra.cache/ChunkCacheInterceptingTest/tests_stage_1___jvm_unit_tests___jvm_unit_compression_tests___testChunkCacheInterception_compression_jdk22/
I believe our friend mockito-inline was to be blamed here. Tatu had the same issues with Stargate, if I remember correctly.

I believe the last I tried a few months ago the latest version did not work with mockito-inline so I had to downgrade and adjust versions in the POC branch. Maybe something has changed since than, so I suggest we open a follow up ticket for that?

but this is jvm-server.options which is not used in unit tests, is it?

Good question, I don't think we actually needed. I will remove it and see if CI complains for some reason that I cannot recall now.

JeremiahDJordan · 2024-11-13T17:15:02Z

build.xml

@@ -515,6 +606,7 @@
        <jvmarg value="-Dstorage-config=${test.conf}"/>
        <jvmarg value="-Dcassandra.reads.thresholds.coordinator.defensive_checks_enabled=true" /> <!-- enable defensive checks -->
        <jvmarg value="-javaagent:${build.dir.lib}/jars/jamm-${jamm.version}.jar" />
+        <jvmarg value="-Dnet.bytebuddy.experimental=true"/>


none of the byte buddy -D should be needed as we are using version 1.14.17 which supports up to JDK23

Check this comment:
#1408 (comment)
Issues with mockito-inline

JeremiahDJordan

looks good. we should remove all the byte buddy -D, they should not be needed. possibly upgrade to the latest byte buddy to prevent needing to touch it again later.

ekaterinadimitrova2 · 2024-11-13T17:28:30Z

Thank you for the review!

looks good. we should remove all the byte buddy -D, they should not be needed. possibly upgrade to the latest byte buddy to prevent needing to touch it again later.

Please check below comment:
#1408 (comment)

jacek-lewandowski · 2024-11-20T11:03:34Z

bin/cassandra.in.sh

@@ -122,11 +122,16 @@ jvmver=`echo "$java_ver_output" | grep '[openjdk|java] version' | awk -F'"' 'NR=
 JVM_VERSION=${jvmver%_*}
 short=$(echo "${jvmver}" | cut -c1-2)

-JAVA_VERSION=17
+JAVA_VERSION=22
 if [ "$short" = "11" ]  ; then
     JAVA_VERSION=11
 elif [ "$JVM_VERSION" \< "17" ] ; then


is this correct and consistent with what we have in main? I suggested to allow only LTS versions but eventually @JeremiahDJordan opposed and we allow for any >= 11; While here I can see 12..16 and 18..21 seem to be disallowed?

There is a patch in trunk that adds a flag to be able to allow those. We should port it and not allow it by default. But let's not do it here?
This comment I wrote in the issue:
UPDATE: I will pull the patch from CASSANDRA-18688 in a separate ticket. We can add it when we fix all tests and enable main branch for testing with JDK22 in CI. Otherwise, there will be confusion that the branch is JDK22 ready when it is still not fully ready.

jacek-lewandowski · 2024-11-20T11:08:02Z

build.xml

+        <string>--add-opens java.base/jdk.internal.reflect=ALL-UNNAMED</string>
+        <string>--add-opens java.base/jdk.internal.vm=ALL-UNNAMED</string>
+        <string>--add-opens java.base/sun.nio.ch=ALL-UNNAMED</string>
+        <string>--add-opens jdk.compiler/com.sun.tools.javac=ALL-UNNAMED</string>


I have similar thoughts as for main - we should leverage the fact we mostly extend the sets of exports and opens with newer JVM versions

I already responded on the other PR, it applies to this comment too:
#1398 (comment)

jacek-lewandowski · 2024-11-20T11:11:08Z

conf/jvm22-server.options

+--add-opens java.base/java.nio=ALL-UNNAMED
+--add-opens java.base/java.util=ALL-UNNAMED
+--add-opens java.base/java.util.concurrent.atomic=ALL-UNNAMED
+--add-opens java.base/sun.nio.ch=ALL-UNNAMED


Similarly as in main there are duplications

jacek-lewandowski · 2024-11-20T11:12:51Z

build.xml

@@ -1247,6 +1340,7 @@
      <jvmarg value="-Dcassandra.ring_delay_ms=1000"/>
      <jvmarg value="-Dcassandra.tolerate_sstable_size=true"/>
      <jvmarg value="-Dcassandra.skip_sync=true" />
+      <jvmarg value="-Dnet.bytebuddy.experimental=true"/>


I don't think you need to add it individually to each test configuration, to IntellliJ files, etc. - just add it to jdk22 props which are applied everywhere

jacek-lewandowski · 2024-11-20T11:14:22Z

src/java/org/apache/cassandra/index/sasi/utils/trie/AbstractPatriciaTrie.java

@@ -790,7 +790,8 @@ TrieEntry<K, V> nextEntryImpl(TrieEntry<K, V> start, TrieEntry<K, V> previous, T
     * This is implemented by going always to the left until
     * we encounter a valid uplink. That uplink is the first key.
     */
-    TrieEntry<K, V> firstEntry()
+    // @Override needed in JDK 21+.


if @Override is needed, then why don't you add it?

Because it is needed only if we build on JDK21+. There is nothing to override on previous version and we still build on 11.

ekaterinadimitrova2 · 2024-11-20T22:01:24Z

Feedback addressed. @jacek-lewandowski , I believe the main question here is:

let me know what you want me to do with this one - #1398 (comment)
Option 1: remove it from both main and main-5.0 and diverge from Apache (I am against that);
Option 2: remove the deletions in main and open follow up ticket to improve that target for all branches? (I prefer this one)

* CNDB-11498: Compile on JDK22: - firstEntry() and lastEntry() were trying to assign weaker access privileges (e.g., private or package-private) when they need to be public according to the interface they are implementing (SequencedMap). - JDK 21+ adds SequencedCollection: The List and NavigableSet interfaces now inherit from a new interface called SequencedCollection, which introduces the reversed() method. However, NavigableSet.reversed() returns a NavigableSet, while List.reversed() returns a List. This results in a conflict, as the types are incompatible.

…roperties `java.default` and `java.supported`. ant generate-idea-files now support JDK 8, JDK 11 and JDK 22. To add support of another JDK the java-jvmargs property must be set for the JDK in question (see how it's done in build.xml for Java 11 and 22) Other minor, but notable changes are: - test jvmargs are now added to idea run configurations - .idea dir and project iml file are first removed and then recreated during `ant generate-idea-files` Based on what was done in CASSANDRA-18467, CASSANDRA-18179, CASSANDRA-18258 for 17 plus additional stuff for 21 Co-authored-by: Ekaterina Dimitrova<[email protected]> Co-authored-by: Mick Semb Wever <[email protected]> Co-authored-by: Jakub Zytka <[email protected]> Fix add-opens and add-exports CNDB-11508: CQLSHLIB tests update Fix EmptyValuesTest Fix sjk Fix tools/bin/cassandra.in.sh; fix some JDK internals openings for audit logger and vectorizedMismatch Upgrade Mockito to fix UnifiedCompactionStrategyTest Get rid of JDK8 in scripts, not supported anywhere anymore

…or JDK11

…aybe we still want to check newer version for JDK22 specifically. Though this is the last ecj version to support JDK11. Upgrade: - ecj plus fix the java udf functions for JDK11+ - snakeyaml - it was already bumped in CNDB for security vulnerability - test dependencies: jacoco, byteman - higher version than CNDB but it is needed for catching up on JDK22 in tests findbugs - aligned with CNDB version but we probably want at some point to get to major version upgrade; not a priority for now jmh, bytebuddy - bumped to latest versions as they are known for not working on newer JDK versions

When running tests with JDK17 we see NoClassDefFoundError when trying to instante org.apache.cassandra.Util class due caused by getSupportedMTimeGranularity trying to access a private field. This patch modified the build configuration to add a jvm option allowing that access.

…sion provided

ekaterinadimitrova2 · 2024-11-22T18:31:43Z

Thank you for the reviews! There are a few conversations about how we can improve/change certain things like generate-idea-files ant target or test JDK properties - we agreed in Slack we can tackle any of that in follow-up tickets if we want to improve things further, but the current work is already good enough to be committed as-is. I just pushed the rebased branch. To save resources, I will wait to confirm the JDK 11 PR gate looks good, and then I will push the JDK17 and JDK22 last runs and whatever else we need if there are no new issues.

ekaterinadimitrova2 · 2024-11-22T20:46:28Z

JDK11 CI finished successfully.
There are a few test failures marked new. They all do not seem related, and all but two I cannot reproduce, seems like flakies.
The two I can reproduce, I can reproduce also on main-5.0 . They are part of - ClientRequestMetricsLatenciesTest

Moving forward to the additional testing I will have to push manually. I publish links on the main issue.

sonarcloud · 2024-11-22T23:27:20Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

cassci-bot · 2024-11-22T23:29:47Z

❌ Build ds-cassandra-pr-gate/PR-1408 rejected by Butler

18 new test failure(s) in 6 builds
See build details here

Found 18 new test failures

Showing only first 15 new test failures

Test	Explanation	Branch history
t.TestCqlshUnicode.test_unicode_desc	flaky	🔴🔵🔴🔵🔴🔴
t.TestCqlshUnicode.test_unicode_identifier	flaky	🔴🔵🔴🔴🔴🔵
t.TestCqlshUnicode.test_unicode_value_round_trip	flaky	🔵🔴🔵🔴🔵🔴
...adCommitLogAndSSTablesWithDroppedColumnTestCC40	failing	🔴🔴🔴🔴🔴🔴
...adCommitLogAndSSTablesWithDroppedColumnTestCC50	failing	🔴🔴🔴🔴🔴🔴
...oadCommitLogAndSSTablesWithDroppedColumnTestDSE	failing	🔴🔴🔴🔴🔴🔴
...thRestartTest.testReadingValuesOfDroppedColumns	failing	🔴🔴🔴🔴🔴🔴
...rrorsTest.testNoSuchRepairSessionAnticompaction	flaky	🔴🔵🔴🔴🔴🔴
o.a.c.d.t.r.ForceRepairTest.forceWithDifference	regression	🔴🔴🔵🔴🔴🔵
...ereUpdateMakesRowMatchNonOrderingPredicates[dc]	regression	🔴🔵🔵🔵
o.a.c.i.c.CompressionMetadataTest.testMemoryIsF...	failing	🔴🔴🔴🔴🔴🔴
....i.c.CompressionMetadataTest.testMemoryIsShared	failing	🔴🔴🔴🔴🔴🔴
...ientRequestMetricsLatenciesTest.testReadMetrics	regression	🔴🔴🔵🔵
...entRequestMetricsLatenciesTest.testWriteMetrics	regression	🔴🔴🔵🔵
o.a.c.u.b.BinLogTest.testTruncationReleasesLogS...	regression	🔴🔴🔵🔵

Found 3 known test failures

ekaterinadimitrova2 marked this pull request as draft November 8, 2024 17:13

ekaterinadimitrova2 self-assigned this Nov 12, 2024

ekaterinadimitrova2 force-pushed the CNDB-11508-5.0 branch from 889f389 to 890af4b Compare November 12, 2024 20:08

ekaterinadimitrova2 marked this pull request as ready for review November 12, 2024 21:18

JeremiahDJordan reviewed Nov 13, 2024

View reviewed changes

conf/jvm17-server.options Show resolved Hide resolved

JeremiahDJordan reviewed Nov 13, 2024

View reviewed changes

JeremiahDJordan approved these changes Nov 13, 2024

View reviewed changes

jacek-lewandowski reviewed Nov 20, 2024

View reviewed changes

jacek-lewandowski approved these changes Nov 22, 2024

View reviewed changes

ekaterinadimitrova2 and others added 13 commits November 22, 2024 13:29

Fix antlr crash on JDK22 - port from CNDB-11133

2f0f8e5

CC5 fixes dtests by not hardening gc.log location - same as 61589a2 f…

c40dbe8

…or JDK11

Remove useless comment

60e601f

JdkProperties does not need to account for JDK8 anymore

806b3e0

Sort alphabetically add-exports/add-opens as per review request

dea3b70

Bring parts of CNDB-11336 from main missing here

e2243da

Add some comment clarification from CNDB-11356, the rest is already here

8e4909c

Missing add-opens in jvm17-server.options

9c9ad13

Update mockito/bytebuddy; Fix our pom to really use the bytebuddy.ver…

029259a

…sion provided

ekaterinadimitrova2 added 2 commits November 22, 2024 13:29

Fix broken commit from cherry-pick

bc28efa

Review feedback

a56afc0

ekaterinadimitrova2 force-pushed the CNDB-11508-5.0 branch from a4f42ec to a56afc0 Compare November 22, 2024 18:31

Fix build.xml for JDK22

699fcbc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CNDB-11508 main-5.0 branch #1408

CNDB-11508 main-5.0 branch #1408

ekaterinadimitrova2 commented Nov 7, 2024 •

edited

Loading

ekaterinadimitrova2 commented Nov 8, 2024 •

edited

Loading

JeremiahDJordan Nov 13, 2024

JeremiahDJordan Nov 13, 2024

ekaterinadimitrova2 Nov 13, 2024

jacek-lewandowski Nov 20, 2024

ekaterinadimitrova2 Nov 20, 2024

JeremiahDJordan Nov 13, 2024

ekaterinadimitrova2 Nov 13, 2024

JeremiahDJordan left a comment

ekaterinadimitrova2 commented Nov 13, 2024 •

edited

Loading

jacek-lewandowski Nov 20, 2024

ekaterinadimitrova2 Nov 20, 2024

jacek-lewandowski Nov 20, 2024

ekaterinadimitrova2 Nov 20, 2024 •

edited

Loading

jacek-lewandowski Nov 20, 2024

jacek-lewandowski Nov 20, 2024

jacek-lewandowski Nov 20, 2024

ekaterinadimitrova2 Nov 20, 2024

ekaterinadimitrova2 commented Nov 20, 2024

ekaterinadimitrova2 commented Nov 22, 2024 •

edited

Loading

ekaterinadimitrova2 commented Nov 22, 2024

sonarcloud bot commented Nov 22, 2024

cassci-bot commented Nov 22, 2024

CNDB-11508 main-5.0 branch #1408

Are you sure you want to change the base?

CNDB-11508 main-5.0 branch #1408

Conversation

ekaterinadimitrova2 commented Nov 7, 2024 • edited Loading

What is the issue

What does this PR fix and why was it fixed

Checklist before you submit for review

ekaterinadimitrova2 commented Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeremiahDJordan left a comment

Choose a reason for hiding this comment

ekaterinadimitrova2 commented Nov 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ekaterinadimitrova2 Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ekaterinadimitrova2 commented Nov 20, 2024

ekaterinadimitrova2 commented Nov 22, 2024 • edited Loading

ekaterinadimitrova2 commented Nov 22, 2024

sonarcloud bot commented Nov 22, 2024

Quality Gate passed

cassci-bot commented Nov 22, 2024

❌ Build ds-cassandra-pr-gate/PR-1408 rejected by Butler

Found 18 new test failures

Found 3 known test failures

ekaterinadimitrova2 commented Nov 7, 2024 •

edited

Loading

ekaterinadimitrova2 commented Nov 8, 2024 •

edited

Loading

ekaterinadimitrova2 commented Nov 13, 2024 •

edited

Loading

ekaterinadimitrova2 Nov 20, 2024 •

edited

Loading

ekaterinadimitrova2 commented Nov 22, 2024 •

edited

Loading