-
Notifications
You must be signed in to change notification settings - Fork 25.3k
lucene_snapshot: Update to new Lucene 10.3 postings format #128240
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
lucene_snapshot: Update to new Lucene 10.3 postings format #128240
Conversation
Pinging @elastic/es-search (Team:Search) |
server/src/test/java/org/elasticsearch/index/mapper/DateFieldTypeTests.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/search/query/QueryPhaseTests.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/index/codec/PerFieldFormatSupplier.java
Outdated
Show resolved
Hide resolved
...main/java/org/elasticsearch/index/codec/vectors/es818/DirectIOLucene99FlatVectorsFormat.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comments, thanks @ChrisHegarty !
server/src/test/java/org/elasticsearch/index/mapper/DateFieldTypeTests.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/index/mapper/DateFieldTypeTests.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/search/query/QueryPhaseTests.java
Outdated
Show resolved
Hide resolved
@@ -817,6 +817,13 @@ public void testNumericSortOptimization() throws Exception { | |||
|
|||
Query q = LongPoint.newRangeQuery(fieldNameLong, startLongValue, startLongValue + numDocs); | |||
|
|||
// 0. test assertion - the query rewritten to a match all - https://github.com/apache/lucene/pull/14609/ | |||
// TODO: reflow total hits expectations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this refers to the other TODO below or is it something else?
server/src/main/java/org/elasticsearch/index/codec/postings/Lucene90BlockTreeTermsWriter.java
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/index/codec/postings/CompressionAlgorithm.java
Outdated
Show resolved
Hide resolved
...main/java/org/elasticsearch/index/codec/vectors/es818/DirectIOLucene99FlatVectorsFormat.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/index/codec/PerFieldFormatSupplier.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test comments can be addressed here, or after this is merged (to get the branch compiling again)
server/src/test/java/org/elasticsearch/index/mapper/DateFieldTypeTests.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left a small comment, LGTM though
server/src/main/java/org/elasticsearch/index/codec/postings/Lucene90BlockTreeTermsWriter.java
Outdated
Show resolved
Hide resolved
@elasticmachine run elasticsearch-ci/part-1 |
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
server/src/main/java/org/elasticsearch/index/codec/Elasticsearch92Lucene103Codec.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/index/codec/PerFieldFormatSupplier.java
Show resolved
Hide resolved
While the CI is not completely green, there are some failures that seem unrelated, I'm going to merge this PR so that we can sync main into the branch also, and unblock further changes. |
There are a few different things going on in this PR, all of which are required to get the
lucene_snspahot
branch building again, but the most substantial is the update to the new Lucene 10.3 postings format,Lucene103PostingsFormat
.The
Lucene90BlockTreeTermsWriter
class is used in the implementation of the 10.1 postings codec in Lucene. With the new 10.3 postings format that class is no longer needed, so it has been moved to a test-only location, in order to support backward compatibility testing.In Elasticsearch we were using Lucene90BlockTreeTermsWriter (from Lucene) directly, through our copy of the Lucene 9.0 postings format, namely ES812PostingsFormat. So if you look at ES812PostingsFormat , you should that the imports should now show that we're using our own copy.
Additionally, changes are required because of the removal of deprecated methods in IOContext, as well as the override of hints for direct IO.