Bump Apache Lucene from 10.3.2 to 10.4.0

[jainankitk]

Description

Bumps up Apache Lucene to 10.4.0. Merging this change from the feature/3.x-lucene branch

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 0b378f6.

Path	Line	Severity	Description
gradle/libs.versions.toml	3	medium	Dependency upgraded to snapshot/pre-release version (10.4.0-snapshot-a8c1455) instead of stable release. Snapshot versions may contain untested code and could be modified in the repository. Verify the snapshot source and commit hash a8c1455 corresponds to legitimate Lucene development, and consider using an official release version for production.

The table above displays the top 10 most important findings.

Total: 1 | Critical: 0 | High: 0 | Medium: 1 | Low: 0

---

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

---

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit c72c3e9)

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

✅ No TODO sections

🔀 Multiple PR themes Sub-PR theme: Update Lucene version constants and dependencies Relevant files: libs/core/src/main/java/org/opensearch/Version.java server/src/main/java/org/opensearch/common/lucene/Lucene.java gradle/libs.versions.toml CHANGELOG.md Sub-PR theme: Migrate codec implementations to Lucene104 Relevant files: server/src/main/java/org/opensearch/index/codec/CodecService.java server/src/main/java/org/opensearch/index/codec/PerFieldMappingPostingFormatCodec.java server/src/main/java/org/opensearch/index/codec/composite/CompositeCodecFactory.java server/src/main/java/org/opensearch/index/codec/composite/composite104/Composite104Codec.java server/src/main/java/org/opensearch/index/codec/composite/composite104/package-info.java server/src/main/java/org/opensearch/index/codec/composite/backward_codecs/composite103/Composite103Codec.java server/src/main/java/org/opensearch/index/codec/composite/backward_codecs/composite103/package-info.java server/src/main/java/org/opensearch/index/mapper/CompletionFieldMapper.java server/src/main/resources/META-INF/services/org.apache.lucene.codecs.Codec Sub-PR theme: Update engine and field data for Lucene API changes Relevant files: server/src/main/java/org/opensearch/index/engine/Engine.java server/src/main/java/org/opensearch/index/engine/NRTReplicationReaderManager.java server/src/main/java/org/opensearch/index/engine/NoOpEngine.java server/src/main/java/org/opensearch/index/fielddata/plain/NonPruningSortedSetOrdinalsIndexFieldData.java

