Skip to content

Remove LArray usage#15342

Merged
gortiz merged 1 commit intoapache:masterfrom
gortiz:remove-larray
Mar 25, 2025
Merged

Remove LArray usage#15342
gortiz merged 1 commit intoapache:masterfrom
gortiz:remove-larray

Conversation

@gortiz
Copy link
Contributor

@gortiz gortiz commented Mar 21, 2025

This PR removes the dependency and all usages of LArray, as the last final PR to implement #12810.

As mentioned, LArray is the library we used to use to support buffers longer than 2GBs. However, it has several issues, and our library has replaced it. In the future, we would like to work directly on top of the new standard Foreign Memory API introduced in Java 22.

This PR is backward incompatible, although most users should not notice the change. In Pinot 1.2.0, we used our library when running on Java 17 and 21, and in Pinot 1.3.0, we made our library the default in all deployments. The only way users could be using larray is by changing pinot.offheap.buffer.factory. In Pinot 1.4.0 this configuration won't support the value larray.

The buffer API used by Pinot can be configured with plugins (as shown in #12809). In case some user actually wants to keep using LArray, they could create their own plugin using the code that is being removed here. I would strongly recommend against that (as it is discussed in #12810), but I just wanted to mention this in the strange case someone actually prefer LArray.

@gortiz gortiz mentioned this pull request Aug 28, 2024
2 tasks
@gortiz gortiz requested a review from Jackie-Jiang March 21, 2025 07:59
@gortiz gortiz added incompatible Indicate PR that introduces backward incompatibility cleanup labels Mar 21, 2025
@gortiz gortiz mentioned this pull request Mar 21, 2025
@gortiz gortiz requested a review from siddharthteotia March 21, 2025 08:09
@codecov-commenter
Copy link

codecov-commenter commented Mar 21, 2025

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 63.59%. Comparing base (59551e4) to head (23d27be).
⚠️ Report is 2777 commits behind head on master.

Files with missing lines Patch % Lines
...nt/spi/memory/unsafe/UnsafePinotBufferFactory.java 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #15342      +/-   ##
============================================
+ Coverage     61.75%   63.59%   +1.83%     
- Complexity      207     1373    +1166     
============================================
  Files          2436     2780     +344     
  Lines        133233   156868   +23635     
  Branches      20636    24082    +3446     
============================================
+ Hits          82274    99753   +17479     
- Misses        44911    49589    +4678     
- Partials       6048     7526    +1478     
Flag Coverage Δ
custom-integration1 100.00% <ø> (+99.99%) ⬆️
integration 100.00% <ø> (+99.99%) ⬆️
integration1 100.00% <ø> (+99.99%) ⬆️
integration2 0.00% <ø> (ø)
java-11 63.56% <0.00%> (+1.85%) ⬆️
java-21 63.58% <0.00%> (+1.95%) ⬆️
skip-bytebuffers-false 63.58% <0.00%> (+1.83%) ⬆️
skip-bytebuffers-true 63.55% <0.00%> (+35.82%) ⬆️
temurin 63.59% <0.00%> (+1.83%) ⬆️
unittests 63.58% <0.00%> (+1.83%) ⬆️
unittests1 56.05% <0.00%> (+9.16%) ⬆️
unittests2 34.24% <0.00%> (+6.51%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gortiz gortiz merged commit 526e7b7 into apache:master Mar 25, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup incompatible Indicate PR that introduces backward incompatibility

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants