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

[serhiy-bzhezytskyy]

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.

[mikemccand]

Thank you for resurrecting this dying-on-the-vine PR @serhiy-bzhezytskyy!

[mikemccand]

This looks great -- thank you @gokaai and @serhiy-bzhezytskyy for renewing this long ago and important improvement to our corrupt index messaging! I left small polish comments; I think this is otherwise ready.

Is it backportable to 10.x?

[mikemccand]

Thank you for the clean-as-you-go.

[mikemccand]

Could we move this inside the for loop, just above the new try {? (Shrink-wrap the scoping)

[mikemccand]

== false?