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

Don't handle FNF exceptions when reading snapshot #8086

Merged
merged 1 commit into from Oct 22, 2014

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Oct 15, 2014

We used to handle FNF exceptions in the store when reading a snapshot.
For instance if we can't open a segments file for a given commit point
we just return an empty metadata object and tracelog the even. This can
cause shards to be false marked as corrupted if a shard is forcefully
removed while a recovery started at the same time. We should in general
bubble up these exceptions and let the caller decided how to handle the
IOExceptions.

@bleskes
Copy link
Contributor

bleskes commented Oct 15, 2014

The change looks good, but I'm worried about the implication it has for recovery as we use the same api to list the files already available on the local directory. I'm not sure if FNF can be thrown in that cause, but I think we should be safe.

I'm working on a change to the recovery code that will give us better protection in that case, after which we can pull this in? (PR Coming)

@bleskes
Copy link
Contributor

bleskes commented Oct 15, 2014

Here is the PR mentioned: #8092

@s1monw s1monw removed the v1.3.5 label Oct 22, 2014
@bleskes
Copy link
Contributor

bleskes commented Oct 22, 2014

I'm not sure if FNF can be thrown in that cause, but I think we should be safe.

It turns out this is not a concern because when listing local files on a recovery target we do not supply a commit point but rather pass a null to Store.readSegmentsInfo(commit, directory). In this case it try to find a commit point but not throw a FNF if none was found.

LGTM

We used to handle FNF exceptions in the store when reading a snapshot.
For instance if we can't open a segments file for a given commit point
we just return an empty metadata object and tracelog the even. This can
cause shards to be false marked as corrupted if a shard is forcefully
removed while a recovery started at the same time. We should in general
bubble up these exceptions and let the caller decided how to handle the
IOExceptions.
@s1monw s1monw merged commit 40945ae into elastic:master Oct 22, 2014
@clintongormley clintongormley changed the title [CORE] Don't handle FNF exceptions when reading snapshot Internal: Don't handle FNF exceptions when reading snapshot Nov 3, 2014
@clintongormley clintongormley added the :Core/Infra/Core Core issues without another label label Mar 19, 2015
@clintongormley clintongormley changed the title Internal: Don't handle FNF exceptions when reading snapshot Don't handle FNF exceptions when reading snapshot Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants