From 094d17eb60ce2f88ac8792d8f7dd178b223bb95d Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 7 May 2019 14:39:05 +0200 Subject: [PATCH 1/3] Make unwrapCorrupt Check Suppressed Ex. * As discussed in #24800 we want to check for suppressed corruption indicating exceptions here as well to more reliably categorize corruption related exceptions * Closes #24800, 41201 --- .../java/org/elasticsearch/ExceptionsHelper.java | 14 +++++++++++--- .../recovery/RecoverySourceHandlerTests.java | 4 +++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/ExceptionsHelper.java b/server/src/main/java/org/elasticsearch/ExceptionsHelper.java index e4269a375dd6c..17c4cc9ca5c31 100644 --- a/server/src/main/java/org/elasticsearch/ExceptionsHelper.java +++ b/server/src/main/java/org/elasticsearch/ExceptionsHelper.java @@ -176,9 +176,17 @@ public static T useOrSuppress(T first, T second) { } public static IOException unwrapCorruption(Throwable t) { - return (IOException) unwrap(t, CorruptIndexException.class, - IndexFormatTooOldException.class, - IndexFormatTooNewException.class); + IOException corruptionException = + (IOException) unwrap(t, CorruptIndexException.class, IndexFormatTooOldException.class, IndexFormatTooNewException.class); + if (corruptionException == null && t != null) { + for (Throwable suppressed : t.getSuppressed()) { + corruptionException = unwrapCorruption(suppressed); + if (corruptionException != null) { + return corruptionException; + } + } + } + return corruptionException; } public static Throwable unwrap(Throwable t, Class... clazzes) { diff --git a/server/src/test/java/org/elasticsearch/indices/recovery/RecoverySourceHandlerTests.java b/server/src/test/java/org/elasticsearch/indices/recovery/RecoverySourceHandlerTests.java index b63c7a2e0e8f6..b49bef57aceb1 100644 --- a/server/src/test/java/org/elasticsearch/indices/recovery/RecoverySourceHandlerTests.java +++ b/server/src/test/java/org/elasticsearch/indices/recovery/RecoverySourceHandlerTests.java @@ -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) { From c53f1853054a7ed345532ef9d07860c828f1954c Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 8 May 2019 13:49:52 +0200 Subject: [PATCH 2/3] CR: also catch suppressions on the causes --- .../org/elasticsearch/ExceptionsHelper.java | 26 +++++++++++------ .../elasticsearch/ExceptionsHelperTests.java | 28 +++++++++++++++++++ 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/ExceptionsHelper.java b/server/src/main/java/org/elasticsearch/ExceptionsHelper.java index 17c4cc9ca5c31..a8e4d1dd73807 100644 --- a/server/src/main/java/org/elasticsearch/ExceptionsHelper.java +++ b/server/src/main/java/org/elasticsearch/ExceptionsHelper.java @@ -175,18 +175,26 @@ public static T useOrSuppress(T first, T second) { return first; } + private static final List> CORRUPTION_EXCEPTIONS = + List.of(CorruptIndexException.class, IndexFormatTooOldException.class, IndexFormatTooNewException.class); + public static IOException unwrapCorruption(Throwable t) { - IOException corruptionException = - (IOException) unwrap(t, CorruptIndexException.class, IndexFormatTooOldException.class, IndexFormatTooNewException.class); - if (corruptionException == null && t != null) { - for (Throwable suppressed : t.getSuppressed()) { - corruptionException = unwrapCorruption(suppressed); - if (corruptionException != null) { - return corruptionException; + 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 corruptionException; + return null; } public static Throwable unwrap(Throwable t, Class... clazzes) { diff --git a/server/src/test/java/org/elasticsearch/ExceptionsHelperTests.java b/server/src/test/java/org/elasticsearch/ExceptionsHelperTests.java index 1d2a4ca6d5f75..2de2f259e6ff1 100644 --- a/server/src/test/java/org/elasticsearch/ExceptionsHelperTests.java +++ b/server/src/test/java/org/elasticsearch/ExceptionsHelperTests.java @@ -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; @@ -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()); + } } From 3692054884a1d6f2d557152c8a14d350e70fc20f Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Fri, 10 May 2019 10:31:23 +0200 Subject: [PATCH 3/3] CR: Add javadoc --- .../java/org/elasticsearch/ExceptionsHelper.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/ExceptionsHelper.java b/server/src/main/java/org/elasticsearch/ExceptionsHelper.java index a8e4d1dd73807..48461ffe30d4b 100644 --- a/server/src/main/java/org/elasticsearch/ExceptionsHelper.java +++ b/server/src/main/java/org/elasticsearch/ExceptionsHelper.java @@ -178,6 +178,12 @@ public static T useOrSuppress(T first, T second) { private static final List> 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) { if (t != null) { do { @@ -197,6 +203,14 @@ public static IOException unwrapCorruption(Throwable t) { 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 {