-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-27715 Refactoring the long tryAdvanceEntry method in WALEntrySt… #5105
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
| return HasNext.RETRY; | ||
| } | ||
| Pair<WALTailingReader.State, Boolean> p = readNextEntryAndRecordReaderPosition(); | ||
| state = pair.getFirst(); |
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.
There is a typo, this maybe the problem why some replication related tests are unstable...
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
| assert !pair.getSecond(); | ||
| if (!state.eof()) { | ||
| // we still have something to read after reopen | ||
| return HasNext.YES; |
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 sleep a bit and retry to see if there are new entries coming since the file is // still being written return HasNext.RETRY;"
When state not eof, the return value changed from HasNext.RETRY to HasNext.YES?
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 should also be a problem in the old code. We should return something here but in the old code, we just fall into the next switch section...
And after consideration, I think here we also need to consider whether there are errors, let me update the code.
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.
Updated. Will return RETRY if there are errors, otherwise return YES. If still get EOF, we can dequeue the current file.
2005hithlj
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. |
|
OK, good, all green. Let me merge. |
…ream (#5105) Signed-off-by: Liangjun He <[email protected]> (cherry picked from commit 1f2e1f5)
…ream