Use Lucene101 postings format by default #126080

jordan-powers · 2025-04-01T20:36:52Z

With this change, new standard indices will use the Lucene101PostingsFormat instead of the current default ES812PostingsFormat.

The current implementation enables the new format for all standard indices. We might consider instead only enabling the new format only for non-datastream standard indices, and/or only when index.codec is not set to best_compression.

Currently, use of the new codec is gated behind a feature flag.

We may or may not decide to backport this to 8.x depending on how the nightly benchmarks look.

Closes #119051

elasticsearchmachine · 2025-04-01T23:24:32Z

Pinging @elastic/es-storage-engine (Team:StorageEngine)

martijnvg

Left one comment, LGTM otherwise.

martijnvg · 2025-04-02T09:10:18Z

server/src/test/java/org/elasticsearch/index/codec/PerFieldMapperCodecTests.java

-        assertThat(perFieldMapperCodec.getPostingsFormatForField("another_field"), instanceOf(ES812PostingsFormat.class));
+        assertThat(
+            perFieldMapperCodec.getPostingsFormatForField("another_field"),
+            instanceOf(timeSeries ? ES812PostingsFormat.class : Lucene101PostingsFormat.class)


I think these tests fail if build is in release mode. Can you add test-release label to this PR?
I think we should have an if check here. If feature flag is enabled we expect the lucene 101 posting format otherwise the es812 format.

javanna · 2025-04-02T14:07:13Z

Do we need to also test full bwc on this PR just to make sure? Backwards compatibility is handled via the new index version and applying the new codec on write only for newly created indices, correct? Do we need any additional testing compared to what we already have?

martijnvg · 2025-04-02T14:35:03Z

Do we need to also test full bwc on this PR just to make sure?

I think we can also add the test-full-bwc label here.

Backwards compatibility is handled via the new index version and applying the new codec on write only for newly created indices, correct?

This isn't gated behind an index version (for now just feature flag). Any non tsdb/logsdb index (including existing indices) will start to use the new codec for new segments. The old segments remain to be used by the previous posting format. The codec name is stored in disk and that is used to load codec via service provider.

Do we need any additional testing compared to what we already have?

I think current mixed cluster / rolling upgrade integration testing is sufficient? We don't modify an existing posting format, just use a different one.

jordan-powers · 2025-04-02T15:20:41Z

This isn't gated behind an index version (for now just feature flag). Any non tsdb/logsdb index (including existing indices) will start to use the new codec for new segments.

Actually, I did add a new index version and I check the index version when deciding which postings format to use. I wasn't sure about backwards compatibility--would lucene still read the old indices correctly if it's expecting the new postings format? If so, I can remove the check

martijnvg · 2025-04-02T15:26:06Z

Actually, I did add a new index version and I check the index version when deciding which postings format to use. I wasn't sure about backwards compatibility--would lucene still read the old indices correctly if it's expecting the new postings format? If so, I can remove the check

Whoops, you're right. I think we can remove the index version check. I don't gating behind index version is needed, given that changing codec on per field level should work (as long as the codec remains available in codebase).

…-format

javanna · 2025-04-02T18:56:30Z

Right, getPostingsFormatPerField is called on write. What has been already written has the old codec name on disk, and the appropriate formats will be loaded via SPI, they just need to be available in the classpath and registered to SPI, which is the case. This does mean that existing indices will get new segments written with the new format, if that's what you folks want to achieve here, as opposed to starting to use the new format only for new indices. I agree that if there is a feature flag, the index version is probably redundant and we should retain compatibility enabling and disabling the feature flag. Do we test that? Probably not, but I guess it does not matter as we won't enable the feature flag in prod?

jordan-powers · 2025-04-02T19:58:15Z

This does mean that existing indices will get new segments written with the new format

I think that's the goal.

we should retain compatibility enabling and disabling the feature flag

Could you clarify what you mean by this? The feature flag is just there because we're not sure we want to push this to prod yet. Once we've benchmarked it and we're ready to make it generally available, I expect we'll remove the feature flag so that it's always enabled. It's not intended to be toggled on/off depending on compatibility.

My understanding is that compatibility is guaranteed because the formatted postings include the name of the format class, so when they're loaded, the service provider knows which class to use to decode them. Sure, if we need to write new postings to disk, they'll always be in the new format so indices might end up with a mix of different posting formats, but that's fine because each written postings file includes the codec name.

we won't enable the feature flag in prod?

Right, at this point we just want to see the effect on the nightly benchmarks

…-format

martijnvg · 2025-04-03T12:42:46Z

I chatted with Luca and we both agree that when the feature flag gets removed we should introduce an index version, so that only new indices will use 101 postings codec. This makes reasoning about which postings format an index uses easier.

This reverts commit 2637976.

…-format

jordan-powers · 2025-04-03T14:53:33Z

Ok, I'll reintroduce the index version check then.

jordan-powers · 2025-04-03T17:14:11Z

Release tests are failing due to #124263. Waiting for that fix to be merged before I merge this PR.

jordan-powers · 2025-04-03T19:54:19Z

Actually seems that #124263 won't fix the problem. Issue is being tracked in #126086. Will wait for resolution before merging this PR.

…-format

This reverts commit 4c174a8.

ChrisHegarty · 2025-05-20T16:10:31Z

server/src/main/java/org/elasticsearch/index/codec/PerFieldFormatSupplier.java

@@ -31,19 +34,33 @@
 * vectors.
 */
 public class PerFieldFormatSupplier {
+    public static final FeatureFlag USE_LUCENE101_POSTINGS_FORMAT = new FeatureFlag("use_lucene101_postings_format");


@jordan-powers @martijnvg Lucene 10.3 (still in development) will bring a new postings format. Is the intention that this feature flag to select the "latest" codec? Or is it really specific to 10.1? I ask only because we need to determine if a use_lucene101_postings_format is warranted or maybe make this flag more general?

The intent is the use the latest default lucene postings format. Unfortunately the name doesn't reflect that, but we want it to use the latest default postings format.

Change PerFieldFormatSupplier to get default postings from current codec being used. This way the default codec for standard index mode isn't tied to Lucene101PostingsFormat specifically. And rename the feature flag that controls when stock Lucene posting format is used. Addressed comment from elastic#126080

…rmat is used. Addressed comment from elastic#126080

…rmat is used (#128236) Addressed comment from #126080

jordan-powers added 2 commits April 1, 2025 13:05

Use lucene 10.1 postings format in some cases

994b83a

Enable lucene postings format feature flag for rest tests

0b3e039

jordan-powers added >non-issue :StorageEngine/Codec v9.1.0 labels Apr 1, 2025

jordan-powers self-assigned this Apr 1, 2025

jordan-powers marked this pull request as ready for review April 1, 2025 23:24

jordan-powers requested a review from martijnvg April 1, 2025 23:24

elasticsearchmachine added the Team:StorageEngine label Apr 1, 2025

martijnvg approved these changes Apr 2, 2025

View reviewed changes

jordan-powers added test-full-bwc Trigger full BWC version matrix tests test-release Trigger CI checks against release build labels Apr 2, 2025

jordan-powers added 4 commits April 2, 2025 08:26

Check feature flag in PerFieldMapperCodecTests

2495aaa

Merge remote-tracking branch 'upstream/main' into use-lucene-postings…

c642fb7

…-format

Remove USE_LUCENE101_POSTINGS_FORMAT index version

2637976

Merge remote-tracking branch 'upstream/main' into use-lucene-postings…

91f7ea8

…-format

jordan-powers added 2 commits April 2, 2025 14:07

Merge remote-tracking branch 'upstream/main' into use-lucene-postings…

375cf2d

…-format

Merge remote-tracking branch 'upstream/main' into use-lucene-postings…

c610ab8

…-format

jordan-powers added 2 commits April 3, 2025 07:52

Revert "Remove USE_LUCENE101_POSTINGS_FORMAT index version"

a803d4c

This reverts commit 2637976.

Merge remote-tracking branch 'upstream/main' into use-lucene-postings…

09aa306

…-format

Merge remote-tracking branch 'upstream/main' into use-lucene-postings…

9cfb46f

…-format

jordan-powers merged commit 4c174a8 into elastic:main Apr 4, 2025
19 checks passed

jordan-powers deleted the use-lucene-postings-format branch April 4, 2025 19:41

jordan-powers added a commit to jordan-powers/elasticsearch that referenced this pull request Apr 16, 2025

Revert "Use Lucene101 postings format by default (elastic#126080)"

38614c8

This reverts commit 4c174a8.

jordan-powers mentioned this pull request Apr 16, 2025

Revert "Use Lucene101 postings format by default" #126932

Closed

martijnvg mentioned this pull request May 7, 2025

Move to Lucene 9.12's new PostingsFormat. #115021

Open

5 tasks

ChrisHegarty reviewed May 20, 2025

View reviewed changes

martijnvg mentioned this pull request May 21, 2025

Renamed the feature flag that controls when default Lucene posting format is used #128236

Merged

martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request May 21, 2025

Renamed the feature flag that controls when default Lucene posting fo…

a60be8e

…rmat is used. Addressed comment from elastic#126080

martijnvg added a commit that referenced this pull request May 21, 2025

Renamed the feature flag that controls when default Lucene posting fo…

14355a7

…rmat is used (#128236) Addressed comment from #126080

Use Lucene101 postings format by default #126080

Use Lucene101 postings format by default #126080

Uh oh!

Conversation

jordan-powers commented Apr 1, 2025

Uh oh!

elasticsearchmachine commented Apr 1, 2025

Uh oh!

martijnvg left a comment

Choose a reason for hiding this comment

Uh oh!

martijnvg Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

javanna commented Apr 2, 2025

Uh oh!

martijnvg commented Apr 2, 2025

Uh oh!

jordan-powers commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

martijnvg commented Apr 2, 2025

Uh oh!

javanna commented Apr 2, 2025

Uh oh!

jordan-powers commented Apr 2, 2025

Uh oh!

martijnvg commented Apr 3, 2025

Uh oh!

jordan-powers commented Apr 3, 2025

Uh oh!

jordan-powers commented Apr 3, 2025

Uh oh!

jordan-powers commented Apr 3, 2025

Uh oh!

Uh oh!

ChrisHegarty May 20, 2025

Choose a reason for hiding this comment

Uh oh!

martijnvg May 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jordan-powers commented Apr 2, 2025 •

edited

Loading