-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-23696 Stop WALProcedureStore after migration finishes #1050
Conversation
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.
LGTM, just a question regarding stopping it on the catch body. I had something similar in HBASE-23694 after observing the issues described there, but didn't enclose it in a try/catch.
throw new IOException("Failed to delete the WALProcedureStore migrated proc wal directory " + | ||
procWALDir); | ||
LOG.info("Migration of WALProcedureStore finished"); | ||
} catch (IOException ioe) { |
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.
Do we need the catch, or could just stop it on the finally and let the IOE be thrown upwards?
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.
Sorry @wchevreuil . I only now noticed you'd just put up a fix for the same thing.
On this line, I should have rethrown the caught exception. I wanted to pass in 'true' for abort if exception as opposed to 'false' for the finally block. I might be overthinking it since IIRC, if problem here, we'll crash out the Master. Let me resolve this in favor of yours.
c04448a
to
7b28524
Compare
Closing as dupe of HBASE-23696 |
💔 -1 overall
This message was automatically generated. |
No description provided.