After update to Lucene 9.4 use `--enable-preview` on Java==19 (exact) to allow mmap use new JDK APIs

[uschindler]

Description

Apache Lucene 9.4 will have support for Java 19 Panama APIs to mmap index files (using a MR-JAR). See apache/lucene#912 for more information.

As those APIs are not yet enabled by default in the JDK, we have to still use some opt-in approach, controlled by Java's command line:

• Lucene by default uses the old implementation using MappedByteBuffer and several hacks which may also risk crushing the JDK if an open index is closed from another thread while a search is running (this is well known). If Java 19 is detected, Lucene will log a warning through JUL when MMapDirectory is initialized (see below).
• If you pass --enable-preview to the Java command line (next to heap settings), it will enable preview APIs in JDK (https://openjdk.org/jeps/12). Lucene detects this and switches MMapDirectory and uses a new implementation MemorySegmentIndexInput for the inputs to use those new APIs (at moment it will also log this as "info" message to JUL). The new APIs are safe and can no longer crush the JVM. But most importantly, all index files are now mapped in portions of 16 GiB instead of 1 GiB into memory. In fact, unless an index is force-merged to one segment, all index files will then consist only of one memory mapping spawning the whole file! This will help Hotspot to further optimize reading as only one implementation on MemorySegmentIndexInput ist used. In addition, because the number of mappings is dramatically reduced (approximately 5 times less mappings, because the maximum segment size is 5 Gigabytes by default and all such segments now use one instead of 5 mappings). This may allow users to no longer change sysctl (see max_map_count @ https://opensearch.org/docs/latest/opensearch/install/important-settings/) and go with defaults of OS. On the other hand users may host more indexes with many more segments on one node.

Some TODOs:

• Make sure that Opensearch also redirects stuff logged via java.util.logging (JUL) to its own log file, so they do not land in console. This can be done with log4j by adding the log4j-jul adapter and install it using a system property in the Bootstrap classes. I have not checked if this is already done. The reason for this is that Apache Lucene now logs some events using java.util.logging since Lucene 9.0. Some of those events are MMapDirectory messages (e.g., when unmapping was not working) or few others like some module system settings are incorrect. Logging is very seldom, but for this feature it will definitely log using JUL, so it would be good to make sure Opensearch redirects JUL logging correctly to its own loggers. This could be a separate issue!
• The Opensearch startup script should pass --enable-preview as command line flag if exactly Java 19 is used to start up Opensearch. If this is not done, a warning gets logged (see above).

Important: Lucene 9.4 only supports this on Java 19 (exactly), because the APIs are in flux. If you start with Java 20, it falls back to the classical MMapDirectory. We will add support for Java 20 in a later release. The reason for this is that the class files of new implementation are marked by some special version numbers that make them ONLY compatible to Java 19, not earlier or later, to allow the JDK to apply changes to the API before final release in Java 21. But passing --enable-preview to later versions won't hurt, so maybe enable it on all versions >= 19.

A last note: The downside of this new code is that closing and unmapping an index file gets more heavy (it will trigger an safepoint in the JVM). We have not yet found out how much this impacts servers opening/closing index files a lot. Because of this we would really like Amazon/Opensearch to do benchmarking on this, ideally if their users and customers could optionally enable it. But benchmarking should be done now, because with hopefully Java 21, Lucene will use the new implementation by default. Java 20 will be the second and last preview round.

[dblock]

Thanks for the detailed proposal. Alongside these changes I think we need to start running at least some CI on both JDK 17 and JDK19. Currently we run 11 and 17 via https://github.com/opensearch-project/OpenSearch/blob/main/buildSrc/version.properties#L5.

[uschindler]

Ah OK, I did not know which versions you are running. Because the sister project ES is already on JDK 19 for testing and bundles 18 at moment.

[reta]

We did some work to support JDK-18 [1], somehow overlooked JDK-19, @dblock I can take this part, will update CI images, etc.

[1] #1710

[dblock]

@reta What do you think about extracting a small subset of tests (not ITs for example) to run on GHA, so we can make the JDK matrix easy and visible in the repo, just like we do for plugins? (e.g. this one). Something more than precommit but less than gradle check? and reducing gradle check to ITs

[reta]

@reta What do you think about extracting a small subset of tests (not ITs for example) to run on GHA, so we can make the JDK matrix easy and visible in the repo, just like we do for plugins? (e.g. this one). Something more than precommit but less than gradle check? and reducing gradle check to ITs

@dblock certainly +1 to give it a try, I think we should be able to run everything on GHA but starting with subset is probably a good idea.

[uschindler]

Of course when Lucene comes out tomorrow (hopefully, the schedule suggests it, but something bad can always happen), please update Lucene to 9.4.0 first. I have no idea at which stage the snapshot currently used is: 9.4.0-snapshot-ddf0d0a

[dblock]

@uschindler Yes, of course, but don't let us stop you :)

Read more about Lucene Snapshots here: https://github.com/opensearch-project/OpenSearch/blob/main/DEVELOPER_GUIDE.md#lucene-snapshots

[dbwiddis]

+1 on this. I've been playing around with JDK19 and there's some great performance gains to be had. Downside is the temporary nature of anything we do, as we'll have to tweak it for 20 and then 21, but will be good to work out the kinks with an opt-in.

[uschindler]

@dbwiddis You can use a similar approach for/with JNA vs. panama-foreign like we do by using an MR-JAR and adding some try/catch logic when loading the classes.

[dbwiddis]

You can use a similar approach for/with JNA vs. panama-foreign like we do by using an MR-JAR

@uschindler it's less the practical aspect (it can be done) but more the future question of "what do we do a year from now when JDK 21 LTS is out". Or even 6 months from now when JDK 20 is out and JDK 19 is EOL and no longer receiving updates.

Are we going to continue to "support" older versions that are not getting security updates (which I might suggest are more likely with native memory access than other features)? Do we back out the "19" branch from the MRJAR? If it's the same API in 20 but there's a security fix in 20 that's not in 19, which version do we enforce in the MRJAR?

I'm all for trying to enable features (early) that we think will be in JDK 21 (with some possible tweaks to the API) but I think it should be with the expectation that we will flip 19 to 20, and then 21, and not make any attempt to "support" EOL versions.

[uschindler]

Hi,
My plan is to have another Gradle SourceSet in Lucene for 20 and then one for 21. The logic to load will be the same (no changes needed).

If I figure out that in JDK 20 there are no API changes (most likely for the part of Panama we use), we may just add a Gradle task to copy the v19 class files to the v20 MR-JAR folder while patching classfile major and preview minor version (directly or via ASM visitor). This spares us to autoprovision JDK20. I will see what works. When JDK 21 LTS comes out we may drop support for the 19 and 20 in newer Lucene versions and use a plain MR-JAR variant without any preview supporting logic. But this would not break any code here. JDK 19 would fallback to the old code. 🤣

In short: I will support 20 in addition later.

[dbwiddis]

When JDK 21 LTS comes out we may drop support for the 19 and 20 in newer Lucene versions

I think that aligns with my thinking. Of course I'm not the one to convince. :-)