update after review

Author: drempapis

x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/codecs/BWCCodec.java

@@ -28,6 +28,7 @@
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.IOContext;
 import org.apache.lucene.util.Version;
+import org.elasticsearch.core.UpdateForV10;
 import org.elasticsearch.xpack.lucene.bwc.codecs.lucene80.BWCLucene80Codec;
 import org.elasticsearch.xpack.lucene.bwc.codecs.lucene84.BWCLucene84Codec;
 import org.elasticsearch.xpack.lucene.bwc.codecs.lucene86.BWCLucene86Codec;
@@ -97,23 +98,60 @@ public SegmentInfoFormat segmentInfoFormat() {
         return segmentInfosFormat;
     }
 
+    /**
+     * This method is not supported for archive indices and older codecs and will always throw an {@link UnsupportedOperationException}.
+     * This method is never called in practice, as we rewrite field infos to override the info about which features are present in
+     * the index. Even if norms are present, field info lies about it.
+     *
+     * @return nothing, as this method always throws an exception
+     * @throws UnsupportedOperationException always thrown to indicate that this method is not supported
+     */
     @Override
     public final NormsFormat normsFormat() {
         throw new UnsupportedOperationException();
     }
 
+    /**
+     * This method is not supported for archive indices and older codecs and will always throw an {@link UnsupportedOperationException}.
+     * This method is never called in practice, as we rewrite field infos to override the info about which features are present in
+     * the index. Even if term vectors are present, field info lies about it.
+     *
+     * @return nothing, as this method always throws an exception
+     * @throws UnsupportedOperationException always thrown to indicate that this method is not supported
+     */
     @Override
     public final TermVectorsFormat termVectorsFormat() {
         throw new UnsupportedOperationException();
     }
 
+    /**
+     * This method is not supported for archive indices and older codecs and will always throw an {@link UnsupportedOperationException}.
+     * The knn vectors can't be present because it is not supported yet in any of the lucene versions that we support for archive indices.
+     *
+     * @return nothing, as this method always throws an exception
+     * @throws UnsupportedOperationException always thrown to indicate that this method is not supported
+     */
     @Override
     public final KnnVectorsFormat knnVectorsFormat() {
         throw new UnsupportedOperationException();
     }
 
+    /**
+     * Returns the original {@link SegmentInfoFormat} used by this codec.
+     * This method should be implemented by subclasses to provide the specific
+     * {@link SegmentInfoFormat} that this codec is intended to use.
+     *
+     * @return the original {@link SegmentInfoFormat} used by this codec
+     */
     protected abstract SegmentInfoFormat originalSegmentInfoFormat();
 
+    /**
+     * Returns the original {@link FieldInfosFormat} used by this codec.
+     * This method should be implemented by subclasses to provide the specific
+     * {@link FieldInfosFormat} that this codec is intended to use.
+     *
+     * @return the original {@link FieldInfosFormat} used by this codec
+     */
     protected abstract FieldInfosFormat originalFieldInfosFormat();
 
     // mark all fields as no term vectors, no norms, no payloads, and no vectors.
@@ -170,8 +208,12 @@ public static SegmentInfo wrap(SegmentInfo segmentInfo) {
         return segmentInfo1;
     }
 