⚡ Recommended focus areas for review Breaking Change The constructor Composite103Codec(Lucene103Codec.Mode compressionMode, MapperService mapperService, Logger logger) was removed. This is a breaking change that may affect backward compatibility with existing indices using Composite103Codec. Verify that existing indices can still be read and that the migration path is properly handled. /** API Change Added a new null parameter to StandardDirectoryReader.open() call. This appears to be a Lucene API change. Verify that passing null for this new parameter is the correct behavior and doesn't introduce any issues with segment reading. try (DirectoryReader innerReader = StandardDirectoryReader.open(store.directory(), segmentInfos, null, null, null)) { API Change Added two new parameters to StandardDirectoryReader.open() including engineConfig.getLeafSorter() and null. Verify that these parameters are correctly set and that the null parameter doesn't cause unexpected behavior. engineConfig.getLeafSorter(), null Missing Implementation Four new doOpenIfChanged method overloads with ExecutorService parameter were added, all returning null. Verify whether these methods should have actual implementations or if returning null is the intended behavior for NoOpEngine. @Override protected DirectoryReader doOpenIfChanged(ExecutorService executorService) { return null; } @Override protected DirectoryReader doOpenIfChanged(IndexCommit commit) { return null; } @Override protected DirectoryReader doOpenIfChanged(IndexCommit commit, ExecutorService executorService) { return null; } @Override protected DirectoryReader doOpenIfChanged(IndexWriter writer, boolean applyAllDeletes) { return null; } @Override protected DirectoryReader doOpenIfChanged(IndexWriter writer, boolean applyAllDeletes, ExecutorService executorService) { return null; } API Removal Removed setBytesComparator and getBytesComparator methods. This appears to be a Lucene API change. Verify that no code depends on these methods and that the removal doesn't break any sorting functionality. @Override public FieldComparator<?> getComparator(int numHits, Pruning pruning) { return delegate.getComparator(numHits, pruning);

[github-actions]

❌ Gradle check result for b123252: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 4b7978e

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to c72c3e9

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Verify backward codec compatibility The Composite103Codec has been moved to the backward_codecs package and now imports Lucene103Codec from org.apache.lucene.backward_codecs.lucene103 instead of the main codecs package. Verify that the backward codec implementation maintains full compatibility with existing indices created with the original Composite103Codec, as this is critical for index backward compatibility during upgrades. server/src/main/java/org/opensearch/index/codec/composite/backward_codecs/composite103/Composite103Codec.java [9-17] package org.opensearch.index.codec.composite.backward_codecs.composite103; import org.apache.lucene.backward_codecs.lucene103.Lucene103Codec; import org.apache.lucene.codecs.Codec; import org.apache.lucene.codecs.DocValuesFormat; import org.apache.lucene.codecs.FilterCodec; import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.index.codec.composite.composite912.Composite912DocValuesFormat; import org.opensearch.index.mapper.MapperService; +// Ensure backward compatibility with indices created using Lucene 10.3.x + Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that Composite103Codec has been moved to the backward_codecs package and now imports from org.apache.lucene.backward_codecs.lucene103. This is an important observation about backward compatibility. However, the suggestion only asks to verify compatibility rather than identifying a concrete issue, and the 'improved_code' merely adds a comment without substantive changes.	Medium
General	Document null ExecutorService parameter The StandardDirectoryReader.open() call now includes an additional null parameter. Verify that this new parameter (likely an ExecutorService) is intentionally set to null and document why synchronous operation is preferred here, or consider whether parallel segment loading would improve performance for this operation. server/src/main/java/org/opensearch/index/engine/Engine.java [312] +// Using null ExecutorService for synchronous segment loading during max seq no retrieval try (DirectoryReader innerReader = StandardDirectoryReader.open(store.directory(), segmentInfos, null, null, null)) { Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies the addition of a new null parameter to StandardDirectoryReader.open(). While documenting this choice could improve code clarity, the suggestion only asks to verify and document the decision rather than identifying a concrete problem. The 'improved_code' adds a comment but doesn't change functionality.	Low
	Evaluate parallel segment loading option The StandardDirectoryReader.open() now requires an additional null parameter. Consider whether NRT replication could benefit from parallel segment loading by passing an appropriate ExecutorService instead of null, especially for large indices where segment loading performance is critical. server/src/main/java/org/opensearch/index/engine/NRTReplicationReaderManager.java [83-88] +// Consider using ExecutorService for parallel segment loading in NRT replication final DirectoryReader innerReader = StandardDirectoryReader.open( referenceToRefresh.directory(), currentInfos, subs, engineConfig.getLeafSorter(), - null + null // TODO: Evaluate if parallel loading improves NRT replication performance ); Suggestion importance[1-10]: 4 __ Why: The suggestion identifies the new null parameter and proposes considering an ExecutorService for parallel loading. However, this is a performance optimization suggestion rather than a correctness issue. The 'improved_code' only adds a TODO comment without implementing any change, and the suggestion doesn't provide evidence that parallel loading would be beneficial in this context.	Low

---

Previous suggestions

Suggestions up to commit 47ae628

Category	Suggestion	Impact
Possible issue	Verify backward codec compatibility The Composite103Codec has been moved to the backward codecs package and now imports Lucene103Codec from org.apache.lucene.backward_codecs.lucene103 instead of the main codecs package. Verify that the backward codec implementation maintains full compatibility with existing indices that were created with the original Composite103Codec to prevent index corruption or read failures. server/src/main/java/org/opensearch/index/codec/composite/backward_codecs/composite103/Composite103Codec.java [9-18] package org.opensearch.index.codec.composite.backward_codecs.composite103; import org.apache.lucene.backward_codecs.lucene103.Lucene103Codec; import org.apache.lucene.codecs.Codec; import org.apache.lucene.codecs.DocValuesFormat; import org.apache.lucene.codecs.FilterCodec; import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.index.codec.composite.composite912.Composite912DocValuesFormat; import org.opensearch.index.mapper.MapperService; +// Add validation to ensure backward compatibility + Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that Composite103Codec has been moved to the backward codecs package and now imports from org.apache.lucene.backward_codecs.lucene103. This is an important observation about maintaining backward compatibility with existing indices. However, the suggestion only asks for verification without providing concrete implementation guidance, limiting its actionable value.	Medium
General	Review null ExecutorService parameter The StandardDirectoryReader.open() call now includes an additional null parameter. Ensure this new parameter (likely an ExecutorService) is intentionally set to null and that this doesn't disable any parallel processing capabilities that might be beneficial for performance during segment reading operations. server/src/main/java/org/opensearch/index/engine/Engine.java [312] -try (DirectoryReader innerReader = StandardDirectoryReader.open(store.directory(), segmentInfos, null, null, null)) { +// Consider providing an appropriate ExecutorService if parallel processing is desired +try (DirectoryReader innerReader = StandardDirectoryReader.open(store.directory(), segmentInfos, null, null, executorService)) { Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies the addition of a fifth null parameter to StandardDirectoryReader.open() and raises a valid concern about whether this intentionally disables parallel processing. However, it only asks for verification rather than identifying a definite issue, and the improved_code proposes using an undefined executorService variable that doesn't exist in the context.	Low
	Evaluate ExecutorService for NRT operations The StandardDirectoryReader.open() method now accepts an additional parameter (set to null). Verify that passing null for this parameter (likely an ExecutorService) doesn't negatively impact NRT replication performance, especially for large segment refreshes where parallel processing could be beneficial. server/src/main/java/org/opensearch/index/engine/NRTReplicationReaderManager.java [83-88] final DirectoryReader innerReader = StandardDirectoryReader.open( referenceToRefresh.directory(), currentInfos, subs, engineConfig.getLeafSorter(), - null + engineConfig.getExecutorService() // Consider using configured executor ); Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies the new null parameter in StandardDirectoryReader.open() and raises a valid performance consideration for NRT replication. However, the improved_code proposes using engineConfig.getExecutorService() which may not exist as a method, and the suggestion is primarily asking for evaluation rather than identifying a concrete issue.	Low

Suggestions up to commit a739e0e

Category	Suggestion	Impact
Possible issue	Verify backward codec import path The Composite103Codec now uses Lucene103Codec from the backward codecs package, but the import statement may not have been updated. Verify that the import points to org.apache.lucene.backward_codecs.lucene103.Lucene103Codec instead of the removed org.apache.lucene.codecs.lucene103.Lucene103Codec to ensure backward compatibility works correctly. server/src/main/java/org/opensearch/index/codec/composite/backward_codecs/composite103/Composite103Codec.java [11] +// Ensure import is: import org.apache.lucene.backward_codecs.lucene103.Lucene103Codec; public Composite103Codec() { this(COMPOSITE_INDEX_CODEC_NAME, new Lucene103Codec(), null); } Suggestion importance[1-10]: 8 __ Why: The PR moves Composite103Codec to the backward codecs package and changes the import from org.apache.lucene.codecs.lucene103.Lucene103Codec to org.apache.lucene.backward_codecs.lucene103.Lucene103Codec. This is a critical change for backward compatibility with Lucene 10.4.0, and the suggestion correctly identifies the need to verify this import path change.	Medium
General	Validate new API parameter usage The StandardDirectoryReader.open() call now includes an additional null parameter (5th parameter). Verify that this new parameter is correctly positioned and represents the intended argument (likely an ExecutorService) according to Lucene 10.4.0 API changes. server/src/main/java/org/opensearch/index/engine/Engine.java [312] +// Verify the 5th parameter represents ExecutorService and null is the intended value try (DirectoryReader innerReader = StandardDirectoryReader.open(store.directory(), segmentInfos, null, null, null)) { Suggestion importance[1-10]: 7 __ Why: The suggestion asks to verify that the new 5th parameter (likely ExecutorService) is correctly positioned and that null is the intended value. This is a reasonable verification request for an API change, though it's not a critical bug since the code compiles and the change appears intentional.	Medium
	Review ExecutorService parameter necessity The StandardDirectoryReader.open() call adds a new null parameter as the 5th argument. Confirm this parameter represents an ExecutorService and that passing null is appropriate for this NRT replication context, as it may impact concurrent refresh operations. server/src/main/java/org/opensearch/index/engine/NRTReplicationReaderManager.java [83-87] +// Consider if ExecutorService should be provided for concurrent operations referenceToRefresh.directory(), currentInfos, subs, engineConfig.getLeafSorter(), -null +null // ExecutorService - verify null is appropriate Suggestion importance[1-10]: 6 __ Why: The suggestion asks to confirm whether passing null for the ExecutorService parameter is appropriate in the NRT replication context. While this is a valid consideration for performance optimization, it's more of a verification request than identifying a concrete issue, as the code appears to be intentionally passing null.	Low

Suggestions up to commit 4b7978e

Category	Suggestion	Impact
Possible issue	Restore removed constructor for backward compatibility The Composite103Codec has been moved to backward codecs but the constructor removed in the diff was essential for write operations. Without the constructor accepting compressionMode, mapperService, and logger, the codec cannot be instantiated for writing new segments with compression settings, breaking backward compatibility for existing indices. server/src/main/java/org/opensearch/index/codec/composite/backward_codecs/composite103/Composite103Codec.java [32-38] package org.opensearch.index.codec.composite.backward_codecs.composite103; +import org.apache.logging.log4j.Logger; import org.apache.lucene.backward_codecs.lucene103.Lucene103Codec; import org.apache.lucene.codecs.Codec; import org.apache.lucene.codecs.DocValuesFormat; import org.apache.lucene.codecs.FilterCodec; import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.index.codec.PerFieldMappingPostingFormatCodec; import org.opensearch.index.codec.composite.composite912.Composite912DocValuesFormat; import org.opensearch.index.mapper.MapperService; @ExperimentalApi public class Composite103Codec extends FilterCodec { public static final String COMPOSITE_INDEX_CODEC_NAME = "Composite103Codec"; private final MapperService mapperService; // needed for SPI - this is used in reader path public Composite103Codec() { this(COMPOSITE_INDEX_CODEC_NAME, new Lucene103Codec(), null); } + public Composite103Codec(Lucene103Codec.Mode compressionMode, MapperService mapperService, Logger logger) { + this(COMPOSITE_INDEX_CODEC_NAME, new PerFieldMappingPostingFormatCodec(compressionMode, mapperService, logger), mapperService); + } + Suggestion importance[1-10]: 9 __ Why: The removed constructor is critical for backward compatibility. The Composite103Codec was moved to backward codecs but lost the constructor needed for write operations with compression settings. Without it, existing indices using Composite103Codec with specific compression modes cannot be properly maintained, which is a significant compatibility issue.	High
General	Evaluate async segment opening for NRT The new null parameter for ExecutorService in StandardDirectoryReader.open() may impact NRT replication performance. Consider whether asynchronous segment opening would benefit replication scenarios, especially for large segment refreshes. server/src/main/java/org/opensearch/index/engine/NRTReplicationReaderManager.java [83-88] +// Using null ExecutorService for synchronous opening to maintain consistency with existing NRT behavior +// TODO: Evaluate async opening for large segment refreshes final DirectoryReader innerReader = StandardDirectoryReader.open( referenceToRefresh.directory(), currentInfos, subs, engineConfig.getLeafSorter(), null ); Suggestion importance[1-10]: 5 __ Why: This suggestion raises a valid performance consideration about potentially using asynchronous segment opening for NRT replication. However, it's more of a future optimization opportunity rather than an issue with the current PR. The null parameter maintains existing synchronous behavior during the Lucene upgrade, which is appropriate for compatibility.	Low
	Document null ExecutorService parameter intent The StandardDirectoryReader.open() call now includes an additional null parameter. Verify that this new parameter (likely an ExecutorService) is intentionally null and document why asynchronous segment opening is not needed in this context to prevent potential performance issues. server/src/main/java/org/opensearch/index/engine/Engine.java [312] +// Synchronous opening is intentional here as we need immediate access to max sequence numbers try (DirectoryReader innerReader = StandardDirectoryReader.open(store.directory(), segmentInfos, null, null, null)) { Suggestion importance[1-10]: 4 __ Why: While documenting the intent of the null parameter is helpful for code clarity, this is a minor improvement. The suggestion correctly identifies that the new parameter is added due to Lucene 10.4.0 API changes, but the change itself appears intentional and correct for this synchronous context.	Low

[github-actions]

❌ Gradle check result for 4b7978e: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit a739e0e

[github-actions]

❌ Gradle check result for a739e0e: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 47ae628

[github-actions]

❌ Gradle check result for 47ae628: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit c72c3e9

[github-actions]

✅ Gradle check result for c72c3e9: SUCCESS

[codecov]

Codecov Report

❌ Patch coverage is 84.61538% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.15%. Comparing base (c12aa29) to head (c72c3e9).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
...n/java/org/opensearch/index/engine/NoOpEngine.java	0.00%	3 Missing ⚠️
...opensearch/index/mapper/CompletionFieldMapper.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #20735      +/-   ##
============================================
- Coverage     73.28%   73.15%   -0.14%     
+ Complexity    72066    71964     -102     
============================================
  Files          5786     5787       +1     
  Lines        329620   329680      +60     
  Branches      47568    47572       +4     
============================================
- Hits         241574   241183     -391     
- Misses        68700    69117     +417     
- Partials      19346    19380      +34     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[navneet1v]

waiting for this to merge