serhiy-bzhezytskyy opened a new pull request, #16476: URL: https://github.com/apache/lucene/pull/16476
### Description `CheckIndex` reports that a commit point could not be read, but not *which* segment is at fault: ``` ERROR: could not read latest commit point from segments file "segments_3" in directory ``` The information exists at the point of failure — a deleted `.si` arrives as a `NoSuchFileException` carrying the full path, a truncated one as a suppressed `EOFException` naming the `MemorySegmentIndexInput` — but `SegmentInfos#parseSegmentInfos` lets the codec's exception propagate untyped, so nothing downstream can tell which segment it was. This is the November 2023 comment on #7820: > the exception "does not make it clear which segment(s) are broken" After this change: ``` ERROR: could not read segment "_1" referenced by the latest commit point in segments file "segments_3": its _1.si is missing or corrupt ``` ### Changes The first commit is @gokaai's from #12872, rebased onto `main` with authorship intact. It introduces `CorruptSegmentInfoException` and throws it from `SegmentInfos` when a segment's `.si` cannot be read. Two hunks needed adjusting for `main`: `IOContext.READ` became `IOContext.READONCE`, and the wildcard import was replaced with explicit ones, which the ecj lint requires. The second commit finishes the two things asked for in that PR's review but not done there: - **The root cause was dropped.** The throw site caught the codec's failure and called the three-argument constructor, discarding it. @mikemccand asked on the PR: *"Can we somehow return the root cause exception here and include it in `CheckIndexException`"*. It is now passed, and the catch is `Exception | AssertionError` rather than `Exception`, per the same review: *"Corruption in `.si` can result in exotic exceptions"*. - **`segmentName` was unreachable.** It was package-private with no accessor, so nothing outside `org.apache.lucene.index` could read the name the exception exists to carry, and no test could assert on it. `getSegmentName()` is added, the six constructors are reduced to the one that is used, and both arguments are `Objects.requireNonNull`. On top of that, `CheckIndex` records the name in `Status#brokenSegmentName` and puts it in the message, and `TestCheckIndex#testCorruptSegmentInfoNamesTheSegment` covers a deleted and a truncated `.si`, asserting the segment name, the cause's type, and that the cause names `_1.si`. #12872 had no test for the exception it added. ### Relationship to #12872 and #16474 #12872 has been open since 2023-12 and is no longer mergeable; the last human comment there is from 2024-02, after a `git` digression, and @mikemccand's *"I'll try to review soon!"* never got a follow-up. The pain it addresses is still there, so this carries it forward rather than leaving it. If @gokaai would rather finish it there, close this one — the point is the fix landing, not where. #16474 is the other half of the same thread: `exorciseIndex` throws `NullPointerException` instead of refusing when `Status.newSegments` is null. That is the piece @gokaai explicitly deferred: > Will create a new commit (or issue?) to add in fixes and unit tests for `exorciseIndex` The two are independent and can land in either order. ### Verification - `testCorruptSegmentInfoNamesTheSegment` fails without the fix, and fails again if the root cause is replaced with a synthetic one — so it checks the cause rather than just its presence - `:lucene:core:test` for `TestCheckIndex`, `TestTransactions`, `SegmentInfos*` and `IndexWriter*` passes (362 tests) - `:lucene:core:check` and `tidy` pass Verified on `main` only. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
