Support 7x segments as archive in 8x / 9x#119503
Conversation
drempapis
added
v8.18.0
Team:Search Foundations
|
Pinging @elastic/es-search-foundations (Team:Search Foundations) |
|
Hi @drempapis, I've created a changelog YAML for you. |
…hub.com:drempapis/elasticsearch into fix/117042_Support_7x_segments_as_archive_in_8x
.../src/main/java/org/elasticsearch/xpack/lucene/bwc/codecs/lucene60/MetadataOnlyBKDReader.java
Outdated
Show resolved
Hide resolved
javanna
left a comment
javanna
left a comment
There was a problem hiding this comment.
left two minors, LGTM otherwise. This was not an easy one, thanks for bearing with me!
...ava/org/elasticsearch/xpack/lucene/bwc/codecs/lucene86/Lucene86MetadataOnlyPointsReader.java
Outdated
Show resolved
Hide resolved
| pointCount = metaIn.readVLong(); | ||
| docCount = metaIn.readVInt(); | ||
|
|
||
| // This code has been introduced to process IndexInput created with Lucene86Codec+ |
There was a problem hiding this comment.
Maybe explain that we are good ignoring additional fields with older formats, just to leave a trace of our analysis.
There was a problem hiding this comment.
Could you clarify this comment and add the context from my previous comment around why we conditionally read, and why nothing fails if you remove the conditional? I can see how we may be asking ourselves this same question in the future.
There was a problem hiding this comment.
I've added this; I will extend it with more words.
// This code has been introduced to process IndexInput created with Lucene86Codec+. This is not necessary
// in the read-only version for older formats.
There was a problem hiding this comment.
Created an additional pr to address the issues post-merge issues #125666
|
Thanks for reviewing this @javanna. I learned a lot of stuff working on this! |
drempapis
added
v9.0.0
auto-backport
💚 Backport successful
|
Added BWCLucene8*Codecs wrapper classes for the lucene8* equivalents. A BWC wrapper is initialized for archive indices and provides read-only capabilities for an index.
|
I think that this needs to be backported to 8.x, 8.18 and 8.17 as well, but you'll get conflicts and it may be easier to do it manually. Let me know if you need help. |
Added BWCLucene8*Codecs wrapper classes for the lucene8* equivalents. A BWC wrapper is initialized for archive indices and provides read-only capabilities for an index.
| if (codec == null) return null; | ||
|
|
||
| return switch (codec.getClass().getSimpleName()) { | ||
| case "Lucene70Codec" -> new BWCLucene70Codec(); |
There was a problem hiding this comment.
hey @drempapis I missed this during review, not a huge deal, but wrapping Lucene70Codec is not required here. Lucene70Codec is no longer shipped with Lucene 10.0, we rather ship it, and it's already extending BWCCodec. I would instead check that the codec we get extends from BWCCodec.
There was a problem hiding this comment.
That's true; I passed it from the debugger to verify it. I'll create a pr to adjust this.
There was a problem hiding this comment.
++ thanks, ping me for review, it'll be quicker this time :)
Added BWCLucene8*Codecs wrapper classes for the lucene8* equivalents. A BWC wrapper is initialized for archive indices and provides read-only capabilities for an index.
Added BWCLucene8*Codecs wrapper classes for the lucene8* equivalents. A BWC wrapper is initialized for archive indices and provides read-only capabilities for an index.
4 participants
The correspondence between the ES version, Lucene Version, and Lucene codec is as follows.
Each Codec version has a wrapper class handling the underlying Lucene codec.
Closes #117042