You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/05/04 07:42:46 UTC
[GitHub] [lucene-solr] jpountz opened a new pull request #1482: LUCENE-7822: CodecUtil#checkFooter should throw a CorruptIndexException as the main exception.
jpountz opened a new pull request #1482:
URL: https://github.com/apache/lucene-solr/pull/1482
See https://issues.apache.org/jira/browse/LUCENE-7822.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1482: LUCENE-7822: CodecUtil#checkFooter should throw a CorruptIndexException as the main exception.
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1482:
URL: https://github.com/apache/lucene-solr/pull/1482#discussion_r419467116
##########
File path: lucene/core/src/test/org/apache/lucene/util/TestOfflineSorter.java
##########
@@ -353,12 +352,10 @@ protected void corruptFile() throws IOException {
// This corruption made OfflineSorter fail with its own exception, but we verify it also went and added (as suppressed) that the
Review comment:
no longer as suppressed
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jpountz commented on a change in pull request #1482: LUCENE-7822: CodecUtil#checkFooter should throw a CorruptIndexException as the main exception.
Posted by GitBox <gi...@apache.org>.
jpountz commented on a change in pull request #1482:
URL: https://github.com/apache/lucene-solr/pull/1482#discussion_r421333675
##########
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:
This case is interesting indeed, it could either be a Lucene bug or a silent corruption.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on a change in pull request #1482: LUCENE-7822: CodecUtil#checkFooter should throw a CorruptIndexException as the main exception.
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org