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) 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..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"; } @@ -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(); 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..24b4f3d1da1b --- /dev/null +++ b/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. + * + *

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; + } +} 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..1c817aa70d9b 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; /** @@ -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); 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); + } + } } 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..c7cefb84ea44 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 == false) { + throw e; + } } // release resources IOUtils.closeWhileHandlingException(r1, r2);