Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make unwrapCorrupt Check Suppressed Ex. #41889

Merged
merged 4 commits into from
May 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 33 additions & 3 deletions server/src/main/java/org/elasticsearch/ExceptionsHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,42 @@ public static <T extends Throwable> T useOrSuppress(T first, T second) {
return first;
}

private static final List<Class<? extends IOException>> CORRUPTION_EXCEPTIONS =
List.of(CorruptIndexException.class, IndexFormatTooOldException.class, IndexFormatTooNewException.class);

/**
* Looks at the given Throwable's and its cause(s) as well as any suppressed exceptions on the Throwable as well as its causes
* and returns the first corruption indicating exception (as defined by {@link #CORRUPTION_EXCEPTIONS}) it finds.
* @param t Throwable
* @return Corruption indicating exception if one is found, otherwise {@code null}
*/
public static IOException unwrapCorruption(Throwable t) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add javadocs to mention that this checks both causes and suppressed exceptions?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and maybe javadocs to unwrap too to make it clear it doesn't look at suppressed exceptions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, added javadoc to both :)

return (IOException) unwrap(t, CorruptIndexException.class,
IndexFormatTooOldException.class,
IndexFormatTooNewException.class);
if (t != null) {
do {
for (Class<?> clazz : CORRUPTION_EXCEPTIONS) {
if (clazz.isInstance(t)) {
return (IOException) t;
}
}
for (Throwable suppressed : t.getSuppressed()) {
IOException corruptionException = unwrapCorruption(suppressed);
if (corruptionException != null) {
return corruptionException;
}
}
} while ((t = t.getCause()) != null);
}
return null;
}

/**
* Looks at the given Throwable and its cause(s) and returns the first Throwable that is of one of the given classes or {@code null}
* if no matching Throwable is found. Unlike {@link #unwrapCorruption} this method does only check the given Throwable and its causes
* but does not look at any suppressed exceptions.
* @param t Throwable
* @param clazzes Classes to look for
* @return Matching Throwable if one is found, otherwise {@code null}
*/
public static Throwable unwrap(Throwable t, Class<?>... clazzes) {
if (t != null) {
do {
Expand Down
28 changes: 28 additions & 0 deletions server/src/test/java/org/elasticsearch/ExceptionsHelperTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch;

import org.apache.commons.codec.DecoderException;
import org.apache.lucene.index.CorruptIndexException;
import org.elasticsearch.action.OriginalIndices;
import org.elasticsearch.action.ShardOperationFailedException;
import org.elasticsearch.action.search.ShardSearchFailure;
Expand Down Expand Up @@ -183,4 +184,31 @@ public void testGroupByNullIndex() {
ShardOperationFailedException[] groupBy = ExceptionsHelper.groupBy(failures);
assertThat(groupBy.length, equalTo(2));
}

public void testUnwrapCorruption() {
final Throwable corruptIndexException = new CorruptIndexException("corrupt", "resource");
assertThat(ExceptionsHelper.unwrapCorruption(corruptIndexException), equalTo(corruptIndexException));

final Throwable corruptionAsCause = new RuntimeException(corruptIndexException);
assertThat(ExceptionsHelper.unwrapCorruption(corruptionAsCause), equalTo(corruptIndexException));

final Throwable corruptionSuppressed = new RuntimeException();
corruptionSuppressed.addSuppressed(corruptIndexException);
assertThat(ExceptionsHelper.unwrapCorruption(corruptionSuppressed), equalTo(corruptIndexException));

final Throwable corruptionSuppressedOnCause = new RuntimeException(new RuntimeException());
corruptionSuppressedOnCause.getCause().addSuppressed(corruptIndexException);
assertThat(ExceptionsHelper.unwrapCorruption(corruptionSuppressedOnCause), equalTo(corruptIndexException));

final Throwable corruptionCauseOnSuppressed = new RuntimeException();
corruptionCauseOnSuppressed.addSuppressed(new RuntimeException(corruptIndexException));
assertThat(ExceptionsHelper.unwrapCorruption(corruptionCauseOnSuppressed), equalTo(corruptIndexException));

assertThat(ExceptionsHelper.unwrapCorruption(new RuntimeException()), nullValue());
assertThat(ExceptionsHelper.unwrapCorruption(new RuntimeException(new RuntimeException())), nullValue());

final Throwable withSuppressedException = new RuntimeException();
withSuppressedException.addSuppressed(new RuntimeException());
assertThat(ExceptionsHelper.unwrapCorruption(withSuppressedException), nullValue());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -438,10 +438,12 @@ protected void failEngine(IOException cause) {
handler.sendFiles(store, metas.toArray(new StoreFileMetaData[0]), () -> 0);
fail("exception index");
} catch (RuntimeException ex) {
assertNull(ExceptionsHelper.unwrapCorruption(ex));
final IOException unwrappedCorruption = ExceptionsHelper.unwrapCorruption(ex);
if (throwCorruptedIndexException) {
assertNotNull(unwrappedCorruption);
assertEquals(ex.getMessage(), "[File corruption occurred on recovery but checksums are ok]");
} else {
assertNull(unwrappedCorruption);
assertEquals(ex.getMessage(), "boom");
}
} catch (CorruptIndexException ex) {
Expand Down