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

Improve exception from Store.failIfCorrupted #7596

Closed
rmuir opened this issue Sep 4, 2014 · 2 comments · Fixed by #7695
Closed

Improve exception from Store.failIfCorrupted #7596

rmuir opened this issue Sep 4, 2014 · 2 comments · Fixed by #7695

Comments

@rmuir
Copy link
Contributor

rmuir commented Sep 4, 2014

If you have previously corrupted files, this method currently builds an exception like:

failed engine [corrupted preexisting index]
failed to start shard

Followed by a CorruptIndexException:

builder.append(" Corrupted index [");
builder.append(file).append("] caused by: ");
builder.append(msg);
ex.add(new CorruptIndexException(builder.toString()));

Is it possible to improve this:

  1. In the string we write (msg), can the string have more details when we write it, e.g. including full stracktrace?
  2. can we change the text 'corrupted preexisting index' to either 'preexisting corrupted index' or 'index has preexisting corruption' or similar?
@s1monw
Copy link
Contributor

s1monw commented Sep 5, 2014

I wonder if we should do both. And log the stacktrace as debug?

@clintongormley
Copy link

@rmuir are you taking this or should it be an adoptme?

s1monw added a commit to s1monw/elasticsearch that referenced this issue Sep 11, 2014
If you have previously corrupted files, this method currently builds an
exception like:
```
    failed engine [corrupted preexisting index]
    failed to start shard
```

Followed by a CorruptIndexException. This commit writes the entire
stacktrace to provide additional information. It also changes the
failure message from `corrupted preexisting index` to `preexisting
corrupted index` to prevent confusion.

Closes elastic#7596
s1monw added a commit that referenced this issue Sep 11, 2014
If you have previously corrupted files, this method currently builds an
exception like:
```
    failed engine [corrupted preexisting index]
    failed to start shard
```

Followed by a CorruptIndexException. This commit writes the entire
stacktrace to provide additional information. It also changes the
failure message from `corrupted preexisting index` to `preexisting
corrupted index` to prevent confusion.

Closes #7596
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants