-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Simplify TranslogWriter#closeWithTragicEvent #29412
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
Simplify TranslogWriter#closeWithTragicEvent #29412
Conversation
This commit simplifies the exception handling in TranslogWriter#closeWithTragicEvent. When invoking this method, the inner close method could throw an exception which we always catch and suppress into the exception that led us to tragically close. This commit moves that repeated logic into closeWithTragicException and now callers simply need to catch, invoke closeWithTragicException, and rethrow.
Pinging @elastic/es-distributed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left one question. Maybe good to mention on the PR that some of the catch blocks have been generalized from IOException
to Exception
(so that they are uniform).
try { | ||
close(); | ||
} catch (final IOException | RuntimeException e) { | ||
ex.addSuppressed(e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just catch (final Exception e) {
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want a compile-time error if a new checked exception crops here so that it's deliberately thought through.
I have done that. |
@ywelsch I responded to your comments; thanks for taking a look. |
This commit simplifies the exception handling in TranslogWriter#closeWithTragicEvent. When invoking this method, the inner close method could throw an exception which we always catch and suppress into the exception that led us to tragically close. This commit moves that repeated logic into closeWithTragicException and now callers simply need to catch, invoke closeWithTragicException, and rethrow.
This commit simplifies the exception handling in TranslogWriter#closeWithTragicEvent. When invoking this method, the inner close method could throw an exception which we always catch and suppress into the exception that led us to tragically close. This commit moves that repeated logic into closeWithTragicException and now callers simply need to catch, invoke closeWithTragicException, and rethrow. Note also that a catch block that was only catching
IOException
has been generalized to catch any exception for consistency with the remaining invocations of TranslogWriter#closeWithTragicEvent.Relates #29401