Java 21 pipeline

[gortiz]

The reason to be of this PR is to be able to progress on #11656.

Specifically it:

• Includes Do not execute spotless in Java 21 #11670
• Includes Move scala dependencies to root pom #11671
• Adds 2 new tests pipelines:
  • One with Java 21.
  • One with Java 21 that always use the new buffer api. Normally this API is only used when buffers are larger than 2GBs. The idea of this execution is to always use this API (even with small buffers) to increase the confidence on the library.

[codecov-commenter]

Codecov Report

Merging #11672 (6b3b853) into master (9378b06) will decrease coverage by 0.02%.
The diff coverage is 70.00%.

@@             Coverage Diff              @@
##             master   #11672      +/-   ##
============================================
- Coverage     66.48%   66.47%   -0.02%     
  Complexity      207      207              
============================================
  Files          2343     2343              
  Lines        126418   126422       +4     
  Branches      19440    19443       +3     
============================================
- Hits          84054    84035      -19     
- Misses        36499    36519      +20     
- Partials       5865     5868       +3     

Flag	Coverage Δ
custom-integration1	<0.01% <0.00%> (ø)
integration	<0.01% <0.00%> (ø)
integration1	<0.01% <0.00%> (ø)
integration2	0.00% <0.00%> (ø)
java-11	50.21% <50.00%> (-16.22%)	⬇️
java-17	?
java-20	?
java-21	66.35% <50.00%> (?)
skip-bytebuffers-false	66.45% <70.00%> (?)
skip-bytebuffers-true	66.31% <50.00%> (?)
temurin	66.47% <70.00%> (-0.02%)	⬇️
unittests	66.47% <70.00%> (-0.02%)	⬇️
unittests1	67.33% <70.00%> (-0.02%)	⬇️
unittests2	17.70% <0.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...ache/pinot/segment/spi/memory/PinotDataBuffer.java	78.89% <70.00%> (-0.55%)	⬇️

... and 24 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[xiangfu0]

This module is still called spark pinot-batch-ingestion-spark-3.2 :p
I think we can either change it to pinot-batch-ingestion-spark-3?

[gortiz]

Yep. I discussed about that in #11701 and its related issue. I'm ok with changing that name

[xiangfu0]

here are too many CI checks, can we try to reduce the number of tests?

[gortiz]

here are too many CI checks, can we try to reduce the number of tests?

Sure, we need to discuss which ones we want to have.

My recommendation right now is:

1. Java 11, skip bytebuffers false
2. Java 17, skip bytebuffers false
3. Java 21, skip bytebuffers false
4. Java 21, skip bytebuffers true

Java 11 with skipbuffers true should also be enable, but given it doesn't pass the tests right now, we need to skip it. What do you think?

[xiangfu0]

here are too many CI checks, can we try to reduce the number of tests?

Sure, we need to discuss which ones we want to have.

My recommendation right now is:

1. Java 11, skip bytebuffers false
2. Java 17, skip bytebuffers false
3. Java 21, skip bytebuffers false
4. Java 21, skip bytebuffers true

Java 11 with skipbuffers true should also be enable, but given it doesn't pass the tests right now, we need to skip it. What do you think?

I think we can just test the default byteBuffer usage.
For the others, if you have good tests, we can just write a test suite for testing different behaviors to ensure the coverage.

[gortiz]

I think we can just test the default byteBuffer usage.
For the others, if you have good tests, we can just write a test suite for testing different behaviors to ensure the coverage.

PinotDataBufferTest already does that... but as is proven by this PR, it is not enough. There are some particularities in LArray that are not tested there. Given the importance of the buffer library (it may imply incorrect results or data corruption), I think it is worth it to have at least one execution where the unsafe buffer is tested.

But we can skip it if you think it is not necessary

[gortiz]

I've changed the pipeline to run with:

• Java 11, with byte buffers
• Java 21, with byte buffers
• Java 21, without byte buffers

So we have the same number of executions as before, but Java 17 and 20 have been replaced with Java 21 with and without bytebuffers. What do you think?

[xiangfu0]

I think we can just test the default byteBuffer usage.
For the others, if you have good tests, we can just write a test suite for testing different behaviors to ensure the coverage.

PinotDataBufferTest already does that... but as is proven by this PR, it is not enough. There are some particularities in LArray that are not tested there. Given the importance of the buffer library (it may imply incorrect results or data corruption), I think it is worth it to have at least one execution where the unsafe buffer is tested.

But we can skip it if you think it is not necessary

Make sense, let's enable this for now and we can disable the tests once we have enough confidence.

[xiangfu0]

Seems there is a missing rebase for: #11804
I separated PR tests files.

[xiangfu0]

Also one thing missing there is to add more codecov flags for skip buffer