serhiy-bzhezytskyy commented on issue #7820: URL: https://github.com/apache/lucene/issues/7820#issuecomment-5157446191
#16474 guards `exorciseIndex` against the `missingSegments` case so it refuses instead of throwing NPE, and adds the first test that calls the method. It does not implement step 2. Four things I noticed while reproducing this that are not in the PR, in case they are useful for step 2. **1. The broken segment's name may already be reachable.** The November 2023 comment says the exception "does not make it clear which segment(s) are broken". Looking at the causes rather than the wrapper, it partly does: | corruption | root cause carries | |---|---| | `.si` deleted | `NoSuchFileException` with the full path to `_1.si` | | `.si` truncated | suppressed `EOFException` naming `MemorySegmentIndexInput(path=".../_1.si")` | | `.si` bit-flipped | suppressed `CorruptIndexException` "checksum passed … possibly transient", also naming the file | | `segments_N` truncated | suppressed `EOFException` naming `segments_N` | So the name is present, just only as text. A field on the exception would still be much better than parsing a message, but this may make step 2 cheaper than it looked. **2. `Status` collapses distinct corruptions into one state.** All of the above report exactly `clean=false, missingSegments=true, numBadSegments=0, totLoseDocCount=0`, yet they need different recovery: | corruption | recoverable by dropping the segment? | |---|---| | `.si` deleted / truncated / bit-flipped | yes — the other segments are intact and `segments_N` parses | | `segments_N` truncated | partly — records before the break parsed, the stream is dead after it | | `segments_N` deleted | no — there is no segment list; only rolling back to an earlier generation | `totLoseDocCount=0` is also misleading in all of them: the count is unknown, because `maxDoc` lives in the `.si` that could not be read. Whatever step 2 does, it will need something other than a number here. **3. `segments_N` deleted is a different failure again**, and it is already #7823: `checkIndex` throws `IndexNotFoundException` rather than returning a `Status`, so callers cannot distinguish "no index" from "index present but unreadable" without catching. That issue has had no comments since 2015 and asks for the same thing from the other side — a `Status` that describes why the commit point could not be read. They look like one gap seen twice. **4. The byte-flip test already exists and is disabled.** #10396 (LUCENE-9356) has @jpountz writing exactly such a test and then reverting it — commit `cf8f83c`, "Disable test, some corruptions are still not detected as corruptions", noting that some cases "do not seem straightforward to fix". Measuring from the other side agrees: flipping one byte at a time across a `.fdt` (2,000 documents, one stored field, codec and mode pinned to `Lucene104`/`BEST_SPEED`, every 37th byte, all documents read back and compared), the outcomes on `main` were `ArrayIndexOutOfBoundsException` from LZ4 in 36% of cases, an incorrect stored-field value returned with no error at all in 48%, and `CorruptIndexException` in under 1%. That sampling is coarse — 2.7% of positions, all eight bits inverted rather than one, only the first exception classified — so I offer it only as agreeing that the disabled test was disabled for a real reason. @rmuir's suggestion in that thread, to corrupt the checksum r ather than flip random bytes, looks like the way to make it deterministic. One small note on the API: `Status#newSegments` is package-private, so external callers cannot check it themselves before calling `exorciseIndex`; `missingSegments` is the only public signal. -- 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]
