-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-28665 WALs not marked closed when there are errors in closing WALs #6027
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
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Apache9
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 only for 2.5.x? Other branches are not affected?
|
@Apache9 This impacts other branches (branch-2.x) as well. I have started with 2.5 will create pull request for other branches as well. Please let me know if we need to start with any specific branch for 2.x |
|
💔 -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. |
cc950f6 to
a20ab24
Compare
|
💔 -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. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
@virajjasani @apurtell can you please help with the review |
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.
Changes look good, master branch does not check for unflushed entries anymore. @Apache9 is it because of HBASE-27231?
|
@kiran-maturi could you please create PR against branch-2? From branch-2, we can backport to both 2.6 and 2.5 cleanly. Since this PR is already opened, 2.5 is already covered but usually branch-2 PR is required to run all tests. |
| closeWriter(this.writer, oldPath, true); | ||
| } finally { | ||
| inflightWALClosures.remove(oldPath.getName()); | ||
| if (!isUnflushedEntries()) { |
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.
Similar to the code below at line 403, should we also have a catch block to Log a WARNING?
Does it make any difference to first remove old path and then call markCloseAndClean or vice versa? Since the JIRA said we are trying to follow the similar as below, we can try to keep it same?
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.
Just another question, does closeWriter also flush?
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.
@ranganathg inflightWALClosures.remove(oldPath.getName()); was there previously as well so not changing the behaviour there. We are explicitly looking for the closing for cleanup
Just another question, does closeWriter also flush?
No, this is in the write path it will not trigger any flush
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.
@ranganathg sorry I got confused with the flush for region. If you are asking for flush for the entries that are there on the ringbuffer that will be done by the sycn runner threads which will increment the sequenceId
|
Merged changes to branch-2 and branch-2.6, let me merge this PR now. |
Jira: HBASE-28665