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

Conversation

original-brownbear
Copy link
Member

@original-brownbear original-brownbear added >non-issue :Distributed/Store Issues around managing unopened Lucene indices. If it touches Store.java, this is a likely label. v8.0.0 v7.2.0 labels May 7, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@original-brownbear
Copy link
Member Author

Jenkins run elasticsearch-ci/1

* As discussed in elastic#24800 we want to check for suppressed corruption
indicating exceptions here as well to more reliably categorize
corruption related exceptions
* Closes elastic#24800, 41201
@original-brownbear
Copy link
Member Author

Jenkins run elasticsearch-ci/1

@ywelsch ywelsch requested a review from jpountz May 8, 2019 11:08
Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

This should also have more tests

IOException corruptionException =
(IOException) unwrap(t, CorruptIndexException.class, IndexFormatTooOldException.class, IndexFormatTooNewException.class);
if (corruptionException == null && t != null) {
for (Throwable suppressed : t.getSuppressed()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to check suppressed exceptions of Throwable.getCause etc?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right! I guess we should will add that together with tests in a sec :)

@original-brownbear
Copy link
Member Author

@ywelsch done, added more tests and made it follow the suppressions on causes as well :)

@@ -175,10 +175,26 @@ public static String formatStackTrace(final StackTraceElement[] stackTrace) {
return first;
}

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

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 :)

@original-brownbear
Copy link
Member Author

@jpountz thanks
@ywelsch you're ok with this one too? :)

Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

LGTM

@original-brownbear
Copy link
Member Author

@jpountz @ywelsch thanks!

jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request May 22, 2019
* master: (82 commits)
  Fix off-by-one error in an index shard test
  Cleanup Redundant BlobStoreFormat Class (elastic#42195)
  remove backcompat handling of 6.2.x versions (elastic#42044)
  Mute testDelayedOperationsBeforeAndAfterRelocated
  Execute actions under permit in primary mode only (elastic#42241)
  Mute another transforms_stats yaml test
  Deprecate support for chained multi-fields. (elastic#41926)
  Mute transforms_stats yaml test
  Make unwrapCorrupt Check Suppressed Ex. (elastic#41889)
  Remove Dead Code from Azure Repo Plugin (elastic#42178)
  Reorganize Painless doc structure (elastic#42303)
  Avoid unnecessary persistence of retention leases (elastic#42299)
  [ML][TEST] Fix limits in AutodetectMemoryLimitIT (elastic#42279)
  [ML Data Frame] Persist and restore checkpoint and position (elastic#41942)
  mute failing filerealm hash caching tests (elastic#42304)
  Safer Wait for Snapshot Success in ClusterPrivilegeTests (elastic#40943)
  Remove 7.0.2 (elastic#42282)
  Revert "Remove 7.0.2 (elastic#42282)"
  [DOCS] Copied note on slicing support to Slicing section. Closes 26114 (elastic#40426)
  Remove 7.0.2 (elastic#42282)
  ...
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
* Make unwrapCorrupt Check Suppressed Ex.

* As discussed in elastic#24800 we want to check for suppressed corruption
indicating exceptions here as well to more reliably categorize
corruption related exceptions
* Closes elastic#24800, 41201
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request May 28, 2019
* Make unwrapCorrupt Check Suppressed Ex.

* As discussed in elastic#24800 we want to check for suppressed corruption
indicating exceptions here as well to more reliably categorize
corruption related exceptions
* Closes elastic#24800, 41201
original-brownbear added a commit that referenced this pull request May 28, 2019
* Make unwrapCorrupt Check Suppressed Ex. (#41889)
* 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
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Jun 3, 2019
* Make unwrapCorrupt Check Suppressed Ex.

* As discussed in elastic#24800 we want to check for suppressed corruption
indicating exceptions here as well to more reliably categorize
corruption related exceptions
* Closes elastic#24800, 41201
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Jun 3, 2019
* Make unwrapCorrupt Check Suppressed Ex. (elastic#41889)
* As discussed in elastic#24800 we want to check for suppressed corruption
indicating exceptions here as well to more reliably categorize
corruption related exceptions
* Closes elastic#24800, 41201
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Jun 3, 2019
* Make unwrapCorrupt Check Suppressed Ex. (elastic#41889)
* As discussed in elastic#24800 we want to check for suppressed corruption
indicating exceptions here as well to more reliably categorize
corruption related exceptions
* Closes elastic#24800, 41201
original-brownbear added a commit that referenced this pull request Jun 3, 2019
* Make unwrapCorrupt Check Suppressed Ex. (#41889)
* 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
original-brownbear added a commit that referenced this pull request Jun 3, 2019
* Make unwrapCorrupt Check Suppressed Ex. (#41889)
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Store Issues around managing unopened Lucene indices. If it touches Store.java, this is a likely label. >non-issue v7.1.2 v7.2.0 v7.3.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Corruption detection is fragile?
5 participants