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 catch FNF/NSF exception when reading metadata #8207

Merged
merged 1 commit into from Oct 24, 2014

Conversation

Projects
None yet
3 participants
@s1monw
Copy link
Contributor

s1monw commented Oct 23, 2014

When reading metadata we do catch FileNotFound and NoSuchFileExceptions
today, log the even and return an empty metadata object. Yet, in some cases
this might be the wrong thing todo ie. if a commit point is provided these
situations are actually an error and should be rethrown. This commit
pushes the responsiblity to the caller to handle this exception.

@s1monw s1monw force-pushed the s1monw:empty_map_on_no_index branch Oct 23, 2014

@s1monw s1monw changed the title [STORE] Rethrow FNF exception if commit-point is provided [STORE] Don't catch FNF/NSF exception when reading metadata Oct 23, 2014

@s1monw

This comment has been minimized.

Copy link
Contributor Author

s1monw commented Oct 23, 2014

@bleskes can you take a look at this please?

@bleskes

View changes

src/main/java/org/elasticsearch/index/store/Store.java Outdated
/**
* Returns a new MetadataSnapshot for the latest commit in this store or
* an empty snapshot if no index exists or can not be opened
*/

This comment has been minimized.

Copy link
@bleskes

bleskes Oct 24, 2014

Member

I think it's good to explicitly mention that this may throw an IndexCorruptedException

This comment has been minimized.

Copy link
@s1monw

s1monw Oct 24, 2014

Author Contributor

will do

@bleskes

View changes

src/main/java/org/elasticsearch/indices/recovery/RecoveryTarget.java Outdated
@@ -153,9 +152,9 @@ private void doRecovery(final RecoveryStatus recoveryStatus) {
assert recoveryStatus.sourceNode() != null : "can't do a recovery without a source node";

logger.trace("collecting local files for {}", recoveryStatus);
final Map<String, StoreFileMetaData> existingFiles;
Map<String, StoreFileMetaData> existingFiles = Collections.emptyMap();

This comment has been minimized.

Copy link
@bleskes

bleskes Oct 24, 2014

Member

I wonder why this change? initial value is unused and once existing files are set, they are never changed.

This comment has been minimized.

Copy link
@s1monw

s1monw Oct 24, 2014

Author Contributor

leftover I will fix

@bleskes

This comment has been minimized.

Copy link
Member

bleskes commented Oct 24, 2014

LGTM. Left two minor comments.

@s1monw

This comment has been minimized.

Copy link
Contributor Author

s1monw commented Oct 24, 2014

@bleskes pushed a new commit

@bleskes

This comment has been minimized.

Copy link
Member

bleskes commented Oct 24, 2014

looks great. Thx @s1monw

[STORE] Don't catch FNF/NSF exception when reading metadata
When reading metadata we do catch FileNotFound and NoSuchFileExceptions
today, log the even and return an empty metadata object. Yet, in some cases
this might be the wrong thing todo ie. if a commit point is provided these
situations are actually an error and should be rethrown. This commit
pushes the responsiblity to the caller to handle this exception.

Closes #8207

@s1monw s1monw force-pushed the s1monw:empty_map_on_no_index branch to c09af6d Oct 24, 2014

@s1monw s1monw removed the review label Oct 24, 2014

@s1monw s1monw merged commit c09af6d into elastic:master Oct 24, 2014

s1monw added a commit that referenced this pull request Oct 24, 2014

[STORE] Don't catch FNF/NSF exception when reading metadata
When reading metadata we do catch FileNotFound and NoSuchFileExceptions
today, log the even and return an empty metadata object. Yet, in some cases
this might be the wrong thing todo ie. if a commit point is provided these
situations are actually an error and should be rethrown. This commit
pushes the responsiblity to the caller to handle this exception.

Closes #8207

s1monw added a commit that referenced this pull request Oct 24, 2014

[STORE] Don't catch FNF/NSF exception when reading metadata
When reading metadata we do catch FileNotFound and NoSuchFileExceptions
today, log the even and return an empty metadata object. Yet, in some cases
this might be the wrong thing todo ie. if a commit point is provided these
situations are actually an error and should be rethrown. This commit
pushes the responsiblity to the caller to handle this exception.

Closes #8207

s1monw added a commit that referenced this pull request Oct 24, 2014

[STORE] Don't catch FNF/NSF exception when reading metadata
When reading metadata we do catch FileNotFound and NoSuchFileExceptions
today, log the even and return an empty metadata object. Yet, in some cases
this might be the wrong thing todo ie. if a commit point is provided these
situations are actually an error and should be rethrown. This commit
pushes the responsiblity to the caller to handle this exception.

Closes #8207

@clintongormley clintongormley changed the title [STORE] Don't catch FNF/NSF exception when reading metadata Resiliency: Don't catch FNF/NSF exception when reading metadata Nov 3, 2014

@clintongormley clintongormley changed the title Resiliency: Don't catch FNF/NSF exception when reading metadata Don't catch FNF/NSF exception when reading metadata Jun 7, 2015

mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015

[STORE] Don't catch FNF/NSF exception when reading metadata
When reading metadata we do catch FileNotFound and NoSuchFileExceptions
today, log the even and return an empty metadata object. Yet, in some cases
this might be the wrong thing todo ie. if a commit point is provided these
situations are actually an error and should be rethrown. This commit
pushes the responsiblity to the caller to handle this exception.

Closes elastic#8207

mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015

[STORE] Don't catch FNF/NSF exception when reading metadata
When reading metadata we do catch FileNotFound and NoSuchFileExceptions
today, log the even and return an empty metadata object. Yet, in some cases
this might be the wrong thing todo ie. if a commit point is provided these
situations are actually an error and should be rethrown. This commit
pushes the responsiblity to the caller to handle this exception.

Closes elastic#8207
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.