You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/01/16 17:38:15 UTC

[GitHub] [lucene-solr] s1monw opened a new pull request #2212: LUCENE-9669: Add an expert API to allow opening indices created < N-1

s1monw opened a new pull request #2212:
URL: https://github.com/apache/lucene-solr/pull/2212


   Today we force indices that were created with N-2 and older versions of lucene
   to fail on open. This check doesn't even check if the codecs are available. In order
   to allow users to open older indices and for us to support N-2 versions this change
   adds an API on DirectoryReader to specify a mininum index version on a per reader basis.
   This doesn't apply for the IndexWriter which will fail on opening older indices.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] s1monw commented on a change in pull request #2212: LUCENE-9669: Add an expert API to allow opening indices created < N-1

Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #2212:
URL: https://github.com/apache/lucene-solr/pull/2212#discussion_r559253990



##########
File path: lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestBackwardsCompatibility.java
##########
@@ -1986,4 +1988,45 @@ static BytesRef toBytes(long value) {
     bytes.bytes[bytes.length++] = (byte) value;
     return bytes;
   }
+
+  public void testFailOpenOldIndex() throws IOException {
+    for (String name : oldNames) {
+      Directory directory = oldIndexDirs.get(name);
+      IndexCommit commit = DirectoryReader.listCommits(directory).get(0);
+      IndexFormatTooOldException ex =
+          expectThrows(
+              IndexFormatTooOldException.class,
+              () -> StandardDirectoryReader.open(commit, Version.LATEST.major));
+      assertTrue(
+          ex.getMessage()
+              .contains(
+                  "only supports reading from version " + Version.LATEST.major + " upwards."));
+      // now open with allowed min version
+      StandardDirectoryReader.open(commit, Version.LATEST.major - 1).close();
+    }
+  }
+
+  public void testReadNMinusTwoCommit() throws IOException {
+    for (String name : this.unsupportedNames) {
+      if (name.startsWith(Version.MIN_SUPPORTED_MAJOR - 1 + ".")) {
+        Path oldIndexDir = createTempDir(name);
+        TestUtil.unzip(getDataInputStream("unsupported." + name + ".zip"), oldIndexDir);
+        try (BaseDirectoryWrapper dir = newFSDirectory(oldIndexDir)) {
+          // don't checkindex, we don't have the codecs yet
+          dir.setCheckIndexOnClose(false);
+          IllegalArgumentException iae =
+              expectThrows(IllegalArgumentException.class, () -> DirectoryReader.listCommits(dir));
+          // TODO fix this once we have the codec for 7.0 recreated
+          assertEquals(
+              "Could not load codec 'Lucene70'. Did you forget to add lucene-backward-codecs.jar?",

Review comment:
       it seems like we only had `LUCENE70` in this release https://github.com/apache/lucene-solr/tree/branch_8x/lucene/backward-codecs/src/java/org/apache/lucene/codecs




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] s1monw merged pull request #2212: LUCENE-9669: Add an expert API to allow opening indices created < N-1

Posted by GitBox <gi...@apache.org>.
s1monw merged pull request #2212:
URL: https://github.com/apache/lucene-solr/pull/2212


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] mikemccand commented on a change in pull request #2212: LUCENE-9669: Add an expert API to allow opening indices created < N-1

Posted by GitBox <gi...@apache.org>.
mikemccand commented on a change in pull request #2212:
URL: https://github.com/apache/lucene-solr/pull/2212#discussion_r559531239



##########
File path: lucene/core/src/java/org/apache/lucene/index/CheckIndex.java
##########
@@ -3900,6 +3907,12 @@ public static Options parseOptions(String[] args) {
         }
         i++;
         opts.dirImpl = args[i];
+      } else if ("-min_version_created".equals(args[i])) {

Review comment:
       `-min_major_version_created`?

##########
File path: lucene/core/src/java/org/apache/lucene/index/CheckIndex.java
##########
@@ -462,6 +463,11 @@ public void setInfoStream(PrintStream out, boolean verbose) {
     this.verbose = verbose;
   }
 
+  /** Set the minimum index version created for the index to check */
+  public void setMinIndexVersionCreated(int minIndexVersionCreated) {

Review comment:
       Could we consistently rename to `setMinIndexMajorVersionCreated`, and `minIndexMajorVersionCreated`?  (I see e.g. in `SIS.readCommit` below that we include `major` in the name).

##########
File path: lucene/core/src/java/org/apache/lucene/index/CheckIndex.java
##########
@@ -3900,6 +3907,12 @@ public static Options parseOptions(String[] args) {
         }
         i++;
         opts.dirImpl = args[i];
+      } else if ("-min_version_created".equals(args[i])) {
+        if (i == args.length - 1) {
+          throw new IllegalArgumentException("ERROR: missing value for -min_version_created");

Review comment:
       Hmm, we should also update the `Usage: ...` exception (around line 3928 in this modified version) to document this new option?
   
   If a user tries to `CheckIndex` a too-old index without this option they'll see a `IndexFormatTooOldException` right?  Should we catch that and rethrow w/ better message suggesting to use this option?  Should we maybe by default just set this option (always allow `CheckIndex` on a too-old index as long as you have the old Codecs around...)?

##########
File path: lucene/core/src/java/org/apache/lucene/index/DirectoryReader.java
##########
@@ -104,6 +104,23 @@ public static DirectoryReader open(final IndexCommit commit) throws IOException
     return StandardDirectoryReader.open(commit.getDirectory(), commit);
   }
 
+  /**
+   * Expert: returns an IndexReader reading the index in the given {@link IndexCommit}. This method

Review comment:
       s/`in`/`on`

##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -1009,6 +1009,14 @@ public IndexWriter(Directory d, IndexWriterConfig conf) throws IOException {
         changed();
 
       } else if (reader != null) {
+        if (reader.segmentInfos.getIndexCreatedVersionMajor() < Version.MIN_SUPPORTED_MAJOR) {

Review comment:
       Hmm does `addIndexes` try to verify version of the incoming index is not too old?  We will keep doing that, right?  I.e. the only added best effort here is when directly opening an `IndexReader` you can (with this change) now ask that older versions be allowed.

##########
File path: lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java
##########
@@ -499,7 +515,7 @@ private static Codec readCodec(DataInput input) throws IOException {
         throw new IllegalArgumentException(
             "Could not load codec '"
                 + name
-                + "'.  Did you forget to add lucene-backward-codecs.jar?",
+                + "'. Did you forget to add lucene-backward-codecs.jar?",

Review comment:
       Heh

##########
File path: lucene/core/src/java/org/apache/lucene/index/DirectoryReader.java
##########
@@ -104,6 +104,23 @@ public static DirectoryReader open(final IndexCommit commit) throws IOException
     return StandardDirectoryReader.open(commit.getDirectory(), commit);
   }
 
+  /**
+   * Expert: returns an IndexReader reading the index in the given {@link IndexCommit}. This method
+   * allows to open indices that were created wih a Lucene version older than N-1 provided that all
+   * codecs for this index are available in the classpath and the segment file format used was
+   * created with Lucene 7 or older. Users of this API must be aware that Lucene doesn't guarantee
+   * semantic compatibility for indices created with versions older than N-1. All backwards
+   * compatibility aside of the file format is optional and applied on a best effort basis.

Review comment:
       s/`of`/`from`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] s1monw commented on pull request #2212: LUCENE-9669: Add an expert API to allow opening indices created < N-1

Posted by GitBox <gi...@apache.org>.
s1monw commented on pull request #2212:
URL: https://github.com/apache/lucene-solr/pull/2212#issuecomment-762429660


   I plan to merge this during the next 24 hours thanks for the reviews


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] s1monw commented on a change in pull request #2212: LUCENE-9669: Add an expert API to allow opening indices created < N-1

Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #2212:
URL: https://github.com/apache/lucene-solr/pull/2212#discussion_r559620857



##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -1009,6 +1009,14 @@ public IndexWriter(Directory d, IndexWriterConfig conf) throws IOException {
         changed();
 
       } else if (reader != null) {
+        if (reader.segmentInfos.getIndexCreatedVersionMajor() < Version.MIN_SUPPORTED_MAJOR) {

Review comment:
       yes we verify that it's the same major as the index we are adding to.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] s1monw commented on a change in pull request #2212: LUCENE-9669: Add an expert API to allow opening indices created < N-1

Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #2212:
URL: https://github.com/apache/lucene-solr/pull/2212#discussion_r559179175



##########
File path: lucene/core/src/java/org/apache/lucene/index/LeafMetaData.java
##########
@@ -31,15 +31,19 @@
   private final Sort sort;
 
   /** Expert: Sole constructor. Public for use by custom {@link LeafReader} impls. */
-  public LeafMetaData(int createdVersionMajor, Version minVersion, Sort sort) {
+  public LeafMetaData(int createdVersionMajor, Version minVersion, int minVersionSupported, Sort sort) {
     this.createdVersionMajor = createdVersionMajor;
+    if (minVersionSupported > Version.LATEST.major || minVersionSupported < 0) {
+      throw new IllegalArgumentException("minVersionSupported must be positive and <= "
+              + Version.LATEST.major + " but was: " + minVersionSupported);
+    }
     if (createdVersionMajor > Version.LATEST.major) {
       throw new IllegalArgumentException(
           "createdVersionMajor is in the future: " + createdVersionMajor);
     }
-    if (createdVersionMajor < 6) {
+    if (createdVersionMajor < minVersionSupported) {
       throw new IllegalArgumentException(
-          "createdVersionMajor must be >= 6, got: " + createdVersionMajor);
+          "createdVersionMajor must be >= " + minVersionSupported + ", got: " + createdVersionMajor);

Review comment:
       I rolled this back. I really misread the logic. thanks for catching it.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] s1monw commented on a change in pull request #2212: LUCENE-9669: Add an expert API to allow opening indices created < N-1

Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #2212:
URL: https://github.com/apache/lucene-solr/pull/2212#discussion_r559618608



##########
File path: lucene/core/src/java/org/apache/lucene/index/CheckIndex.java
##########
@@ -3900,6 +3907,12 @@ public static Options parseOptions(String[] args) {
         }
         i++;
         opts.dirImpl = args[i];
+      } else if ("-min_version_created".equals(args[i])) {
+        if (i == args.length - 1) {
+          throw new IllegalArgumentException("ERROR: missing value for -min_version_created");

Review comment:
       I am all for old indices and remove this option. WDOT?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] romseygeek commented on a change in pull request #2212: LUCENE-9669: Add an expert API to allow opening indices created < N-1

Posted by GitBox <gi...@apache.org>.
romseygeek commented on a change in pull request #2212:
URL: https://github.com/apache/lucene-solr/pull/2212#discussion_r559604398



##########
File path: lucene/core/src/java/org/apache/lucene/index/DirectoryReader.java
##########
@@ -104,6 +104,23 @@ public static DirectoryReader open(final IndexCommit commit) throws IOException
     return StandardDirectoryReader.open(commit.getDirectory(), commit);
   }
 
+  /**
+   * Expert: returns an IndexReader reading the index in the given {@link IndexCommit}. This method
+   * allows to open indices that were created wih a Lucene version older than N-1 provided that all
+   * all codecs for this index are available in the classpath and the segment file format used was
+   * created with Lucene 7 or older. Users of this API must be aware that Lucene doesn't guarantee

Review comment:
       The javadoc should read 'Lucene 7 or newer' I think?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] jpountz commented on a change in pull request #2212: LUCENE-9669: Add an expert API to allow opening indices created < N-1

Posted by GitBox <gi...@apache.org>.
jpountz commented on a change in pull request #2212:
URL: https://github.com/apache/lucene-solr/pull/2212#discussion_r559039707



##########
File path: lucene/core/src/java/org/apache/lucene/index/DirectoryReader.java
##########
@@ -104,6 +105,17 @@ public static DirectoryReader open(final IndexCommit commit) throws IOException
     return StandardDirectoryReader.open(commit.getDirectory(), commit);
   }
 
+  /**
+   * Expert: returns an IndexReader reading the index in the given {@link IndexCommit}.
+   *
+   * @param commit the commit point to open
+   * @param minIndexVersionCreated the minimum index version created to check before opening the index

Review comment:
       I like how `listCommits` calls it a "major" version which avoids some ambiguity, maybe do the same here?
   
   Also could you improve javadocs to better describe how this version parameter affects the behavior of this method?

##########
File path: lucene/core/src/java/org/apache/lucene/index/LeafMetaData.java
##########
@@ -31,15 +31,19 @@
   private final Sort sort;
 
   /** Expert: Sole constructor. Public for use by custom {@link LeafReader} impls. */
-  public LeafMetaData(int createdVersionMajor, Version minVersion, Sort sort) {
+  public LeafMetaData(int createdVersionMajor, Version minVersion, int minVersionSupported, Sort sort) {
     this.createdVersionMajor = createdVersionMajor;
+    if (minVersionSupported > Version.LATEST.major || minVersionSupported < 0) {
+      throw new IllegalArgumentException("minVersionSupported must be positive and <= "
+              + Version.LATEST.major + " but was: " + minVersionSupported);
+    }
     if (createdVersionMajor > Version.LATEST.major) {
       throw new IllegalArgumentException(
           "createdVersionMajor is in the future: " + createdVersionMajor);
     }
-    if (createdVersionMajor < 6) {
+    if (createdVersionMajor < minVersionSupported) {
       throw new IllegalArgumentException(
-          "createdVersionMajor must be >= 6, got: " + createdVersionMajor);
+          "createdVersionMajor must be >= " + minVersionSupported + ", got: " + createdVersionMajor);

Review comment:
       These checks were introduced in the first place because 7.0 was the version that started recording the major version that created the index, and any older index would have this constant set to 6, rather than because 6 was the minimum supported version. So I think we can keep the logic how it is today without adding a new parameter to the ctor?

##########
File path: lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestBackwardsCompatibility.java
##########
@@ -1986,4 +1989,36 @@ static BytesRef toBytes(long value) {
     bytes.bytes[bytes.length++] = (byte) value;
     return bytes;
   }
+
+  public void testFailOpenOldIndex() throws IOException {
+    for (String name : oldNames) {
+      Directory directory = oldIndexDirs.get(name);
+      IndexCommit commit = DirectoryReader.listCommits(directory).get(0);
+      IndexFormatTooOldException ex = expectThrows(IndexFormatTooOldException.class, () ->
+              StandardDirectoryReader.open(commit, Version.LATEST.major));
+      ex.getMessage().contains("only supports reading from version " + Version.LATEST.major + " upwards." );

Review comment:
       this should be within an `assertTrue`?

##########
File path: lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java
##########
@@ -40,42 +41,49 @@
   final SegmentInfos segmentInfos;
   private final boolean applyAllDeletes;
   private final boolean writeAllDeletes;
+  private final int minVersionCreated;
 
   /** called only from static open() methods */
   StandardDirectoryReader(
-      Directory directory,
-      LeafReader[] readers,
-      IndexWriter writer,
-      SegmentInfos sis,
-      boolean applyAllDeletes,
-      boolean writeAllDeletes)
+          Directory directory,
+          LeafReader[] readers,
+          IndexWriter writer,
+          SegmentInfos sis,
+          boolean applyAllDeletes,
+          boolean writeAllDeletes, int minVersionCreated)

Review comment:
       put on a new line like other parameters?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] s1monw commented on a change in pull request #2212: LUCENE-9669: Add an expert API to allow opening indices created < N-1

Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #2212:
URL: https://github.com/apache/lucene-solr/pull/2212#discussion_r559614818



##########
File path: lucene/core/src/java/org/apache/lucene/index/DirectoryReader.java
##########
@@ -104,6 +104,23 @@ public static DirectoryReader open(final IndexCommit commit) throws IOException
     return StandardDirectoryReader.open(commit.getDirectory(), commit);
   }
 
+  /**
+   * Expert: returns an IndexReader reading the index in the given {@link IndexCommit}. This method
+   * allows to open indices that were created wih a Lucene version older than N-1 provided that all
+   * all codecs for this index are available in the classpath and the segment file format used was
+   * created with Lucene 7 or older. Users of this API must be aware that Lucene doesn't guarantee

Review comment:
       👍 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] s1monw commented on pull request #2212: LUCENE-9669: Add an expert API to allow opening indices created < N-1

Posted by GitBox <gi...@apache.org>.
s1monw commented on pull request #2212:
URL: https://github.com/apache/lucene-solr/pull/2212#issuecomment-762301076


   @mikemccand pushed changes 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] s1monw commented on a change in pull request #2212: LUCENE-9669: Add an expert API to allow opening indices created < N-1

Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #2212:
URL: https://github.com/apache/lucene-solr/pull/2212#discussion_r559385012



##########
File path: lucene/core/src/java/org/apache/lucene/index/DirectoryReader.java
##########
@@ -104,6 +104,23 @@ public static DirectoryReader open(final IndexCommit commit) throws IOException
     return StandardDirectoryReader.open(commit.getDirectory(), commit);
   }
 
+  /**
+   * Expert: returns an IndexReader reading the index in the given {@link IndexCommit}. This method
+   * allows to open indices that were created wih a Lucene version older than N-1 provided that all
+   * all codecs for this index are available in the classpath and the segment file format used was
+   * created with Lucene 7 or older. Users of this API must be aware that Lucene doesn't guarantee

Review comment:
       this is due to the fact that the segments info format only supports 7.0 and upwards




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] msokolov commented on a change in pull request #2212: LUCENE-9669: Add an expert API to allow opening indices created < N-1

Posted by GitBox <gi...@apache.org>.
msokolov commented on a change in pull request #2212:
URL: https://github.com/apache/lucene-solr/pull/2212#discussion_r559262779



##########
File path: lucene/core/src/java/org/apache/lucene/index/DirectoryReader.java
##########
@@ -104,6 +104,23 @@ public static DirectoryReader open(final IndexCommit commit) throws IOException
     return StandardDirectoryReader.open(commit.getDirectory(), commit);
   }
 
+  /**
+   * Expert: returns an IndexReader reading the index in the given {@link IndexCommit}. This method
+   * allows to open indices that were created wih a Lucene version older than N-1 provided that all
+   * all codecs for this index are available in the classpath and the segment file format used was

Review comment:
       "all all" -> "all"

##########
File path: lucene/core/src/java/org/apache/lucene/index/DirectoryReader.java
##########
@@ -104,6 +104,23 @@ public static DirectoryReader open(final IndexCommit commit) throws IOException
     return StandardDirectoryReader.open(commit.getDirectory(), commit);
   }
 
+  /**
+   * Expert: returns an IndexReader reading the index in the given {@link IndexCommit}. This method
+   * allows to open indices that were created wih a Lucene version older than N-1 provided that all
+   * all codecs for this index are available in the classpath and the segment file format used was
+   * created with Lucene 7 or older. Users of this API must be aware that Lucene doesn't guarantee

Review comment:
       Instead of "Lucene 7" should we reference the `minSupportedMajorVersion` parameter here? Or .. is it  `Version.MIN_SUPPORTED_MAJOR`?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] jpountz commented on a change in pull request #2212: LUCENE-9669: Add an expert API to allow opening indices created < N-1

Posted by GitBox <gi...@apache.org>.
jpountz commented on a change in pull request #2212:
URL: https://github.com/apache/lucene-solr/pull/2212#discussion_r559240488



##########
File path: lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestBackwardsCompatibility.java
##########
@@ -1986,4 +1988,45 @@ static BytesRef toBytes(long value) {
     bytes.bytes[bytes.length++] = (byte) value;
     return bytes;
   }
+
+  public void testFailOpenOldIndex() throws IOException {
+    for (String name : oldNames) {
+      Directory directory = oldIndexDirs.get(name);
+      IndexCommit commit = DirectoryReader.listCommits(directory).get(0);
+      IndexFormatTooOldException ex =
+          expectThrows(
+              IndexFormatTooOldException.class,
+              () -> StandardDirectoryReader.open(commit, Version.LATEST.major));
+      assertTrue(
+          ex.getMessage()
+              .contains(
+                  "only supports reading from version " + Version.LATEST.major + " upwards."));
+      // now open with allowed min version
+      StandardDirectoryReader.open(commit, Version.LATEST.major - 1).close();
+    }
+  }
+
+  public void testReadNMinusTwoCommit() throws IOException {
+    for (String name : this.unsupportedNames) {
+      if (name.startsWith(Version.MIN_SUPPORTED_MAJOR - 1 + ".")) {
+        Path oldIndexDir = createTempDir(name);
+        TestUtil.unzip(getDataInputStream("unsupported." + name + ".zip"), oldIndexDir);
+        try (BaseDirectoryWrapper dir = newFSDirectory(oldIndexDir)) {
+          // don't checkindex, we don't have the codecs yet
+          dir.setCheckIndexOnClose(false);
+          IllegalArgumentException iae =
+              expectThrows(IllegalArgumentException.class, () -> DirectoryReader.listCommits(dir));
+          // TODO fix this once we have the codec for 7.0 recreated
+          assertEquals(
+              "Could not load codec 'Lucene70'. Did you forget to add lucene-backward-codecs.jar?",

Review comment:
       I'm surprised that this works, not all 7.x indices use Lucene70 as a codec, e.g. there must be some indices that use Lucene75 or something like that as a codec? What am I missing?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] s1monw commented on pull request #2212: LUCENE-9669: Add an expert API to allow opening indices created < N-1

Posted by GitBox <gi...@apache.org>.
s1monw commented on pull request #2212:
URL: https://github.com/apache/lucene-solr/pull/2212#issuecomment-761810107


   @jpountz thanks, great feedback. I pushed a new commit


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org