mikemccand commented on a change in pull request #1482:
URL: https://github.com/apache/lucene-solr/pull/1482#discussion_r419407888
##########
File path: lucene/core/src/java/org/apache/lucene/codecs/CodecUtil.java
##########
@@ -448,24 +448,27 @@ public static void checkFooter(ChecksumIndexInput in,
Throwable priorException)
checkFooter(in);
} else {
try {
+ // If we have evidence of corruption then we return the corruption as
the
+ // main exception and the prior exception gets suppressed. Otherwise we
+ // return the prior exception with a suppressed exception that notifies
+ // the user that checksums matched.
long remaining = in.length() - in.getFilePointer();
if (remaining < footerLength()) {
// corruption caused us to read into the checksum footer already: we
can't proceed
- priorException.addSuppressed(new CorruptIndexException("checksum
status indeterminate: remaining=" + remaining +
- ", please run
checkindex for more details", in));
+ throw new CorruptIndexException("checksum status indeterminate:
remaining=" + remaining +
+ ", please run checkindex for more
details", in);
} else {
// otherwise, skip any unread bytes.
in.skipBytes(remaining - footerLength());
// now check the footer
- try {
- long checksum = checkFooter(in);
- priorException.addSuppressed(new CorruptIndexException("checksum
passed (" + Long.toHexString(checksum) +
- ").
possibly transient resource issue, or a Lucene or JVM bug", in));
- } catch (CorruptIndexException t) {
- priorException.addSuppressed(t);
- }
+ long checksum = checkFooter(in);
+ priorException.addSuppressed(new CorruptIndexException("checksum
passed (" + Long.toHexString(checksum) +
Review comment:
Do we normally (in other places) also use `CorruptIndexException` to
indicate a valid checksum? I feel like we need a `NotCorruptIndexException`
for this :)
##########
File path: lucene/core/src/java/org/apache/lucene/codecs/CodecUtil.java
##########
@@ -448,24 +448,27 @@ public static void checkFooter(ChecksumIndexInput in,
Throwable priorException)
checkFooter(in);
} else {
try {
+ // If we have evidence of corruption then we return the corruption as
the
+ // main exception and the prior exception gets suppressed. Otherwise we
+ // return the prior exception with a suppressed exception that notifies
+ // the user that checksums matched.
long remaining = in.length() - in.getFilePointer();
if (remaining < footerLength()) {
// corruption caused us to read into the checksum footer already: we
can't proceed
- priorException.addSuppressed(new CorruptIndexException("checksum
status indeterminate: remaining=" + remaining +
- ", please run
checkindex for more details", in));
+ throw new CorruptIndexException("checksum status indeterminate:
remaining=" + remaining +
+ ", please run checkindex for more
details", in);
Review comment:
Nitpick: `;` instead of `,` since these are really two separate
sentences?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
