Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explore alternative to LBuffer for larger than 2GB buffers #9162

Closed
siddharthteotia opened this issue Aug 4, 2022 · 8 comments
Closed

Explore alternative to LBuffer for larger than 2GB buffers #9162

siddharthteotia opened this issue Aug 4, 2022 · 8 comments
Assignees

Comments

@siddharthteotia
Copy link
Contributor

siddharthteotia commented Aug 4, 2022

For off heap buffers (direct or mmap'd), we currently use Lbuffer library if the size is more than 2GB (which is supported for raw forward index by the v3 and v4 writer).

We have seen production issues on at least 2 occasions where a race condition in the library (map followed by unmap and then map) can lead to SIGSEGV where the address the code is trying to access is 3GB beyond the total virtual address space mapped for Pinot segments on the host where crash was observed.

Not easily reproducible but has happened twice in production.

Also, the library is no longer being maintained by the original author so may be there is benefit in writing our own allocator for large buffers.

TODO - add a link to the race condition issue and add more details from the production issues we observed.

@siddharthteotia siddharthteotia changed the title Explore alternative to LBuffer for handling larger than 2GB buffers Explore alternative to LBuffer for larger than 2GB buffers Aug 4, 2022
@richardstartin
Copy link
Member

Relates to #8529 as this library blocks JDK17 adoption.

@siddharthteotia
Copy link
Contributor Author

Thanks @richardstartin. Seems like JDK17 is also another motivation to consider moving away from this library.

cc @Jackie-Jiang @walterddr

@Jackie-Jiang
Copy link
Contributor

@siddharthteotia Have you figured out which part of the code can have race condition? We've run into similar issues (and I remember seeing SIGSEGV before at LI), but turns out it is caused by reading a segment that is already released. Is it possible that you are reading a released segment?

@gortiz
Copy link
Contributor

gortiz commented Aug 24, 2022

I would recommend to give a try to Chronicle bytes. We could use it to replace larray (and therefore be able to run with Java 17) and also to be able to map files larger than 4GBs.

Chronicle Bytes is part of the Chronicle HFT libraries and it is the only buffer library that supports 63 bits references that is maintained.

The other option would be to wait until Project Panama Foreign Memory Access API, but that may take a while and it would force use either to use modern JVMs (which doesn't seem acceptable) or to use multi-release jars (which doesn't seem easy right now given the amount of shading we use)

@gortiz
Copy link
Contributor

gortiz commented Nov 25, 2022

I've tried Chronicle Bytes in #9842, but I after fixing some errors for a couple of days, I would recommend to do not use it. Some design decision are quite different to what someone using ByteBuffers or LArrays would expect (which is even worse when the methods are not properly documented), some features are not present (like copying to a ByteBuffer from a given offset), some others are not consistent (OpenHFT/Chronicle-Bytes#478), some mistakes derive in the JVM been killed, etc. I think it is an amazing library, but it is not maintained as a generic byte buffer library but very focused on how people from OpenHFT use it, which I totally understand but would require some time inversion from us to be able to use it.

On the other hand, I've been trying to understand why LArray cannot be used in Java 17 and it seems to be related to the way @xerial creates ByteBuffers views. To do so, LArray uses a private constructor that has changed from in Java 17: A new parameter has been added. Xerial has already dealt with that issue here so I think it is feasible to do it in LArray. That would be fantastic because it would allow us to run with modern JVMs without needing to change a bit of code.

I'm still thinking that a proper Foreign Memory Access API (like JEP-424](https://openjdk.org/jeps/424)) is the way to go, but given that the solution is still not stable, the best we can do is to help Xerial to update his library.

@gortiz
Copy link
Contributor

gortiz commented May 26, 2023

I've open #10783 as a PEP and #10528 as a PR in order to be able to run with Java 17 (and 21!)

@gortiz
Copy link
Contributor

gortiz commented Sep 22, 2023

We may want to close this issue

@gortiz
Copy link
Contributor

gortiz commented Sep 22, 2023

I think this other issue/PEP may be interesting for readers: #11656

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants