From a610ddf19a5a52ba90e32a74547aebe64d067355 Mon Sep 17 00:00:00 2001 From: gokai Date: Fri, 19 Jan 2024 12:39:25 +0000 Subject: [PATCH 1/4] Throw CorruptSegmentInfoException on encountering missing segment info (_N.si) file in CheckIndex (cherry picked from commit 4c5c628ae83f13761f6746f1c011f4a0fabc25a3) (cherry picked from commit 2c33c716f81eebe3c305bb88150d465790f7ccd3) --- .../org/apache/lucene/index/CheckIndex.java | 2 +- .../index/CorruptSegmentInfoException.java | 70 +++++++++++++++++++ .../org/apache/lucene/index/SegmentInfos.java | 18 ++++- .../apache/lucene/index/TestTransactions.java | 5 +- 4 files changed, 90 insertions(+), 5 deletions(-) create mode 100644 lucene/core/src/java/org/apache/lucene/index/CorruptSegmentInfoException.java diff --git a/lucene/core/src/java/org/apache/lucene/index/CheckIndex.java b/lucene/core/src/java/org/apache/lucene/index/CheckIndex.java index 8939c5b59c91..8592c5baf915 100644 --- a/lucene/core/src/java/org/apache/lucene/index/CheckIndex.java +++ b/lucene/core/src/java/org/apache/lucene/index/CheckIndex.java @@ -991,7 +991,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(); diff --git a/lucene/core/src/java/org/apache/lucene/index/CorruptSegmentInfoException.java b/lucene/core/src/java/org/apache/lucene/index/CorruptSegmentInfoException.java new file mode 100644 index 000000000000..6df76f9c2dfa --- /dev/null +++ b/lucene/core/src/java/org/apache/lucene/index/CorruptSegmentInfoException.java @@ -0,0 +1,70 @@ +/* + * 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 org.apache.lucene.store.DataInput; +import org.apache.lucene.store.DataOutput; + +/** + * This exception is thrown when Lucene is unable to read a SegmentInfo file _N.si (When it is + * missing or corrupt) + */ +public class CorruptSegmentInfoException extends CorruptIndexException { + + // The segment name of the missing .si file is always stored for accurate error reporting + String segmentName; + + /** Create exception with the segmentName and message only */ + public CorruptSegmentInfoException(String segmentName, String message, DataInput input) { + super(message, input); + this.segmentName = segmentName; + } + + /** Create exception with the segmentName and message only */ + public CorruptSegmentInfoException(String segmentName, String message, DataOutput output) { + super(message, output); + this.segmentName = segmentName; + } + + /** Create exception with the segmentName, message and root cause. */ + public CorruptSegmentInfoException( + String segmentName, String message, DataInput input, Throwable cause) { + super(message, input, cause); + this.segmentName = segmentName; + } + + /** Create exception with the segmentName, message and root cause. */ + public CorruptSegmentInfoException( + String segmentName, String message, DataOutput output, Throwable cause) { + super(message, output, cause); + this.segmentName = segmentName; + } + + /** Create exception with the segmentName and message only */ + public CorruptSegmentInfoException( + String segmentName, String message, String resourceDescription) { + super(message, resourceDescription); + this.segmentName = segmentName; + } + + /** Create exception with the segmentName, message and root cause. */ + public CorruptSegmentInfoException( + String segmentName, String message, String resourceDescription, Throwable cause) { + super(message, resourceDescription, cause); + this.segmentName = segmentName; + } +} diff --git a/lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java b/lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java index 44ef259e12c0..4d4ebc616f9d 100644 --- a/lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java +++ b/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; /** @@ -388,14 +389,25 @@ private static void parseSegmentInfos( } long totalDocs = 0; - + SegmentInfo info; for (int seg = 0; seg < numSegments; seg++) { String segName = input.readString(); 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); + 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); diff --git a/lucene/core/src/test/org/apache/lucene/index/TestTransactions.java b/lucene/core/src/test/org/apache/lucene/index/TestTransactions.java index 49703ac8a7c4..f8c6c60f5104 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestTransactions.java +++ b/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)) { + throw e; + } } // release resources IOUtils.closeWhileHandlingException(r1, r2); From 59472bd7a337a0f6db4394117e20f09b0ba6b2f9 Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Sun, 2 Aug 2026 14:44:56 +0300 Subject: [PATCH 2/4] GITHUB#7820: carry the segment name and root cause out of a broken .si Follow-up to the previous commit, which is @gokaai's from #12872, rebased onto main. That PR introduced CorruptSegmentInfoException but did not finish the two things asked for in its review, so nothing downstream could use the exception: - The throw site caught the codec's failure and dropped it, passing the 3-arg constructor. Mike McCandless asked on the PR: "Can we somehow return the root cause exception here and include it in CheckIndexException". The cause is what names the file on disk (NoSuchFileException carries the full path to _1.si, a truncated .si surfaces as an EOFException naming the MemorySegmentIndexInput), so dropping it discards the only concrete detail available. - segmentName was package-private with no accessor, so no caller outside org.apache.lucene.index could read it, and no test could assert on it. Changes: - CorruptSegmentInfoException keeps one constructor rather than six, requires a non-null segmentName and cause, and exposes getSegmentName(). - SegmentInfos passes the cause, and catches Exception | AssertionError rather than Exception, per the review comment that corruption in a .si "can result in exotic exceptions". IOContext.READONCE is main's, kept over the PR's READ. - CheckIndex records the name in Status#brokenSegmentName and reports it, so the message says which segment is broken instead of only that a commit point was unreadable. - TestCheckIndex#testCorruptSegmentInfoNamesTheSegment covers both a deleted and a truncated .si, asserting the segment name, the cause type, and that the cause names _1.si. #12872 had no test for the exception it added. --- .../org/apache/lucene/index/CheckIndex.java | 26 +++++- .../index/CorruptSegmentInfoException.java | 66 ++++++---------- .../apache/lucene/index/TestCheckIndex.java | 79 +++++++++++++++++++ 3 files changed, 127 insertions(+), 44 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/index/CheckIndex.java b/lucene/core/src/java/org/apache/lucene/index/CheckIndex.java index 8592c5baf915..8c09a098181d 100644 --- a/lucene/core/src/java/org/apache/lucene/index/CheckIndex.java +++ b/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 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"; } diff --git a/lucene/core/src/java/org/apache/lucene/index/CorruptSegmentInfoException.java b/lucene/core/src/java/org/apache/lucene/index/CorruptSegmentInfoException.java index 6df76f9c2dfa..24b4f3d1da1b 100644 --- a/lucene/core/src/java/org/apache/lucene/index/CorruptSegmentInfoException.java +++ b/lucene/core/src/java/org/apache/lucene/index/CorruptSegmentInfoException.java @@ -16,55 +16,41 @@ */ package org.apache.lucene.index; +import java.util.Objects; import org.apache.lucene.store.DataInput; -import org.apache.lucene.store.DataOutput; /** - * This exception is thrown when Lucene is unable to read a SegmentInfo file _N.si (When it is - * missing or corrupt) + * 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. + * + *

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 { - // The segment name of the missing .si file is always stored for accurate error reporting - String segmentName; - - /** Create exception with the segmentName and message only */ - public CorruptSegmentInfoException(String segmentName, String message, DataInput input) { - super(message, input); - this.segmentName = segmentName; - } - - /** Create exception with the segmentName and message only */ - public CorruptSegmentInfoException(String segmentName, String message, DataOutput output) { - super(message, output); - this.segmentName = segmentName; - } - - /** Create exception with the segmentName, message and root cause. */ + /** 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, cause); - this.segmentName = segmentName; + super(message, input, Objects.requireNonNull(cause)); + this.segmentName = Objects.requireNonNull(segmentName); } - /** Create exception with the segmentName, message and root cause. */ - public CorruptSegmentInfoException( - String segmentName, String message, DataOutput output, Throwable cause) { - super(message, output, cause); - this.segmentName = segmentName; - } - - /** Create exception with the segmentName and message only */ - public CorruptSegmentInfoException( - String segmentName, String message, String resourceDescription) { - super(message, resourceDescription); - this.segmentName = segmentName; - } - - /** Create exception with the segmentName, message and root cause. */ - public CorruptSegmentInfoException( - String segmentName, String message, String resourceDescription, Throwable cause) { - super(message, resourceDescription, cause); - this.segmentName = segmentName; + /** Returns the name of the segment whose {@code .si} file could not be read. */ + public String getSegmentName() { + return segmentName; } } diff --git a/lucene/core/src/test/org/apache/lucene/index/TestCheckIndex.java b/lucene/core/src/test/org/apache/lucene/index/TestCheckIndex.java index 8e2afb228ddc..e38a697ef2c7 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestCheckIndex.java +++ b/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); + } + } } From ea7e0fd4cc1e8e4111e2dc306dcc4625747da5a9 Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Sun, 2 Aug 2026 17:01:41 +0300 Subject: [PATCH 3/4] GITHUB#7820: add CHANGES entry --- lucene/CHANGES.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 92e7fed208e2..f05b630a55af 100644 --- a/lucene/CHANGES.txt +++ b/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) From 8935d53a32332e960a051c3c6188a7a3648590e3 Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Wed, 5 Aug 2026 00:45:54 +0300 Subject: [PATCH 4/4] GITHUB#7820: shrink-wrap the SegmentInfo scope and use the == false form Signed-off-by: Serhiy Bzhezytskyy --- lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java | 3 ++- .../src/test/org/apache/lucene/index/TestTransactions.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java b/lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java index 4d4ebc616f9d..1c817aa70d9b 100644 --- a/lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java +++ b/lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java @@ -389,12 +389,13 @@ private static void parseSegmentInfos( } long totalDocs = 0; - SegmentInfo info; + for (int seg = 0; seg < numSegments; seg++) { String segName = input.readString(); byte[] segmentID = new byte[StringHelper.ID_LENGTH]; input.readBytes(segmentID, 0, segmentID.length); Codec codec = readCodec(input); + final SegmentInfo info; try { info = codec.segmentInfoFormat().read(directory, segName, segmentID, IOContext.READONCE); } catch (ThreadInterruptedException e) { diff --git a/lucene/core/src/test/org/apache/lucene/index/TestTransactions.java b/lucene/core/src/test/org/apache/lucene/index/TestTransactions.java index f8c6c60f5104..c7cefb84ea44 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestTransactions.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestTransactions.java @@ -207,7 +207,7 @@ public void doWork() throws Throwable { // can be rethrown as RuntimeException if it happens during a close listener if (!e.getMessage().contains("on purpose")) { // Caught "on-purpose" IOException can be rethrown as CorruptSegmentInfoException - if (!(e instanceof CorruptSegmentInfoException)) { + if (e instanceof CorruptSegmentInfoException == false) { throw e; } }