GITHUB#7820: refuse to exorcise an index whose commit point could not be read

[serhiy-bzhezytskyy]

Description

CheckIndex#exorciseIndex dereferences Status#newSegments after checking only Status#partial. When the commit point cannot be read, checkIndex leaves newSegments null and sets missingSegments instead, so the public method fails with:

java.lang.NullPointerException: Cannot invoke "org.apache.lucene.index.SegmentInfos.changed()"
  because "result.newSegments" is null
  at org.apache.lucene.index.CheckIndex.exorciseIndex(CheckIndex.java:4594)

Reproduced on main with three corruptions, each of them one that users report on #7820:

corruption	DirectoryReader.open	checkIndex	exorciseIndex
_1.si deleted	CorruptIndexException	clean=false missingSegments=true	NPE
_1.si truncated	CorruptIndexException	same	NPE
segments_N truncated	CorruptIndexException	same	NPE

The command line is not affected — doCheck has if (result.missingSegments) return 1; at CheckIndex.java:4880. Luke is: CheckIndexDialogFactory enables its Repair button on if (!st.clean) (:304) and reads missingSegments only when building the result text (:329), while IndexUtils#tryRepairIndex (:318) calls ci.exorciseIndex(st) with no check of its own. So a Luke user who checks an index with an unreadable commit point gets an enabled Repair button and an NPE when they press it.

This is what @mikemccand warned about when skipMissing was proposed on #7820 in 2015:

skipMissing is a spooky option; if accidentally mis-used it can make you think you have all segments if you forget to check missingSegmentInfo

This change does not implement the repair that #7820 asks for. It only makes the failure legible until that exists, and the guard is symmetric with the partial check already in the method.

The test

@rmuir asked for exactly this on #12530 in August 2023:

as far as the exorcise stuff, I think a good next step would be to start writing some unit tests that invoke exorcise? we have a grand total of zero tests exercising-exorcise, and I think it is part of the problem.

@mikemccand agreed in the same thread — "It's terrifying to have zero test coverage for such a dangerous feature!". grep exorciseIndex lucene/core/src/test/ still returns nothing on main, so testExorciseUnreadableCommitPoint is the first test that calls the method at all. It covers all three corruptions and sits next to testPriorBrokenCommitPoint, which is step 1 of the same issue.

Relationship to #12872

@gokaai's PR #12872 is open since 2023-12 and its description says it is "a step towards enabling #7820". Its author also wrote there:

Will create a new commit (or issue?) to add in fixes and unit tests for exorciseIndex

That is what this PR does, so it is not a competing change — it is the piece deferred there, and it stands whether or not #12872 lands. If it is easier to fold this into that PR instead, I will close this one. I am not proposing to take over step 2.

Verification

• The test fails without the fix: AssertionFailedError: Unexpected exception type, expected IllegalArgumentException but got java.lang.NullPointerException
• Removing the guard and re-running fails; restoring it passes
• ./gradlew :lucene:core:test --tests TestCheckIndex passes
• ./gradlew :lucene:core:check -x test passes
• ./gradlew tidy reports no changes

Verified on main only; the 10.x branches are not checked.