Navigation Menu

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

Refactor the Translog.read(Location) method #7780

Merged
merged 1 commit into from Sep 29, 2014

Conversation

dakrone
Copy link
Member

@dakrone dakrone commented Sep 18, 2014

It was only used by readSource, it has been changed to return a
Translog.Operation, which can have .getSource() called on it to return
the source. readSource has been removed.

This also removes the checked IOException, any exception thrown is
unexpected and should throw a runtime exception.

Moves the ReleasableBytesStreamOutput allocation into the body of the
try-catch block so the lock can be released in the event of an exception
during allocation.

@kimchy
Copy link
Member

kimchy commented Sep 18, 2014

LGTM

}
} catch (IOException e) {
// switched on us, read it from the reader
Translog.Operation op = translog.read(versionValue.translogLocation());
Copy link
Contributor

Choose a reason for hiding this comment

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

this can still cause an IOException no? so we should keep the catch block here?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh no it now throws ElasticsearchException which I think is weird it should throw IOException

@s1monw
Copy link
Contributor

s1monw commented Sep 18, 2014

I left some comments

@s1monw s1monw removed the review label Sep 19, 2014
@dakrone dakrone added the review label Sep 19, 2014
@s1monw
Copy link
Contributor

s1monw commented Sep 24, 2014

LGTM

It was only used by `readSource`, it has been changed to return a
Translog.Operation, which can have .getSource() called on it to return
the source. `readSource` has been removed.

This also removes the checked IOException, any exception thrown is
unexpected and should throw a runtime exception.

Moves the ReleasableBytesStreamOutput allocation into the body of the
try-catch block so the lock can be released in the event of an exception
during allocation.
@dakrone dakrone merged commit 168b375 into elastic:master Sep 29, 2014
@dakrone dakrone deleted the minor-translog-cleanup branch September 29, 2014 08:47
@clintongormley clintongormley changed the title Refactor the Translog.read(Location) method Internal: Refactor the Translog.read(Location) method Nov 3, 2014
@clintongormley clintongormley changed the title Internal: Refactor the Translog.read(Location) method Refactor the Translog.read(Location) method Jun 7, 2015
@clintongormley clintongormley added :Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. and removed :Translog :Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. labels Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Engine Anything around managing Lucene and the Translog in an open shard. >enhancement v1.4.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants