GITHUB#7820: report which segment's .si could not be read

lucene/CHANGES.txt

@@ -131,6 +131,10 @@ New Features
 
 Improvements
 ---------------------
+* GITHUB#7820: CheckIndex now reports which segment's .si file could not be read, rather than only that
+  a commit point was unreadable. SegmentInfos throws CorruptSegmentInfoException, carrying the segment
+  name and the underlying failure. (Gokul Manoj, Serhiy Bzhezytskyy)
+
 * GITHUB#15704: Replace LinkedList with more efficient data structure. (Renato Haeberli)
 
 * GITHUB#15682: Use ArrayDeque instead of LinkedList in CompoundWordTokenFilterBase.java. (Renato Haeberli)

lucene/core/src/java/org/apache/lucene/index/CheckIndex.java

@@ -133,6 +133,13 @@ public static class Status {
     /** True if we were unable to locate and load the segments_N file. */
     public boolean missingSegments;
 
+    /**
+     * Name of the segment whose {@code .si} file could not be read, when that is why {@link
+     * #missingSegments} is set; null when the commit point itself could not be parsed far enough to
+     * name a segment.
+     */
+    public String brokenSegmentName;
+
     /** Name of latest segments_N file in the index. */
     public String segmentsFileName;
 
@@ -676,16 +683,27 @@ public Status checkIndex(List<String> onlySegments, ExecutorService executorServ
           throw IOUtils.rethrowAlways(t);
         }
 
+        String which = isLastCommit ? "latest" : "old (not latest)";
         String message;
 
-        if (isLastCommit) {
+        if (t instanceof CorruptSegmentInfoException corrupt) {
+          // The commit point itself parsed far enough to name the segment, so say which one.
+          result.brokenSegmentName = corrupt.getSegmentName();
           message =
-              "ERROR: could not read latest commit point from segments file \""
+              "ERROR: could not read segment \""
+                  + corrupt.getSegmentName()
+                  + "\" referenced by the "
+                  + which
+                  + " commit point in segments file \""
                   + fileName
-                  + "\" in directory";
+                  + "\": its "
+                  + corrupt.getSegmentName()
+                  + ".si is missing or corrupt";
         } else {
           message =
-              "ERROR: could not read old (not latest) commit point segments file \""
+              "ERROR: could not read "
+                  + which
+                  + " commit point from segments file \""
                   + fileName
                   + "\" in directory";
         }
@@ -991,7 +1009,7 @@ private Status.SegmentInfoStatus testSegment(
 
     final Version version = info.info.getVersion();
     if (info.info.maxDoc() <= 0) {
-      throw new CheckIndexException(" illegal number of documents: maxDoc=" + info.info.maxDoc());
+      throw new CheckIndexException("illegal number of documents: maxDoc=" + info.info.maxDoc());
     }
 
     int toLoseDocCount = info.info.maxDoc();

lucene/core/src/java/org/apache/lucene/index/CorruptSegmentInfoException.java

@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.index;
+
+import java.util.Objects;
+import org.apache.lucene.store.DataInput;
+
+/**
+ * Thrown when a single segment's {@code _N.si} file cannot be read, because it is missing or
+ * corrupt, while the commit point that references it parsed far enough to name the segment.
+ *
+ * <p>Unlike a plain {@link CorruptIndexException}, this carries the {@link #getSegmentName() name
+ * of the affected segment}, so a caller such as {@link CheckIndex} can report which segment is
+ * broken rather than only that something was. The root cause is always attached, since it is what
+ * names the file on disk.
+ *
+ * @lucene.internal
+ */
+public class CorruptSegmentInfoException extends CorruptIndexException {
+
+  /** Name of the segment whose {@code .si} file could not be read. */
+  private final String segmentName;
+
+  /**
+   * Create an exception naming the segment whose {@code .si} could not be read.
+   *
+   * @param segmentName name of the affected segment, must not be null
+   * @param message description of what went wrong
+   * @param input the input being read when the failure was detected
+   * @param cause the underlying failure, must not be null
+   */
+  public CorruptSegmentInfoException(
+      String segmentName, String message, DataInput input, Throwable cause) {
+    super(message, input, Objects.requireNonNull(cause));
+    this.segmentName = Objects.requireNonNull(segmentName);
+  }
+
+  /** Returns the name of the segment whose {@code .si} file could not be read. */
+  public String getSegmentName() {
+    return segmentName;
+  }
+}

lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java

@@ -45,6 +45,7 @@
 import org.apache.lucene.store.IndexOutput;
 import org.apache.lucene.util.IOUtils;
 import org.apache.lucene.util.StringHelper;
+import org.apache.lucene.util.ThreadInterruptedException;
 import org.apache.lucene.util.Version;
 
 /**
@@ -394,8 +395,20 @@ private static void parseSegmentInfos(
       byte[] segmentID = new byte[StringHelper.ID_LENGTH];
       input.readBytes(segmentID, 0, segmentID.length);
       Codec codec = readCodec(input);
-      SegmentInfo info =
-          codec.segmentInfoFormat().read(directory, segName, segmentID, IOContext.READONCE);
+      final SegmentInfo info;
+      try {
+        info = codec.segmentInfoFormat().read(directory, segName, segmentID, IOContext.READONCE);
+      } catch (ThreadInterruptedException e) {
+        throw e;
+      } catch (Exception | AssertionError e) {
+        // Corruption in a .si file can surface as almost anything the codec's reader happens to do
+        // with the bad bytes, so catch broadly, but keep the root cause: it is what names the file.
+        throw new CorruptSegmentInfoException(
+            segName,
+            "segment info file: " + segName + ".si cannot be read - it may be missing or corrupt",
+            input,
+            e);
+      }
       info.setCodec(codec);
       totalDocs += info.maxDoc();
       long delGen = CodecUtil.readBELong(input);

lucene/core/src/test/org/apache/lucene/index/TestCheckIndex.java

@@ -19,7 +19,10 @@
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import java.io.ByteArrayOutputStream;
+import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.io.PrintStream;
+import java.nio.file.NoSuchFileException;
 import java.util.List;
 import org.apache.lucene.document.BinaryPoint;
 import org.apache.lucene.document.Document;
@@ -34,6 +37,9 @@
 import org.apache.lucene.search.Sort;
 import org.apache.lucene.search.SortField;
 import org.apache.lucene.store.Directory;
+import org.apache.lucene.store.IOContext;
+import org.apache.lucene.store.IndexInput;
+import org.apache.lucene.store.IndexOutput;
 import org.apache.lucene.tests.analysis.CannedTokenStream;
 import org.apache.lucene.tests.analysis.Token;
 import org.apache.lucene.tests.index.BaseTestCheckIndex;
@@ -293,4 +299,77 @@ public void testPriorBrokenCommitPoint() throws Exception {
       }
     }
   }
+
+  public void testCorruptSegmentInfoNamesTheSegment() throws Exception {
+    for (String corruption : List.of("delete-si", "truncate-si")) {
+      try (MockDirectoryWrapper dir = newMockDirectory()) {
+        // this test intentionally leaves a broken index behind
+        dir.setCheckIndexOnClose(false);
+
+        IndexWriterConfig iwc = new IndexWriterConfig().setMergePolicy(NoMergePolicy.INSTANCE);
+        try (IndexWriter iw = new IndexWriter(dir, iwc)) {
+          for (int seg = 0; seg < 2; seg++) {
+            Document doc = new Document();
+            doc.add(new StringField("id", "d" + seg, Field.Store.NO));
+            iw.addDocument(doc);
+            iw.commit();
+          }
+        }
+
+        // NOTE: relying on precise file naming, as testPriorBrokenCommitPoint above already does.
+        if (corruption.equals("delete-si")) {
+          dir.deleteFile("_1.si");
+        } else {
+          truncate(dir, "_1.si");
+        }
+
+        // Reading the commit point names the segment whose .si could not be read, and keeps the
+        // root cause: it is what names the file on disk.
+        CorruptSegmentInfoException e =
+            expectThrows(
+                CorruptSegmentInfoException.class,
+                () ->
+                    SegmentInfos.readCommit(
+                        dir, SegmentInfos.getLastCommitSegmentsFileName(dir), 0));
+        assertEquals(corruption, "_1", e.getSegmentName());
+        // the root cause must be the codec's own failure, since that is what names the file on disk
+        Throwable cause = e.getCause();
+        assertNotNull(corruption, cause);
+        if (corruption.equals("delete-si")) {
+          assertTrue(
+              corruption + ": " + cause,
+              cause instanceof NoSuchFileException || cause instanceof FileNotFoundException);
+        } else {
+          assertTrue(corruption + ": " + cause, cause instanceof IOException);
+        }
+        assertTrue(corruption + ": " + cause, cause.toString().contains("_1.si"));
+
+        // ... and CheckIndex reports it rather than only that something was unreadable
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        try (CheckIndex checker = new CheckIndex(dir)) {
+          checker.setInfoStream(new PrintStream(out, false, UTF_8), false);
+          CheckIndex.Status status = checker.checkIndex();
+
+          assertFalse(corruption, status.clean);
+          assertTrue(corruption, status.missingSegments);
+          assertEquals(corruption, "_1", status.brokenSegmentName);
+        }
+        assertTrue(
+            out.toString(UTF_8), out.toString(UTF_8).contains("could not read segment \"_1\""));
+      }
+    }
+  }
+
+  /** Rewrites {@code name} keeping only its first 70% of bytes. */
+  private static void truncate(Directory dir, String name) throws IOException {
+    byte[] bytes;
+    try (IndexInput in = dir.openInput(name, IOContext.READONCE)) {
+      bytes = new byte[(int) (in.length() * 0.7)];
+      in.readBytes(bytes, 0, bytes.length);
+    }
+    dir.deleteFile(name);
+    try (IndexOutput out = dir.createOutput(name, IOContext.DEFAULT)) {
+      out.writeBytes(bytes, bytes.length);
+    }
+  }
 }

lucene/core/src/test/org/apache/lucene/index/TestTransactions.java

@@ -206,7 +206,10 @@ public void doWork() throws Throwable {
         } catch (Exception e) {
           // can be rethrown as RuntimeException if it happens during a close listener
           if (!e.getMessage().contains("on purpose")) {
-            throw e;
+            // Caught "on-purpose" IOException can be rethrown as CorruptSegmentInfoException
+            if (e instanceof CorruptSegmentInfoException == false) {
+              throw e;
+            }
           }
           // release resources
           IOUtils.closeWhileHandlingException(r1, r2);