-    // Special handling for Lucene8xCodecs (which are currently bundled with Lucene)
-    // Use BWCLucene8xCodec instead as that one extends BWCCodec (similar to all other older codecs)
+    /**
+     * Returns a backward-compatible codec for the given codec. If the codec is one of the known Lucene 8.x codecs,
+     * it returns a corresponding read-only backward-compatible codec. Otherwise, it returns the original codec.
+     * This switch if for indices created in ES 6.x, that may have some of their segments written with ES 7.x, hence Lucene 8.x.
+     */
+    @UpdateForV10(owner = UpdateForV10.Owner.SEARCH_FOUNDATIONS)
     private static Codec getBackwardCompatibleCodec(Codec codec) {
         if (codec == null) return null;
 

x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/codecs/lucene60/MetadataOnlyBKDReader.java

@@ -46,10 +46,9 @@ public class MetadataOnlyBKDReader extends PointValues {
     final long pointCount;
     final int docCount;
     final int version;
+    final long minLeafBlockFP;
 
     final int numIndexBytes;
-    final long minLeafBlockFP;
-    private final long indexStartPointer;
 
     public MetadataOnlyBKDReader(IndexInput metaIn) throws IOException {
         version = CodecUtil.checkHeader(metaIn, "BKD", VERSION_START, VERSION_CURRENT);
@@ -93,7 +92,8 @@ public MetadataOnlyBKDReader(IndexInput metaIn) throws IOException {
         // This code has been introduced to process IndexInput created with Lucene86Codec+
         numIndexBytes = metaIn.readVInt();
         minLeafBlockFP = metaIn.readLong();
-        indexStartPointer = metaIn.readLong();
+        // The following fields are not used in this class, but we need to read them to advance the pointer
+        long indexStartPointer = metaIn.readLong();
     }
 
     @Override

x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/codecs/lucene80/BWCLucene80Codec.java

@@ -26,6 +26,7 @@
 import org.elasticsearch.xpack.lucene.bwc.codecs.lucene60.Lucene60MetadataOnlyPointsFormat;
 
 /**
+ * This is a fork of {@link org.apache.lucene.backward_codecs.lucene80.Lucene80Codec}
  * Implements the Lucene 8.0 index format. Loaded via SPI for indices created/written with Lucene 8.0.0-8.3.0
  * (Elasticsearch [7.0.0-7.5.2]), mounted as archive indices in Elasticsearch 8.x / 9.x.
  */
@@ -48,11 +49,7 @@ public DocValuesFormat getDocValuesFormatForField(String field) {
     // Needed for SPI loading
     @SuppressWarnings("unused")
     public BWCLucene80Codec() {
-        this("BWCLucene80Codec");
-    }
-
-    public BWCLucene80Codec(String name) {
-        super(name);
+        super("BWCLucene80Codec");
         this.storedFieldsFormat = new Lucene50StoredFieldsFormat(Lucene50StoredFieldsFormat.Mode.BEST_SPEED);
     }
 

x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/codecs/lucene84/BWCLucene84Codec.java

@@ -28,6 +28,7 @@
 import org.elasticsearch.xpack.lucene.bwc.codecs.lucene60.Lucene60MetadataOnlyPointsFormat;
 
 /**
+ * This is a fork of {@link org.apache.lucene.backward_codecs.lucene84.Lucene84Codec}
  * Implements the Lucene 8.4 index format. Loaded via SPI for indices created/written with Lucene 8.4.0-8.5.1
  * (Elasticsearch [7.6.0-7.8.1]), mounted as archive indices in Elasticsearch 8.x / 9.x.
  */
@@ -58,11 +59,7 @@ public DocValuesFormat getDocValuesFormatForField(String field) {
     // Needed for SPI loading
     @SuppressWarnings("unused")
     public BWCLucene84Codec() {
-        this("BWCLucene84Codec");
-    }
-
-    public BWCLucene84Codec(String name) {
-        super(name);
+        super("BWCLucene84Codec");
         this.storedFieldsFormat = new Lucene50StoredFieldsFormat(Lucene50StoredFieldsFormat.Mode.BEST_SPEED);
         this.defaultFormat = new Lucene84PostingsFormat();
         this.defaultDVFormat = new Lucene80DocValuesFormat();

x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/codecs/lucene86/BWCLucene86Codec.java

@@ -12,7 +12,6 @@
 import org.apache.lucene.backward_codecs.lucene50.Lucene50StoredFieldsFormat;
 import org.apache.lucene.backward_codecs.lucene60.Lucene60FieldInfosFormat;
 import org.apache.lucene.backward_codecs.lucene80.Lucene80DocValuesFormat;
-import org.apache.lucene.backward_codecs.lucene84.Lucene84PostingsFormat;
 import org.apache.lucene.backward_codecs.lucene86.Lucene86SegmentInfoFormat;
 import org.apache.lucene.codecs.CompoundFormat;
 import org.apache.lucene.codecs.DocValuesFormat;
@@ -23,10 +22,10 @@
 import org.apache.lucene.codecs.SegmentInfoFormat;
 import org.apache.lucene.codecs.StoredFieldsFormat;
 import org.apache.lucene.codecs.perfield.PerFieldDocValuesFormat;
-import org.apache.lucene.codecs.perfield.PerFieldPostingsFormat;
 import org.elasticsearch.xpack.lucene.bwc.codecs.BWCCodec;
 
 /**
+ * This is a fork of {@link org.apache.lucene.backward_codecs.lucene86.Lucene86Codec}
  * Implements the Lucene 8.6 index format. Loaded via SPI for indices created/written with Lucene 8.6.0-8.6.2
  * (Elasticsearch [7.9.0-7.9.3]), mounted as archive indices in Elasticsearch 8.x / 9.x.
  */
@@ -35,16 +34,8 @@ public class BWCLucene86Codec extends BWCCodec {
     private final LiveDocsFormat liveDocsFormat = new Lucene50LiveDocsFormat();
     private final CompoundFormat compoundFormat = new Lucene50CompoundFormat();
     private final PointsFormat pointsFormat = new Lucene86MetadataOnlyPointsFormat();
-    private final PostingsFormat defaultFormat;
     private final DocValuesFormat defaultDVFormat;
 
-    private final PostingsFormat postingsFormat = new PerFieldPostingsFormat() {
-        @Override
-        public PostingsFormat getPostingsFormatForField(String field) {
-            return defaultFormat;
-        }
-    };
-
     private final DocValuesFormat docValuesFormat = new PerFieldDocValuesFormat() {
         @Override
         public DocValuesFormat getDocValuesFormatForField(String field) {
@@ -57,14 +48,8 @@ public DocValuesFormat getDocValuesFormatForField(String field) {
     // Needed for SPI loading
     @SuppressWarnings("unused")
     public BWCLucene86Codec() {
-        this("BWCLucene86Codec");
-    }
-
-    /** Instantiates a new codec. */
-    public BWCLucene86Codec(String name) {
-        super(name);
+        super("BWCLucene86Codec");
         this.storedFieldsFormat = new Lucene50StoredFieldsFormat(Lucene50StoredFieldsFormat.Mode.BEST_SPEED);
-        this.defaultFormat = new Lucene84PostingsFormat();
         this.defaultDVFormat = new Lucene80DocValuesFormat();
     }
 

x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/codecs/lucene87/BWCLucene87Codec.java

@@ -27,32 +27,13 @@
 import org.elasticsearch.xpack.lucene.bwc.codecs.BWCCodec;
 import org.elasticsearch.xpack.lucene.bwc.codecs.lucene86.Lucene86MetadataOnlyPointsFormat;
 
-import java.util.Objects;
-
 /**
+ * This is a fork of {@link org.apache.lucene.backward_codecs.lucene87.Lucene87Codec}
  * Implements the Lucene 8.7 index format. Loaded via SPI for indices created/written with Lucene 8.7.0-8.11.3
  * (Elasticsearch [7.10.0-7-17.26]), mounted as archive indices in Elasticsearch 8.x / 9.x.
  */
 public class BWCLucene87Codec extends BWCCodec {
 
-    private enum Mode {
-        /** Trade compression ratio for retrieval speed. */
-        BEST_SPEED(Lucene87StoredFieldsFormat.Mode.BEST_SPEED, Lucene80DocValuesFormat.Mode.BEST_SPEED),
-        /** Trade retrieval speed for compression ratio. */
-        BEST_COMPRESSION(Lucene87StoredFieldsFormat.Mode.BEST_COMPRESSION, Lucene80DocValuesFormat.Mode.BEST_COMPRESSION);
-
-        /** compression mode for stored fields */
-        private final Lucene87StoredFieldsFormat.Mode storedMode;
-
-        /** compression mode for doc value fields */
-        private final Lucene80DocValuesFormat.Mode dvMode;
-
-        Mode(Lucene87StoredFieldsFormat.Mode storedMode, Lucene80DocValuesFormat.Mode dvMode) {
-            this.storedMode = Objects.requireNonNull(storedMode);
-            this.dvMode = Objects.requireNonNull(dvMode);
-        }
-    }
-
     private final LiveDocsFormat liveDocsFormat = new Lucene50LiveDocsFormat();
     private final CompoundFormat compoundFormat = new Lucene50CompoundFormat();
     private final PointsFormat pointsFormat = new Lucene86MetadataOnlyPointsFormat();
@@ -78,14 +59,10 @@ public DocValuesFormat getDocValuesFormatForField(String field) {
     // Needed for SPI loading
     @SuppressWarnings("unused")
     public BWCLucene87Codec() {
-        this("BWCLucene87Codec", Mode.BEST_COMPRESSION);
-    }
-
-    public BWCLucene87Codec(String name, Mode mode) {
-        super(name);
-        this.storedFieldsFormat = new Lucene87StoredFieldsFormat(mode.storedMode);
+        super("BWCLucene87Codec");
+        this.storedFieldsFormat = new Lucene87StoredFieldsFormat(Lucene87StoredFieldsFormat.Mode.BEST_COMPRESSION);
         this.defaultFormat = new Lucene84PostingsFormat();
-        this.defaultDVFormat = new Lucene80DocValuesFormat(mode.dvMode);
+        this.defaultDVFormat = new Lucene80DocValuesFormat(Lucene80DocValuesFormat.Mode.BEST_COMPRESSION);
     }
 
     @Override

x-pack/plugin/old-lucene-versions/src/test/java/org/elasticsearch/xpack/lucene/bwc/codecs/BWCCodecTests.java

@@ -18,6 +18,9 @@
 import org.apache.lucene.codecs.StoredFieldsFormat;
 import org.elasticsearch.test.ESTestCase;
 
+/**
+ * Unit tests for the {@link BWCCodec} class.
+ */
 public class BWCCodecTests extends ESTestCase {
 
     private final Codec codec;
@@ -66,14 +69,23 @@ public PointsFormat pointsFormat() {
         };
     }
 
+    /**
+     * Tests that the {@link Codec#normsFormat()} method throws an {@link UnsupportedOperationException}.
+     */
     public void testNormsFormatUnsupportedOperation() {
         assertThrows(UnsupportedOperationException.class, codec::normsFormat);
     }
 
+    /**
+     * Tests that the {@link Codec#termVectorsFormat()} method throws an {@link UnsupportedOperationException}.
+     */
     public void testTermVectorsFormatUnsupportedOperation() {
         assertThrows(UnsupportedOperationException.class, codec::termVectorsFormat);
     }
 
+    /**
+     * Tests that the {@link Codec#knnVectorsFormat()} method throws an {@link UnsupportedOperationException}.
+     */
     public void testKnnVectorsFormatUnsupportedOperation() {
         assertThrows(UnsupportedOperationException.class, codec::knnVectorsFormat);
     }

x-pack/plugin/old-lucene-versions/src/test/java/org/elasticsearch/xpack/lucene/bwc/codecs/OldCodecsAvailableTests.java

@@ -28,7 +28,7 @@ public class OldCodecsAvailableTests extends ESTestCase {
      */
     @UpdateForV10(owner = UpdateForV10.Owner.SEARCH_FOUNDATIONS)
     public void testLuceneBWCCodecsAvailable() {
-        assertEquals("Add Lucene BWC codecs for Elasticsearch version 9", 9, Version.CURRENT.major);
+        assertEquals("Add Lucene BWC codecs for Elasticsearch version 8", 9, Version.CURRENT.major);
 
         String codecPathRegex = ".*[\\\\.](Lucene(8[0-9])Codec)";
         Pattern codecPathPattern = Pattern.compile(codecPathRegex);