-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-24713 RS startup with FSHLog throws NPE after HBASE-21751 #2125
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
Conversation
ramkrish86
left a comment
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.
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
virajjasani
left a comment
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.
+1
|
Seems the null check was causing some issues as the finally was not getting executed. |
|
🎊 +1 overall
This message was automatically generated. |
ramkrish86
left a comment
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.
This is the right fix. Checked the code. When the WAL is being created first time. We still ask the ring buffer for the first sequence id and expect the runner thread to release that sync future though we have not written any data. We need a clean up there. But that in another JIRA. So this null check should be safe and simple.
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
Think that the test case failures are unrelated. Will commit this in a day or two unless there are any more comments. |
|
Hasn't the NPE been fixed by: HBASE-24034 [Flakey Tests] A couple of fixes and cleanups In FSHLog.java, I see the FSHLog.java:582 mentioned in a comment. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
No.. Here the NPE comes as the writer is null. As part of the initial writer create (that happens via rollWriter call), we ended up attaining a safe point and so publish a sync. This in turn try to call writer.sync() and so the NPE. |
|
Test case failure is unrelated to this fix. |
No description provided.